On Fri, 1 Dec 2023 22:16:45 GMT, Viktor Klang <vkl...@openjdk.org> wrote:
>> Renames GatherersTest to BuiltInGatherersTest for easier deduplication of
>> GathererTest.
>>
>> Fixes a test ordering issue in testMapConcurrentAPIandContract().
>>
>> Adding increased maxOutputSize for Gatherer-related tests to improve
>> debuggability.
>>
>> Lowering the composition threshold of
>> GathererTest.testMassivelyComposedGatherers to 256 to avoid SOE on
>> low-specc:ed machines.
>
> Viktor Klang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Only run the cancellation tests for mapConcurrent for the shorter streams
> to conserve resources
test/jdk/java/util/stream/BuiltInGatherersTest.java line 253:
> 251: throw new RuntimeException();
> 252: })).toList());
> 253: }
mapConcurrent is specified throw RuntimeException if the mapper throws. I read
this to mean an Error is wrapped, and ambiguous on whether a runtime exception
thrown by the mapper is wrapped or not. So I think this is actually tests,
Error, RuntimeException and another runtime exception. This would help with the
"Test cancellation after exception during processing" test too as it's not
clear if TestException or a wrapping RuntimeException is correct.
Not important for now but testMapConcurrentAPIandContract could benefit from
being split into a number of `@Test` methods. These tests aren't parameterized.
test/jdk/java/util/stream/BuiltInGatherersTest.java line 272:
> 270: case 1 -> {
> 271: throwerReady.countDown();
> 272: awaitSensibly(initiateThrow);
20s might not be enough to wait for the count down when running with a debug
build, -Xcomp, ... I would be tempted to just have it wait indefinitely. If
there is an issue then jtreg will timeout the test.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16928#discussion_r1413031877
PR Review Comment: https://git.openjdk.org/jdk/pull/16928#discussion_r1413033710
