Change looks good to me(not a reviewer). Cheers, Henry
On 11/01/2013 10:57 AM, Paul Sandoz wrote:
Hi, This is a "eating humble pie and should of correctly listened to Henry in review" kind of email :-) The changes to DistinctOpTest recently committed result in a test failure, since one of the tests is incorrectly asserting on a particular element, which is non-determinisitic process [*]. See below for a patch that fixes the test. Paul. [*] Somewhat interesting how this can arise given the sequential nature of the source and the way it is split. Intuitively one would expect that the first F/J leaf task is more likely to complete before the second, but we alternate forking of left/right tasks when splitting. The first leaf task gets forked and the next leaf task gets computed directly on the same thread that forked the first leaf task. Thus setting the common pool parallelism to 0 (or running on a single core machine) will consistently make the test fail, and this probably explains why it was caught so soon. hg qdiff diff -r 18c111c17231 test/java/util/stream/test/org/openjdk/tests/java/util/stream/DistinctOpTest.java --- a/test/java/util/stream/test/org/openjdk/tests/java/util/stream/DistinctOpTest.java Thu Oct 31 11:59:04 2013 +0100 +++ b/test/java/util/stream/test/org/openjdk/tests/java/util/stream/DistinctOpTest.java Fri Nov 01 18:42:08 2013 +0100 @@ -54,10 +54,14 @@ // These tests should short-circuit, otherwise will fail with a time-out // or an OOME - Integer one = Stream.iterate(1, i -> i + 1).unordered().parallel().distinct().findAny().get(); - assertEquals(one.intValue(), 1); + // Note that since the streams are unordered and any element is requested + // (a non-deterministic process) the only assertion that can be made is + // that an element should be found - Optional<Integer> oi = ThreadLocalRandom.current().ints().boxed().parallel().distinct().findAny(); + Optional<Integer> oi = Stream.iterate(1, i -> i + 1).unordered().parallel().distinct().findAny(); + assertTrue(oi.isPresent()); + + oi = ThreadLocalRandom.current().ints().boxed().parallel().distinct().findAny(); assertTrue(oi.isPresent()); }