Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec (original) (raw)

Ulf Zibis Ulf.Zibis at gmx.de
Sun Apr 1 17:01:48 UTC 2012

• Previous message: Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec
• Next message: Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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/viewbug.do?bug_id=7121314 <http://bugs.sun.com/bugdatabase/viewbug.do?bugid=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 extends AbstractCollection { 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 iterator() { return new Iterator() { 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(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); test7121314(); } /* * Major target of this testcase, bug 7121314. */ protected void test7121314() 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(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 extends AbstractCollection {

   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.");
           }
       };
   }

  }

}

• Previous message: Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec
• Next message: Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

More information about the core-libs-dev mailing list