test/java/util/Map/Defaults.java

rev 7682 : 8021591: Additional explicit null checks
Reviewed-by: duke

@@ -153,11 +153,11 @@
             NullPointerException.class,
             description);
         assertThrows(
             () -> { map.replaceAll((k,v) -> null); },
             NullPointerException.class,
-            description);
+            description + " should not allow replacement with null value");
     }
 
     @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=withNull values=withNull")
     public static void testRemoveNulls(String description, Map<IntegerEnum, String> map) {
         assertTrue(map.containsKey(null), "null key absent");

@@ -192,10 +192,19 @@
         assertNull(map.get(null), "value not null");
         assertSame(map.replace(null, EXTRA_VALUE), null);
         assertSame(map.get(null), EXTRA_VALUE);
     }
 
+    @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=nonNull values=nonNull")
+    public void testReplaceKVNoNulls(String description, Map<IntegerEnum, String> map) {
+        assertTrue(map.containsKey(FIRST_KEY), "expected key missing");
+        assertSame(map.get(FIRST_KEY), FIRST_VALUE, "found wrong value");
+        assertThrows( () -> {map.replace(FIRST_KEY, null);}, NullPointerException.class, description + ": should throw NPE");
+        assertSame(map.replace(FIRST_KEY, EXTRA_VALUE), FIRST_VALUE, description + ": replaced wrong value");
+        assertSame(map.get(FIRST_KEY), EXTRA_VALUE, "found wrong value");
+    }
+
     @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=all values=all")
     public void testReplaceKV(String description, Map<IntegerEnum, String> map) {
         assertTrue(map.containsKey(KEYS[1]));
         Object expected = map.get(KEYS[1]);
         assertTrue(null == expected || expected == VALUES[1]);

@@ -222,10 +231,20 @@
         assertSame(map.get(null), EXTRA_VALUE);
         assertTrue(map.replace(null, EXTRA_VALUE, EXTRA_VALUE));
         assertSame(map.get(null), EXTRA_VALUE);
     }
 
+    @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=nonNull values=nonNull")
+    public void testReplaceKVVNoNulls(String description, Map<IntegerEnum, String> map) {
+        assertTrue(map.containsKey(FIRST_KEY), "expected key missing");
+        assertSame(map.get(FIRST_KEY), FIRST_VALUE, "found wrong value");
+        assertThrows( () -> {map.replace(FIRST_KEY, FIRST_VALUE, null);}, NullPointerException.class, description + ": should throw NPE");
+        assertThrows( () -> {if (!map.replace(FIRST_KEY, null, EXTRA_VALUE)) throw new NullPointerException("default returns false rather than throwing");}, NullPointerException.class,  description + ": should throw NPE");
+        assertTrue(map.replace(FIRST_KEY, FIRST_VALUE, EXTRA_VALUE), description + ": replaced wrong value");
+        assertSame(map.get(FIRST_KEY), EXTRA_VALUE, "found wrong value");
+    }
+
     @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=all values=all")
     public void testReplaceKVV(String description, Map<IntegerEnum, String> map) {
         assertTrue(map.containsKey(KEYS[1]));
         Object expected = map.get(KEYS[1]);
         assertTrue(null == expected || expected == VALUES[1]);

@@ -468,10 +487,13 @@
         for (int each = 0; each < TEST_SIZE; each++) {
             KEYS[each] = keys[each];
             VALUES[each] = String.valueOf(each);
         }
     }
+
+    private static final IntegerEnum FIRST_KEY = KEYS[0];
+    private static final String FIRST_VALUE = VALUES[0];
     private static final IntegerEnum EXTRA_KEY = IntegerEnum.EXTRA_KEY;
     private static final String EXTRA_VALUE = String.valueOf(TEST_SIZE);
 
     @DataProvider(name = "Map<IntegerEnum,String> rw=all keys=all values=all", parallel = true)
     public static Iterator<Object[]> allMapProvider() {

@@ -581,22 +603,25 @@
      */
     private static Collection<Object[]> makeRWNoNullKeysMaps(boolean nulls) {
         return Arrays.asList(
                 // null key hostile
                 new Object[]{"EnumMap", makeMap(() -> new EnumMap(IntegerEnum.class), false, nulls)},
+                new Object[]{"TreeMap", makeMap(TreeMap::new, false, nulls)},
+                new Object[]{"ExtendsAbstractMap(TreeMap)", makeMap(() -> {return new ExtendsAbstractMap(new TreeMap());}, false, nulls)},
                 new Object[]{"Collections.synchronizedMap(EnumMap)", Collections.synchronizedMap(makeMap(() -> new EnumMap(IntegerEnum.class), false, nulls))}
                 );
     }
 
     private static Collection<Object[]> makeRWNoNullsMaps() {
         return Arrays.asList(
             // null key and value hostile
             new Object[]{"Hashtable", makeMap(Hashtable::new, false, false)},
-            new Object[]{"TreeMap", makeMap(TreeMap::new, false, false)},
             new Object[]{"ConcurrentHashMap", makeMap(ConcurrentHashMap::new, false, false)},
             new Object[]{"ConcurrentSkipListMap", makeMap(ConcurrentSkipListMap::new, false, false)},
+            new Object[]{"Collections.synchronizedMap(ConcurrentHashMap)", Collections.synchronizedMap(makeMap(ConcurrentHashMap::new, false, false))},
             new Object[]{"Collections.checkedMap(ConcurrentHashMap)", Collections.checkedMap(makeMap(ConcurrentHashMap::new, false, false), IntegerEnum.class, String.class)},
+            new Object[]{"ExtendsAbstractMap(ConcurrentHashMap)", makeMap(() -> {return new ExtendsAbstractMap(new ConcurrentHashMap());}, false, false)},
             new Object[]{"ImplementsConcurrentMap", makeMap(ImplementsConcurrentMap::new, false, false)}
             );
     }
 
     /**

@@ -639,35 +664,34 @@
     public static <T extends Throwable> void assertThrows(Thrower<T> thrower, Class<T> throwable) {
         assertThrows(thrower, throwable, null);
     }
 
     public static <T extends Throwable> void assertThrows(Thrower<T> thrower, Class<T> throwable, String message) {
-        Throwable result;
+        Throwable thrown;
         try {
             thrower.run();
-            result = null;
+            thrown = null;
         } catch (Throwable caught) {
-            result = caught;
+            thrown = caught;
         }
 
-        assertInstance(result, throwable,
-            (null != message)
-            ? message
-            : "Failed to throw " + throwable.getCanonicalName());
+        assertInstance(thrown, throwable,
+            ((null != message) ? message : "") +
+            " Failed to throw " + throwable.getCanonicalName());
     }
 
     public static <T extends Throwable> void assertThrows(Class<T> throwable, String message, Thrower<T>... throwers) {
         for(Thrower<T> thrower : throwers) {
             assertThrows(thrower, throwable, message);
         }
     }
 
-    public static <T> void assertInstance(T actual, Class<? extends T> expected) {
+    public static void assertInstance(Object actual, Class<?> expected) {
         assertInstance(expected.isInstance(actual), null);
     }
 
-    public static <T> void assertInstance(T actual, Class<? extends T> expected, String message) {
+    public static void assertInstance(Object actual, Class<?> expected, String message) {
         assertTrue(expected.isInstance(actual), message);
     }
 
     /**
      * A simple mutable map implementation that provides only default