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: @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.