RFR: 7903753: Report duration of testng test methods [v2]

[Christian Stein]

On Fri, 14 Jun 2024 09:00:07 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> src/share/classes/com/sun/javatest/regtest/agent/TestNGRunner.java line 118:
>> 
>>> 116:         // keeps track of the test start time for each test
>>> 117:         private final Map<ITestResult, Long> startTimeNanos =
>>> 118:                 Collections.synchronizedMap(new IdentityHashMap<>());
>> 
>> Why didn't you use `java.util.concurrent.ConcurrentHashMap` here?
>
> Hello Christian, we want the equality check of the key to be based on identity (since I don't think we can rely on the `equals()` implementation of `ITestResult`). ConcurrentHashMap uses `equals()` check unlike `IdentityHashMap` which uses `==`.
> 
> In the case of junit, where you used a ConcurrentHashMap, i think it's fine since the tests in junit appear to have a `UniqueId` unlike here where I don't see that construct exposed in testng.

Understood.

Meaning that in this case we rely on TestNG re-using the exact same object instances for `onTestStart()` and `report()` methods.

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

PR Review Comment: https://git.openjdk.org/jtreg/pull/207#discussion_r1639531300