Fix usages of some async utilities

[Vampire]

Fixes #1600

[Vampire]

Hmmm, the tests failing now were previously flaky at best.

They either were successful because the code behaved properly,
or they were successful because the timeout for the latch was short enough.

E.g. for ParallelSpec#tests can get exclusive access to resources to be meaningful without the assertion I added, you would need a very high timeout value where it practically can never happen that the code misbehaves but the test is green due to the timeout passing fast enough. 100 ms as timeout is surely not matching that constraint.
But if you e. g. configure 10 seconds as timeout, then you actually have to wait those 10 seconds for the test to complete successfully and only a failure would be faster.

@leonard84 do you have any idea how to better properly test this, without depending on "high enough" timeouts?

[Vampire]

Maybe we should not really test the concurrency but trust that JUnit Platform does the right thing and instead just test that the annotations set the proper exclusive resources on the feature infos?

[leonard84]

Why is this necessary?

[Vampire]

If it indeed happens that the watcher thread is interrupted during the initial wait, it should continue the initial wait, shouldn't it?
Or should it really forward the interrupting to the Feature thread, just because it got interrupted for whatever reason?

[leonard84]

Using join to make it un-interruptible?

[Vampire]

It does not get uninterruptible.
It just does not throw checked exceptions but unchecked ones.
If you use get() you might get InterruptedException and then have to handle it properly.
Just ignoring is not ok at that spot as this is not a self-managed thread, so in most cases you should either rethrow or at least re-interrupt so that code higher in the stack can properly handle the interrupt and so on.
And by using join() here you can simply get around all that, as you don't mess with the interrupted exception yourself.

And ExecutionException would never come here anyway, as the future is never completed exceptionally.

[leonard84]

They either were successful because the code behaved properly, or they were successful because the timeout for the latch was short enough.

E.g. for ParallelSpec#tests can get exclusive access to resources to be meaningful without the assertion I added, you would need a very high timeout value where it practically can never happen that the code misbehaves but the test is green due to the timeout passing fast enough. 100 ms as timeout is surely not matching that constraint. But if you e. g. configure 10 seconds as timeout, then you actually have to wait those 10 seconds for the test to complete successfully and only a failure would be faster.

@leonard84 do you have any idea how to better properly test this, without depending on "high enough" timeouts?

Just increasing the timeout to 10s doesn't fix the tests, we'd actually need a way to get access to the lock.

Some time ago, I advocated that the JUnit Platform integrates JFR events, to enable low tracing of parallel executions. Unfortunately, that didn't gain any traction with the sentiment being that the listener->jfr events was good enough.

Maybe this would be a good showcase why it would be good to have those low-level events. If the platform would emit events for locking/unlocking of exclusive resources, we could make assertions upon those events instead of not knowing why if it was ok, because the code worked properly or because there was just a single thread for some reason. @marcphilipp WDYT?

[marcphilipp]

Maybe this would be a good showcase why it would be good to have those low-level events. If the platform would emit events for locking/unlocking of exclusive resources, we could make assertions upon those events instead of not knowing why if it was ok, because the code worked properly or because there was just a single thread for some reason.

I'm not opposed to it in general. The main concern was breaking users' builds. Apparently there are still JDKs that don't include the JFR module (see junit-team/junit5#3279 (comment)) but we might now be able to use jfr-polyfill.

[Vampire]

Any news on that @marcphilipp?

[Vampire]

@AndreasTu before you try again to fix flaky async tests, especially by introducing insanely high timeouts, please be aware of this PR and the comments.

Maybe you have some additional ideas?