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

Ulf Zibis Ulf.Zibis at gmx.de
Fri Mar 23 21:09:26 UTC 2012

Hi Sean,

Am 23.03.2012 07:45, schrieb Sean Chou:

Hi Ulf,

I'm sorry I didn't quite get your testcase. This is not a testcase, just a draft of an additionally more flexible test object to replace TConcurrentHashMap/2.

Will you please provide a jtreg style testcase with main method ? Well, as I'm missing your agreement, that David's test implementation doesn't guarantee to test the right toArray method of AbstractCollection as I explained before, I'm afraid that additional effort would be for garbage.

Aside, as the instantiation of (several) ConcurrentHashMap subclassed test objects seems more expensive, I believe, my simple TestCollection would increase the performance of the testcases.

I was thinking you were worried by the comment "// inherits from AbstractCollection.toArray()" and "map2", if that was the case, I would like to modify. This is additionally true, but not my main issue, as later to me it turned out, that "// inherits from AbstractCollection.toArray()" is not enough to guarantee that the right toArray method is actually tested, aside to look not obvious IMO.

-Ulf

P.S.: better rename to: void setPseudoConcurrentSizeCourse(int... sizes) {...}

On Fri, Mar 23, 2012 at 5:57 AM, Ulf Zibis <Ulf.Zibis at gmx.de <mailto:Ulf.Zibis at gmx.de>> wrote: Hi Sean, bad news ;-) ... Am 22.03.2012 08:28, schrieb Sean Chou: Hi Ulf, I'm glad you agreed my suggestion. To all: Can this patch be committed as it has been reviewed by David Holmes and Mike Duigou, and Ulf also says agreed ?

I agree with your implementation of AbstractCollection, but NOT with the test. For correct testing I suggest to use: /** * * @author Ulf Zibis */ public class TestCollection extends AbstractCollection { private E[] elements; private int[] sizes; private int nextSize; public TestCollection(E[] elements) { this.elements = elements; setConcurrentSizeCourse(null); } void setConcurrentSizeCourse(int... sizes) { this.sizes = sizes == null ? new int[]{elements.length} : sizes; nextSize = 0; } @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."); } }; } } -Ulf

-- Best Regards, Sean Chou

More information about the core-libs-dev mailing list