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

[Ulf Zibis]

Hi David,

thanks to hear again from you after some time.

Am 28.03.2012 07:29, schrieb David Holmes:
> Hi Ulf,
>
> I understand your point about ensuring we test AbstractCollection.toArray
Thanks to hear that.

> but I find this revised test much harder to understand.
Sorry for that, to me it's contrary. (reading own code seems easier ;-) )
To me (1) it's a long journey to use a _concurrent_ Map, which is not subclassed from Collection, 
fill keys AND values manually, manipulate it's size method and receive a candidate Collection via 
values(), (2) have separate subclass for each testcase and (3) it's hard to see, if 
TConcurrentHashMap.values() returns a real AbstractCollection type. (4) my size-manipulation code is 
only a 1-liner, (5) must not be duplicated for each testcase:
=================================
         return sizes[nextSize == sizes.length-1 ? nextSize : nextSize++];
---------------------------------
         int oldsize = super.size();
         remove(keys[0]);
         remove(keys[1]);
         check(super.size() == oldsize - 2);
         return oldsize;
=================================

Don't forget, that my TestCollection is prepared for much more general usage, not for only method 
toArray(), and my code covers 12 testcases instead 2. Additionally it would serve as a template for 
all tests on all existing collections subclassed from AbstractCollection. Currently, it is not 
tested if they all behave correct according "return the original array if the collection shrinks and 
will fit".

In general I would prefer, writing tests in using JUnit infrastructure, but I don't know the 
boundary conditions for jtreg.
Do you know about an example in the jdk repo?

> Also in the name setPseudoConcurrentSizeCourse the word "Course" doesn't fit. I'm not sure what 
> you were meaning here?
:-( I meant this: http://dict.leo.org/ende?search=Verlauf,
     or more specific: http://dict.leo.org/ende?search=zeitlicher%20Verlauf

>   Perhaps just modifySize or emulateConcurrentSizeChange ?
Hm, the modification or emulation is done by the size() method, not by setXxxYyyZyy().
If you find a better translation, maybe setSizeZzz would be enough.
Maybe you could use 'prepare' instead 'set'.

I have missed an important testcase, see attachment...

> Thanks,
> David
Thanks too,
-Ulf


>
> On 28/03/2012 3:01 PM, Ulf Zibis wrote:
>> Hi Sean,
>>
>> Am 26.03.2012 07:02, schrieb Sean Chou:
>>>
>>> On Sat, Mar 24, 2012 at 5:09 AM, Ulf Zibis <Ulf.Zibis at gmx.de
>>> <mailto:Ulf.Zibis at gmx.de>> wrote:
>>>
>>>         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.
>>>
>>> Every testcase or fix goes this way, like the first testcase I
>>> provided. If your suggestion is valuable, I don't think it will be wasted.
>> Ok, here it is.
>>
>>>     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.
>>>
>>> What's the exact problem you want to fix in this case?
>> The execution time of jdk test cases.
>>
>> -Ulf
>>
>
-------------- next part --------------
import java.util.*;

/**
 * @test
 * @summary check result, especially if the collection concurrently changes size
 * @bug 7121314
 * @author Ulf Zibis
 */
public class ToArray extends InfraStructure {

    static final Object[] OBJECTS = { new Object(), new Object(), new Object() };
    static final TestCollection<?> CANDIDATE = new TestCollection<Object>(OBJECTS);
    static final int SIZE = OBJECTS.length;
    Object[] a;
    Object[] res;

    @Override
    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[SIZE]);
            check(false);
        } catch (Throwable t) {
            check(t instanceof ArrayStoreException);
        }

        // Check more elements than a.length
        a = new Object[SIZE-1]; // appears too small
        res = CANDIDATE.toArray(a);
        check(res != a);
        check(res[SIZE-1] != null);

        // Check equal elements as a.length
        a = new Object[SIZE]; // appears to match
        res = CANDIDATE.toArray(a);
        check(res == a);
        check(res[a.length-1] != null);

        // Check equal elements as a.length
        a = new Object[SIZE+1]; // appears too big
        res = CANDIDATE.toArray(a);
        check(res == a);
        check(res[a.length-1] == null);

        // Check less elements than expected, but more than a.length
        a = new Object[SIZE-2]; // appears too small
        CANDIDATE.setPseudoConcurrentSizeCourse(SIZE, SIZE-1);
        res = CANDIDATE.toArray(a);
        check(res != a);
        check(res.length == SIZE-1);
        check(res[SIZE-2] != null);

        // Check less elements than expected, but equal as a.length
        a = Arrays.copyOf(OBJECTS, SIZE); // appears to match
        CANDIDATE.setPseudoConcurrentSizeCourse(SIZE, SIZE-1);
        res = CANDIDATE.toArray(a);
        check(res == a);
        check(res[a.length-1] == null);

        // Check more elements than expected and more than a.length
        a = new Object[SIZE-1]; // appears to match
        CANDIDATE.setPseudoConcurrentSizeCourse(SIZE-1, SIZE);
        res = CANDIDATE.toArray(a);
        check(res != a);
        check(res[SIZE-1] != null);

        // Check more elements than expected, but equal as a.length
        a = new Object[SIZE-1]; // appears to match
        CANDIDATE.setPseudoConcurrentSizeCourse(SIZE-2, SIZE-1);
        res = CANDIDATE.toArray(a);
        check(res == a);
        check(res[a.length-1] != null);

        // Check more elements than expected, but less than a.length
        a = Arrays.copyOf(OBJECTS, SIZE); // appears to match
        CANDIDATE.setPseudoConcurrentSizeCourse(SIZE-2, SIZE-1);
        res = CANDIDATE.toArray(a);
        check(res == a);
        check(res[a.length-1] == null);

        test_7121314();
     }

    /**
     * @bug 7121314
     * @summary return the original array if the collection concurrently shrinks and will fit
     */
    protected void test_7121314() throws Throwable {
        // Check equal elements as a.length, but less than expected
        a = new Object[SIZE-1]; // appears too small
        CANDIDATE.setPseudoConcurrentSizeCourse(SIZE, SIZE-1);
        res = CANDIDATE.toArray(a);
        check(res == a);
        check(res[a.length-1] != null);

        // Check less elements than a.length and less than expected
        a = Arrays.copyOf(OBJECTS, SIZE-1); // appears too small
        CANDIDATE.setPseudoConcurrentSizeCourse(SIZE, SIZE-2);
        res = CANDIDATE.toArray(a);
        check(res == a);
        check(res[a.length-1] == null);
    }

    public static void main(String[] args) throws Throwable {
        run(new ToArray());
    }
}