On Thu, 13 Mar 2025 19:35:27 GMT, Thomas Stuefe <[email protected]> wrote:
>> Jiangli Zhou has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Address @tstuefe review:
>> - Renamed shouldMatchUnconditionally_<os>_libjvm to
>> shouldMatch_<os>_libjvm in SystemMapTestBase.
>> - Added comments to shouldMatchUnconditionally() methods for difference OS
>> cases in SystemMapTestBase.
>> - Added comments to DynLibsTest.java.
>
> test/hotspot/jtreg/serviceability/dcmd/vm/SystemMapTestBase.java line 97:
>
>> 95: regexBase_committed + "/lib/.*/libjvm.so"
>> 96: };
>> 97:
>
> Nit, misnamed, since its not unconditionally anymore.
Done. Also renamed for other OSs.
> test/hotspot/jtreg/serviceability/dcmd/vm/SystemMapTestBase.java line 115:
>
>> 113: } else {
>> 114: return
>> StringArrayUtils.concat(shouldMatchUnconditionally_linux,
>> 115:
>> shouldMatchUnconditionally_linux_libjvm);
>
> Here, and in DumpTest: please provide a short comment about why we don't want
> to match libjvm (even if it seems obvious).
Add comments. Please let me know if that covers all cases in your suggestion.
@tstuefe Could you please approve again after the updates? Thanks for the
review!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23734#discussion_r1994389318
PR Review Comment: https://git.openjdk.org/jdk/pull/23734#discussion_r1994390355
