spockframework
Fix usages of some async utilities
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?
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?
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 resourcesto 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?
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 https://github.com/junit-team/junit5/issues/3279#issuecomment-1531619135) but we might now be able to use jfr-polyfill.
@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?