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.