Thanks for the reviews. On 4 Jun 2015, at 13:59, Ivan Gerasimov <ivan.gerasi...@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.san...@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); + } + } }