On Sep 3 2013, at 02:03 , Paul Sandoz wrote:

> 
> On Aug 28, 2013, at 12:56 AM, Mike Duigou <mike.dui...@oracle.com> 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.

Reply via email to