On Sep 3 2013, at 02:03 , Paul Sandoz wrote:
>
> On Aug 28, 2013, at 12:56 AM, Mike Duigou <[email protected]> wrote:
>
>> Hello all;
>>
>> Here's an updated version of the patch which incorporates feedback and
>> improves the tests (the reason for delay):
>>
>> http://cr.openjdk.java.net/~mduigou/JDK-8021591/1/webrev/
>>
>> The substance of the patch is largely to add missing checks that the
>> collection provided to removeAll()/retainAll() is not null. The
>> specification of these methods in the Collection interface has always
>> indicated that an NPE should be thrown if the passed collection was null.
>> Historically various implementations were inconsistent about whether they
>> threw the NPE if a null collection was passed. Some collections would throw
>> the NPE, some would not. The intent of this patch is to improve consistency
>> and since there were examples of the NPE being correctly thrown the most
>> prudent approach seems to have all implementations throw the NPE. If there
>> had been no examples of the NPE being thrown then it would have been more
>> prudent to amend the interface spec to remove the NPE notice.
>>
>> A few other inconsistencies around null handling were also resolved. Some
>> unit tests issues were also cleaned up.
>>
>
> Looks OK.
> If you have the will I bet you could transform the following pattern repeated
> in many tests:
Heh, all of the test refactoring was a distraction for this patch. I did want
to keep going with changes like this but opted to wrap things up and move on
with the primary focus of the changeset.
Certainly this is a good suggestion though.
Mike
>
> @Test
> public void testForEach() throws Exception {
> CollectionSupplier<Collection<Integer>> supplier = new
> CollectionSupplier((Supplier<Collection<Integer>>[]) TEST_CLASSES, SIZE);
> for (final CollectionSupplier.TestCase<Collection<Integer>> test :
> supplier.get()) { ... }
>
> to:
>
> @DataProvider(name="CollectionSupplier")
> private static Iterator<CollectionSupplier.TestCase<Collection<Integer>>>
> CollectionSupplierDataProvider() {
> CollectionSupplier<Collection<Integer>> supplier = new
> CollectionSupplier((Supplier<Collection<Integer>>[]) TEST_CLASSES, SIZE);
> return supplier.get();
> }
>
> @Test(dataProvider = "CollectionSupplier")
> public void testForEach(CollectionSupplier.TestCase<Collection<Integer>>
> test) throws Exception {
>
> Paul.