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

[Ivan Gerasimov]

Hi Chris!

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);
+        }

Sincerely yours,
Ivan

On 04.06.2015 13:54, Chris Hegarty wrote:
This is a review request to add better test failure output to 
jdk/test/java/util/Arrays/ParallelPrefix.java

This test has been seen to fail occasionally on recent JDK 9 builds. It fails 
with a message similar to:

...
test ParallelPrefix.testParallelPrefixForInt([I@47a7a8ef, 1365, 2730, 
ParallelPrefix$$Lambda$56/1605520928@106dbc48): success
test ParallelPrefix.testParallelPrefixForInt([I@789e465e, 1365, 2731, 
ParallelPrefix$$Lambda$55/1326195546@2c8625ae): failure
java.lang.AssertionError: expected [[I@7b703d4a] but found [[I@73625cb4]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:494)
at org.testng.Assert.assertArrayEquals(Assert.java:144)
at org.testng.Assert.assertEquals(Assert.java:117)
at org.testng.Assert.assertEquals(Assert.java:165)
at ParallelPrefix.testParallelPrefixForInt(ParallelPrefix.java:124)
…

The output is not very helpful. Testng, or at least the version that we have in 
use in jtreg, does not provide pretty failure messages when dealing with 
arrays. It is trivial to add these to the test, see below. Note: I used the 
3-args version of assertEquals as it will trigger the inclusion of the index of 
the array where the values differ.

Hopefully, when we see this test fail again it can produce some meaningful 
output.

-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
@@ -39,6 +39,7 @@
  import static org.testng.Assert.*;
  import org.testng.annotations.DataProvider;
  import org.testng.annotations.Test;
+import static java.lang.System.out;
  
  public class ParallelPrefix {
      //Array size less than MIN_PARTITION
@@ -121,11 +122,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 +138,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 +154,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 +170,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 +294,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) {
+            out.printf("Expected:%s, actual:%s%n", Arrays.toString(expected), 
Arrays.toString(actual));
+            throw x;
+        }
+    }
+
+    static void assertArraysEqual(long[] actual, long[] expected) {
+        try {
+            assertEquals(actual, expected, "");
+        } catch (AssertionError x) {
+            out.printf("Expected:%s, actual:%s%n", Arrays.toString(expected), 
Arrays.toString(actual));
+            throw x;
+        }
+    }
+
+    static void assertArraysEqual(double[] actual, double[] expected) {
+        try {
+            assertEquals(actual, expected, "");
+        } catch (AssertionError x) {
+            out.printf("Expected:%s, actual:%s%n", Arrays.toString(expected), 
Arrays.toString(actual));
+            throw x;
+        }
+    }
+
+    static void assertArraysEqual(String[] actual, String[] expected) {
+        try {
+            assertEquals(actual, expected, "");
+        } catch (AssertionError x) {
+            out.printf("Expected:%s, actual:%s%n", Arrays.toString(expected), 
Arrays.toString(actual));
+            throw x;
+        }
+    }
  }