On Sun, 21 May 2023 13:52:06 GMT, Alan Bateman <[email protected]> wrote:
> This is a test only change to the unit test for the ExecutorService returned
> by Executors.newThreadPerTaskExecutor. The tests for interrupting invokeAll
> assume the threads started to execute the tasks do actually execute the task
> code. The refresh in JEP 444 changed the implementation to use FutureTask,
> and FutureTask checks the interrupt status before it executes the task code.
> So some intermittent timeouts of the tests for interrupting invokeAll as
> those tests were waiting for the task to complete.
>
> The main change is that the tests for interrupting invokeAll are changed to
> interrupt when the main thread blocks in invokeAll. They are also changed to
> check if the task started or not. The tests for interrupting invokeAny
> already did this, but these are changed to use the same infrastructure to
> avoid having two styles of tests in the same source file.
Looks reasonable. Just some question about testing invokeAll/invokeAny when
there is only one task.
test/jdk/java/util/concurrent/ThreadPerTaskExecutor/ThreadPerTaskExecutorTest.java
line 578:
> 576: try {
> 577:
> scheduleInterruptAt("java.util.concurrent.ThreadPerTaskExecutor.invokeAny");
> 578: executor.invokeAny(Set.of(task));
Seems strange to test invokeAny when there is only one task?
test/jdk/java/util/concurrent/ThreadPerTaskExecutor/ThreadPerTaskExecutorTest.java
line 835:
> 833: try {
> 834:
> scheduleInterruptAt("java.util.concurrent.ThreadPerTaskExecutor.invokeAll");
> 835: executor.invokeAll(Set.of(task));
same remark for invokeAll?
-------------
PR Review: https://git.openjdk.org/jdk/pull/14072#pullrequestreview-1436244152
PR Review Comment: https://git.openjdk.org/jdk/pull/14072#discussion_r1200271736
PR Review Comment: https://git.openjdk.org/jdk/pull/14072#discussion_r1200272685
