Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec
Ulf Zibis
Ulf.Zibis at gmx.de
Sun Apr 1 17:01:48 UTC 2012
Hi David, Sean,
tonight, I started to refactor the test for JUnit infrastructure.
Ok, it is too late.
I attach it anyway for your interest, maybe next time in JUnit or testNG (which I'm not familiar)...
Thanks for the involved discussion,
-Ulf
Am 01.04.2012 05:25, schrieb David Holmes:
> Simplified testcase below.
>
> Let's finalise this please.
>
> Thanks,
> David
>
> On 1/04/2012 1:08 PM, Sean Chou wrote:
>> Hi Ulf,
>>
>> This is a regression testcase, there is no performance issue or
>> future refactoring.
>> Please wait for David's comments.
>>
>> On Sun, Apr 1, 2012 at 7:22 AM, Ulf Zibis <Ulf.Zibis at gmx.de
>> <mailto:Ulf.Zibis at gmx.de>> wrote:
>>
>> Hi Sean,
>>
>> thanks for your effort.
>>
>> Am 31.03.2012 11:43, schrieb Sean Chou:
>>
>> Hi David and Ulf,
>>
>> The new webrev is at:
>> http://cr.openjdk.java.net/~__zhouyx/7121314/webrev.03/
>> <http://cr.openjdk.java.net/~zhouyx/7121314/webrev.03/>
>> <http://cr.openjdk.java.net/%__7Ezhouyx/7121314/webrev.03/
>> <http://cr.openjdk.java.net/%7Ezhouyx/7121314/webrev.03/>> .
>>
>>
>> About the fix, I remained the previous one.
>> About the testcase, I merged the 3 files into one.
>> During merging, there are 2 modifications:
>> 1. I added static modifier to the 3 classes, which are enclosed
>> by class ToArrayTest;
>>
>> You do not need the indirection via main()...run()...test() if you
>> have all in 1 file. This was only necessary to feature a general
>> usability of InfraStructure. You can go back to David's 1 + 1 nested
>> class approach replacing TConcurrentHashMapX by TestCollection and
>> maybe rename realMain() to test().
>> Additionally, loading 4 classes for 1 test would have some
>> performance impact on the test run, which could be avoided.
>>
>>
>> 2. I removed field TestCollection.fixedSize, which is never read
>> after Ulf fixed the bug in testcase.
>>
>> This field would serve to "reset" the TestCollection to fixed
>> default size without the need of new instantiation for later
>> refactoring or testcase addition.
>>
>> As just discussed before, the doc for setSizeSequence() could be
>> little more specific:
>> 71 /*
>> 72 * Sets the values that size() will return on each use.
>> The first
>> 73 * call to size will return sizes[0], then sizes[1]
>> etc. This
>> 74 * allows us to emulate a concurrent change to the
>> contents of
>> 75 * the collection without having to perform concurrent
>> changes.
>> 76 * If sizes[n+1] contains a larger value than on last
>> n-th invocation,
>> 77 * the collection will appear to have shrunk when
>> iterated; if a
>> 78 * smaller value then the collection will appear to
>> have grown.
>> 79 * When the last element of sizes is reached, the
>> collection will
>> 80 * appear size-fixed.
>> 81 */
>>
>>
>> The link to the bug:
>> http://bugs.sun.com/__bugdatabase/view_bug.do?bug___id=7121314
>> <http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7121314>
>> The link to the start of the thread:
>> http://mail.openjdk.java.net/__pipermail/core-libs-dev/2012-__March/009512.html
>> <http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-March/009512.html>
>>
>> Good idea, to repeat these links.
>>
>> -Ulf
>>
>>
>>
>>
>> --
>> Best Regards,
>> Sean Chou
>>
>
>
> /*
> * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> * under the terms of the GNU General Public License version 2 only, as
> * published by the Free Software Foundation.
> *
> * This code is distributed in the hope that it will be useful, but WITHOUT
> * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> * version 2 for more details (a copy is included in the LICENSE file that
> * accompanied this code).
> *
> * You should have received a copy of the GNU General Public License version
> * 2 along with this work; if not, write to the Free Software Foundation,
> * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> *
> * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
> * or visit www.oracle.com if you need additional information or have any
> * questions.
> */
>
> /*
> * @test
> * @bug 7121314
> * @summary AbstractCollection.toArray(T[]) doesn't return the given array
> * in concurrent modification.
> * @author Ulf Zibis, David Holmes
> */
>
> import java.util.AbstractCollection;
> import java.util.Arrays;
> import java.util.Iterator;
>
> public class ToArrayTest {
>
> static class TestCollection<E> extends AbstractCollection<E> {
>
> private final E[] elements;
>
> private int[] sizes;
> private int nextSize;
>
> public TestCollection(E[] elements) {
> this.elements = elements;
> setSizeSequence(new int[]{elements.length});
> }
>
> /*
> * Sets the values that size() will return on each use. The next
> * call to size will return sizes[0], then sizes[1] etc. This
> * allows us to emulate a concurrent change to the contents of
> * the collection without having to perform concurrent changes.
> * If sizes[n+1] contains a larger value, the collection will appear to
> * have shrunk when iterated; if a smaller value then the
> * collection will appear to have grown when iterated
> */
> void setSizeSequence(int... sizes) {
> this.sizes = sizes;
> nextSize = 0;
> }
>
> /* can change collection's size after each invocation */
> @Override
> public int size() {
> return sizes[nextSize == sizes.length-1 ? nextSize : nextSize++];
> }
>
> @Override
> public Iterator<E> iterator() {
> return new Iterator<E>() {
> int pos = 0;
> public boolean hasNext() {
> return pos < sizes[nextSize];
> }
> public E next() {
> return elements[pos++];
> }
> public void remove() {
> throw new UnsupportedOperationException("Not supported yet.");
> }
> };
> }
> }
>
> static final Object[] OBJECTS = { new Object(), new Object(), new Object() };
> static final TestCollection<?> CANDIDATE = new TestCollection<Object>(OBJECTS);
> static final int CAP = OBJECTS.length; // capacity of the CANDIDATE
> static final int LAST = CAP - 1; // last possible array index
> Object[] a;
> Object[] res;
>
> int last() { return a.length - 1; }
>
> protected void test() throws Throwable {
> // Check array type conversion
> res = new TestCollection<>(new Object[]{"1", "2"}).toArray(new String[0]);
> check(res instanceof String[]);
> check(res.length == 2);
> check(res[1] == "2");
>
> // Check incompatible type of target array
> try {
> res = CANDIDATE.toArray(new String[CAP]);
> check(false);
> } catch (Throwable t) {
> check(t instanceof ArrayStoreException);
> }
>
> // Check more elements than a.length
> a = new Object[CAP-1]; // appears too small
> res = CANDIDATE.toArray(a);
> check(res != a);
> check(res[LAST] != null);
>
> // Check equal elements as a.length
> a = new Object[CAP]; // appears to match
> res = CANDIDATE.toArray(a);
> check(res == a);
> check(res[last()] != null);
>
> // Check equal elements as a.length
> a = new Object[CAP+1]; // appears too big
> res = CANDIDATE.toArray(a);
> check(res == a);
> check(res[last()] == null);
>
> // Check less elements than expected, but more than a.length
> a = new Object[CAP-2]; // appears too small
> CANDIDATE.setSizeSequence(CAP, CAP-1);
> res = CANDIDATE.toArray(a);
> check(res != a);
> check(res.length == CAP-1);
> check(res[LAST-1] != null);
>
> // Check less elements than expected, but equal as a.length
> a = Arrays.copyOf(OBJECTS, CAP); // appears to match
> CANDIDATE.setSizeSequence(CAP, CAP-1);
> res = CANDIDATE.toArray(a);
> check(res == a);
> check(res[last()] == null);
>
> // Check more elements than expected and more than a.length
> a = new Object[CAP-1]; // appears to match
> CANDIDATE.setSizeSequence(CAP-1, CAP);
> res = CANDIDATE.toArray(a);
> check(res != a);
> check(res[LAST] != null);
>
> // Check more elements than expected, but equal as a.length
> a = new Object[CAP-1]; // appears to match
> CANDIDATE.setSizeSequence(CAP-2, CAP-1);
> res = CANDIDATE.toArray(a);
> check(res == a);
> check(res[last()] != null);
>
> // Check more elements than expected, but less than a.length
> a = Arrays.copyOf(OBJECTS, CAP); // appears to match
> CANDIDATE.setSizeSequence(CAP-2, CAP-1);
> res = CANDIDATE.toArray(a);
> check(res == a);
> check(res[last()] == null);
>
> test_7121314();
> }
>
> /*
> * Major target of this testcase, bug 7121314.
> */
> protected void test_7121314() throws Throwable {
> // Check equal elements as a.length, but less than expected
> a = new Object[CAP-1]; // appears too small
> CANDIDATE.setSizeSequence(CAP, CAP-1);
> res = CANDIDATE.toArray(a);
> check(res == a);
> check(res[last()] != null);
>
> // Check less elements than a.length and less than expected
> a = Arrays.copyOf(OBJECTS, CAP-1); // appears too small
> CANDIDATE.setSizeSequence(CAP, CAP-2);
> res = CANDIDATE.toArray(a);
> check(res == a);
> check(res[last()] == null);
> }
>
>
> public static void main(String[] args) throws Throwable {
> ToArrayTest testcase = new ToArrayTest();
> try { testcase.test(); } catch (Throwable t) { unexpected(t); }
> System.out.printf("%nPassed = %d, failed = %d%n%n", passed, failed);
> if (failed > 0) throw new Exception("Some tests failed");
> }
>
> private static volatile int passed = 0, failed = 0;
> private static void pass() { passed++; }
> private static void fail() { failed++; Thread.dumpStack(); }
> private static void fail(String msg) { System.out.println(msg); fail(); }
> private static void unexpected(Throwable t) { failed++; t.printStackTrace(); }
> static void check(boolean cond) { if (cond) pass(); else fail(); }
> static void equals(Object x, Object y) {
> if (x == null ? y == null : x.equals(y)) pass();
> else {System.out.println(x + " not equal to " + y); fail(); }}
>
> }
>
>
>
-------------- next part --------------
package java.util;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.junit.Assert.*;
/*
* Check AbstractCollection, especially if the collection concurrently changes size
* @test
* @bug 7121314
* @summary return the original array if the collection concurrently shrinks and will fit
* @run junit/samevm
* @author Ulf Zibis, David Holmes
*/
public class AbstractCollectionTest {
private static final Object[] OBJECTS = { new Object(), new Object(), new Object() };
private static final AbstractCollectionImpl<?> CANDIDATE = new AbstractCollectionImpl<Object>(OBJECTS);
private static final int CAP = OBJECTS.length; // capacity of the CANDIDATE
private static final int LAST = CAP - 1; // last possible array index
private Object[] a;
private Object[] result;
private int last() { return a.length - 1; } // last possible input array index
@Before
public void setUp() {
CANDIDATE.setSizeSequence(null);
}
@Test
@Ignore
public void testIterator() {}
@Test
@Ignore
public void testSize() {}
@Test
@Ignore
public void testIsEmpty() {}
@Test
@Ignore
public void testContains() {}
@Test
@Ignore
public void testToArray_0args() {}
/**
* Test of toArray method, of class AbstractCollection.
*/
@Test
public void testToArray_GenericType() {
System.out.println("toArray(T[] a)");
/// Standard targets ///
// Check array type conversion
result = new AbstractCollectionImpl(new Object[]{"1", "2"}).toArray(new String[0]);
assertThat(result, instanceOf(String[].class));
assertEquals(result.length, 2);
assertEquals(result[1], "2");
// Check incompatible type of target array
try {
result = CANDIDATE.toArray(new String[CAP]);
fail();
} catch (Throwable t) {
assertThat(t, instanceOf(ArrayStoreException.class));
}
// Check null input
try {
result = CANDIDATE.toArray(null);
fail();
} catch (Throwable t) {
assertThat(t, instanceOf(NullPointerException.class));
}
// Check more elements than a.length
a = new Object[CAP-1]; // appears too small
result = CANDIDATE.toArray(a);
assertNotSame(result, a);
assertNotNull(result[LAST]);
// Check equal elements as a.length
a = new Object[CAP]; // appears to match
result = CANDIDATE.toArray(a);
assertSame(result, a);
assertNotNull(result[last()]);
// Check less elements than a.length
a = new Object[CAP+1]; // appears too big
result = CANDIDATE.toArray(a);
assertSame(result, a);
assertNull(result[last()]);
// Check less elements than expected, but more than a.length
a = new Object[CAP-2]; // appears too small
CANDIDATE.setSizeSequence(CAP, CAP-1);
result = CANDIDATE.toArray(a);
assertNotSame(result, a);
assertEquals(result.length, CAP-1);
assertNotNull(result[LAST-1]);
// Check less elements than expected, but equal as a.length
a = Arrays.copyOf(OBJECTS, CAP); // appears to match
CANDIDATE.setSizeSequence(CAP, CAP-1);
result = CANDIDATE.toArray(a);
assertSame(result, a);
assertNull(result[last()]);
// Check more elements than expected and more than a.length
a = new Object[CAP-1]; // appears to match
CANDIDATE.setSizeSequence(CAP-1, CAP);
result = CANDIDATE.toArray(a);
assertNotSame(result, a);
assertNotNull(result[LAST]);
// Check more elements than expected, but equal as a.length
a = new Object[CAP-1]; // appears to match
CANDIDATE.setSizeSequence(CAP-2, CAP-1);
result = CANDIDATE.toArray(a);
assertSame(result, a);
assertNotNull(result[last()]);
// Check more elements than expected, but less than a.length
a = Arrays.copyOf(OBJECTS, CAP); // appears to match
CANDIDATE.setSizeSequence(CAP-2, CAP-1);
result = CANDIDATE.toArray(a);
assertSame(result, a);
assertNull(result[last()]);
/// Major targets of this testcase, bug 7121314 ///
// Check equal elements as a.length, but less than expected
a = new Object[CAP-1]; // appears too small
CANDIDATE.setSizeSequence(CAP, CAP-1);
result = CANDIDATE.toArray(a);
assertSame(result, a);
assertNotNull(result[last()]);
// Check less elements than a.length and less than expected
a = Arrays.copyOf(OBJECTS, CAP-1); // appears too small
CANDIDATE.setSizeSequence(CAP, CAP-2);
result = CANDIDATE.toArray(a);
assertSame(result, a);
assertNull(result[last()]);
}
@Test
@Ignore
public void testAdd() {}
@Test
@Ignore
public void testRemove() {}
@Test
@Ignore
public void testContainsAll() {}
@Test
@Ignore
public void testAddAll() {}
@Test
@Ignore
public void testRemoveAll() {}
@Test
@Ignore
public void testRetainAll() {}
@Test
@Ignore
public void testClear() {}
@Test
@Ignore
public void testToString() {}
private static class AbstractCollectionImpl<E> extends AbstractCollection<E> {
private final E[] elements;
private final int[] fixedSize;
private int[] sizes;
private int nextSize;
private AbstractCollectionImpl(E[] elements) {
this.elements = elements;
setSizeSequence(fixedSize = new int[]{elements.length});
}
/**
* Sets the values that size() will return on each use. The first
* call to size will return sizes[0], then sizes[1] etc. This
* allows us to emulate a concurrent change to the contents of
* the collection without having to perform concurrent changes.
* If sizes[n+1] contains a larger value than on last n-th invocation,
* the collection will appear to have shrunk when iterated; if a
* smaller value then the collection will appear to have grown.
* When the last element of sizes is reached, the collection will
* appear size-fixed.
*/
private void setSizeSequence(int... sizes) {
this.sizes = sizes != null ? sizes : fixedSize;
nextSize = 0;
}
/** Returns collection's size; can potentially change after each invocation */
@Override
public int size() {
return sizes[nextSize == sizes.length-1 ? nextSize : nextSize++];
}
@Override
public Iterator<E> iterator() {
return new Iterator<E>() {
int pos = 0;
public boolean hasNext() {
return pos < sizes[nextSize];
}
public E next() {
return elements[pos++];
}
public void remove() {
throw new UnsupportedOperationException("Not supported yet.");
}
};
}
}
}
More information about the core-libs-dev
mailing list