RFR [9]: Better failure output for test/java/util/Arrays/ParallelPrefix.java

[Chris Hegarty]

Thanks for the reviews.

On 4 Jun 2015, at 13:59, Ivan Gerasimov <ivan.gerasimov at oracle.com> wrote:

> Wouldn't it cause the additional error message be printed _before_ the line 'test:...... : failure' ?
> I guess, It may look confusing.
> 
> Would it make sense to re-throw the exception with this additional message instead?
> 
> +        try {
> +            assertEquals(actual, expected, "");
> +        } catch (AssertionError x) {
> +            throw new AssertionError(String.format("Expected:%s, actual:%s%n",
> +                    Arrays.toString(expected), Arrays.toString(actual)), x);
> +        }

You are right. Adding the exception as the cause, or suppressed, is better. Changed.

On 4 Jun 2015, at 13:37, Paul Sandoz <paul.sandoz at oracle.com> wrote:

> ...
> If you wanna go the extra mile it's useful for the data provider to supply a string description argument summarizing the test data.

Added.

> You might want to size the arrays in proportion to the parallelism since the threshold is calculated as:
> 
> this.threshold =
>        (p = (hi - lo) / (ForkJoinPool.getCommonPoolParallelism() << 3))
>        <= MIN_PARTITION ? MIN_PARTITION : p;
> 
> So for a large machine with say a parallelism of 2^5 an array size of 2^12 is not sufficient to go above MIN_PARTITION..

I could be wrong, and I accept your point about sizing as per parallelism, but I think 2^12 should be sufficient for most systems, given MIN_PARTITION = 16. With a parallelism of 2^5, then p will be greater than MIN_PARTITION, no?

Latest changes below.

-Chris.

diff --git a/test/java/util/Arrays/ParallelPrefix.java b/test/java/util/Arrays/ParallelPrefix.java
--- a/test/java/util/Arrays/ParallelPrefix.java
+++ b/test/java/util/Arrays/ParallelPrefix.java
@@ -60,7 +60,7 @@
         LARGE_ARRAY_SIZE
     };
 
-    @DataProvider
+    @DataProvider(name = "intSet")
     public static Object[][] intSet(){
         return genericData(size -> IntStream.range(0, size).toArray(),
                 new IntBinaryOperator[]{
@@ -68,7 +68,7 @@
                     Integer::min});
     }
 
-    @DataProvider
+    @DataProvider(name = "longSet")
     public static Object[][] longSet(){
         return genericData(size -> LongStream.range(0, size).toArray(),
                 new LongBinaryOperator[]{
@@ -76,7 +76,7 @@
                     Long::min});
     }
 
-    @DataProvider
+    @DataProvider(name = "doubleSet")
     public static Object[][] doubleSet(){
         return genericData(size -> IntStream.range(0, size).mapToDouble(i -> (double)i).toArray(),
                 new DoubleBinaryOperator[]{
@@ -84,7 +84,7 @@
                     Double::min});
     }
 
-    @DataProvider
+    @DataProvider(name = "stringSet")
     public static Object[][] stringSet(){
         Function<Integer, String[]> stringsFunc = size ->
                 IntStream.range(0, size).mapToObj(Integer::toString).toArray(String[]::new);
@@ -121,11 +121,11 @@
 
         int[] parallelResult = data.clone();
         Arrays.parallelPrefix(parallelResult, fromIndex, toIndex, op);
-        assertEquals(parallelResult, sequentialResult);
+        assertArraysEqual(parallelResult, sequentialResult);
 
         int[] parallelRangeResult = Arrays.copyOfRange(data, fromIndex, toIndex);
         Arrays.parallelPrefix(parallelRangeResult, op);
-        assertEquals(parallelRangeResult, Arrays.copyOfRange(sequentialResult, fromIndex, toIndex));
+        assertArraysEqual(parallelRangeResult, Arrays.copyOfRange(sequentialResult, fromIndex, toIndex));
     }
 
     @Test(dataProvider="longSet")
