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 at 47a7a8ef, 1365, 2730, ParallelPrefix$$Lambda$56/1605520928 at 106dbc48): success
> test ParallelPrefix.testParallelPrefixForInt([I at 789e465e, 1365, 2731, ParallelPrefix$$Lambda$55/1326195546 at 2c8625ae): failure
> java.lang.AssertionError: expected [[I at 7b703d4a] but found [[I at 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;
> +        }
> +    }
>   }
>   
>
>