RFR: 8308038: java/util/concurrent/ThreadPerTaskExecutor/ThreadPerTaskExecutorTest.java timed out

[Jaikiran Pai]

On Mon, 22 May 2023 10:11:25 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> test/jdk/java/util/concurrent/ThreadPerTaskExecutor/ThreadPerTaskExecutorTest.java line 269:
>> 
>>> 267:         assertTrue(executor.awaitTermination(10, TimeUnit.MILLISECONDS));
>>> 268:         Throwable e = assertThrows(ExecutionException.class, future::get);
>>> 269:         assertTrue(e.getCause() instanceof InterruptedException);
>> 
>> Would using `assertEquals(InterruptedException.class, e.getCause().getClass())` be better? That way if/when the test fails, it even prints the unexpected exception type?
>> 
>> But I then see that this test already uses `assertTrue` for similar cases in some other place, so maybe it's fine in the current form?
>
> I don't mind moving this but it would require changing them everywhere (as you point out).

I think it's fine to leave it in the current form then. If/when any of these tests fail due to unexpected exception type in these asserts, we can then move them all to the other proposed (or some other) construct.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14072#discussion_r1200321915