@@ -137,11 +137,11 @@
 
         long[] parallelResult = data.clone();
         Arrays.parallelPrefix(parallelResult, fromIndex, toIndex, op);
-        assertEquals(parallelResult, sequentialResult);
+        assertArraysEqual(parallelResult, sequentialResult);
 
         long[] parallelRangeResult = Arrays.copyOfRange(data, fromIndex, toIndex);
         Arrays.parallelPrefix(parallelRangeResult, op);
-        assertEquals(parallelRangeResult, Arrays.copyOfRange(sequentialResult, fromIndex, toIndex));
+        assertArraysEqual(parallelRangeResult, Arrays.copyOfRange(sequentialResult, fromIndex, toIndex));
     }
 
     @Test(dataProvider="doubleSet")
@@ -153,11 +153,11 @@
 
         double[] parallelResult = data.clone();
         Arrays.parallelPrefix(parallelResult, fromIndex, toIndex, op);
-        assertEquals(parallelResult, sequentialResult);
+        assertArraysEqual(parallelResult, sequentialResult);
 
         double[] parallelRangeResult = Arrays.copyOfRange(data, fromIndex, toIndex);
         Arrays.parallelPrefix(parallelRangeResult, op);
-        assertEquals(parallelRangeResult, Arrays.copyOfRange(sequentialResult, fromIndex, toIndex));
+        assertArraysEqual(parallelRangeResult, Arrays.copyOfRange(sequentialResult, fromIndex, toIndex));
     }
 
     @Test(dataProvider="stringSet")
@@ -169,11 +169,11 @@
 
         String[] parallelResult = data.clone();
         Arrays.parallelPrefix(parallelResult, fromIndex, toIndex, op);
-        assertEquals(parallelResult, sequentialResult);
+        assertArraysEqual(parallelResult, sequentialResult);
 
         String[] parallelRangeResult = Arrays.copyOfRange(data, fromIndex, toIndex);
         Arrays.parallelPrefix(parallelRangeResult, op);
-        assertEquals(parallelRangeResult, Arrays.copyOfRange(sequentialResult, fromIndex, toIndex));
+        assertArraysEqual(parallelRangeResult, Arrays.copyOfRange(sequentialResult, fromIndex, toIndex));
     }
 
     @Test
@@ -293,5 +293,41 @@
     public static void assertInstance(Object actual, Class<?> expected, String message) {
         assertTrue(expected.isInstance(actual), message);
     }
+
+    static void assertArraysEqual(int[] actual, int[] expected) {
+        try {
+            assertEquals(actual, expected, "");
+        } catch (AssertionError x) {
+            throw new AssertionError(String.format("Expected:%s, actual:%s",
+                    Arrays.toString(expected), Arrays.toString(actual)), x);
+        }
+    }
+
+    static void assertArraysEqual(long[] actual, long[] expected) {
+        try {
+            assertEquals(actual, expected, "");
+        } catch (AssertionError x) {
+            throw new AssertionError(String.format("Expected:%s, actual:%s",
+                    Arrays.toString(expected), Arrays.toString(actual)), x);
+        }
+    }
+
+    static void assertArraysEqual(double[] actual, double[] expected) {
+        try {
+            assertEquals(actual, expected, "");
+        } catch (AssertionError x) {
+            throw new AssertionError(String.format("Expected:%s, actual:%s",
+                    Arrays.toString(expected), Arrays.toString(actual)), x);
+        }
+    }
+
+    static void assertArraysEqual(String[] actual, String[] expected) {
+        try {
+            assertEquals(actual, expected, "");
+        } catch (AssertionError x) {
+            throw new AssertionError(String.format("Expected:%s, actual:%s",
+                    Arrays.toString(expected), Arrays.toString(actual)), x);
+        }
+    }
 }