RFR: 7901757: Race in counting total number of failures from TestNG [v2]

[Jaikiran Pai]

On Fri, 26 Jul 2024 10:36:12 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this change which proposes to fix the issue noted in https://bugs.openjdk.org/browse/CODETOOLS-7901757?
>> 
>> As noted there, the current implementation in jtreg's testng test listener doesn't take into account that the callback methods on that listener instance can be invoked concurrently for different threads. As a result, the count tracking logic in the listener ends up in a race condition which results in reporting incorrect numbers.
>> 
>> The commit in this PR fixes that issue and introduces a self test which reproduces the race issue and verifies the fix. The new test and existing tests continue to pass with this change.
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update src/share/classes/com/sun/javatest/regtest/agent/TestNGRunner.java
>   
>   Co-authored-by: Pavel Rappo <32523691+pavelrappo at users.noreply.github.com>

Hello Christian,

> With soon correct numbers being reported, will we see new errors in the OpenJDK (and other projects') test suites?

I don't expect the change in this PR to expose failures that have gone undetected so far, because the issue here is a race condition that could have incorrectly reported the failure count. For the failure to be not noticed previously, then it would mean that the race would have had to happen every single run of that test, which would be very odd.

In any case, in the next few days, I will include this change as part of the JDK tier testing I run as part of new jtreg changes and see how it goes.

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

PR Comment: https://git.openjdk.org/jtreg/pull/216#issuecomment-2291151467