RFR: 8021591 : (s) Additional explicit null checks

[Mike Duigou]

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

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