Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec (original) (raw)
Ulf Zibis Ulf.Zibis at gmx.de
Wed Mar 28 13:13:50 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,
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(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()); }
}
- 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 ]