On Fri, 7 Apr 2023 07:38:31 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> Alan Bateman has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains 11 additional 
>> commits since the last revision:
>> 
>>  - Test/comments updates
>>  - Merge
>>  - Expand tests for jdk.ThreadSleep event
>>  - Review feedback
>>  - Merge
>>  - Fix ThreadSleepEvent again
>>  - Test updates
>>  - ThreadSleepEvent refactoring
>>  - Merge
>>  - Merge
>>  - ... and 1 more: https://git.openjdk.org/jdk/compare/49869acd...a5bb3fd9
>
> test/jdk/java/lang/Thread/virtual/TraceVirtualThreadLocals.java line 52:
> 
>> 50:             name.get();
>> 51:         });
>> 52:         assertContains(output, "java.lang.ThreadLocal.get");
> 
> Should it also assert the presence of `ThreadLocal.setInitialValue` in the 
> stacktrace?

It's the `get` method that triggers the initial value to be generated so this 
is why the test checks that method. But I think you have a point it would be 
clearer if the the test were to match on something that has "initialValue" in 
the string. It means using the name of an internal method but it might be okay 
here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1160548887

Reply via email to