On Mon, 21 Apr 2025 19:20:35 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> Chris Plummer has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add new settingBreakpoint() API for when thread is null
>
> test/hotspot/jtreg/vmTestbase/nsk/share/jdi/JDIBase.java line 122:
>
>> 120: breakpRequest =
>> eventRManager.createBreakpointRequest(lineLocation);
>> 121: breakpRequest.putProperty("number", property);
>> 122: if (thread != null) {
>
> The change might hide failure if thread is not set in the test.
> I would prefer to have
> private settingBreakpoint(ThreadReference thread,.,.) that allows null
> and
>
> protected final BreakpointRequest settingBreakpoint(ThreadReference thread,
> ReferenceType
> testedClass,
> String methodName,
> String bpLine,
> String property)
>
> that still fails early if thread is null. (I think now it should fail in
> `
> breakpRequest.addThreadFilter(thread);
> `
> string.
> So for any test that don't have proper thread - test fails early when setting
> breakpoint and not because it hasn't find it.
Ok. I updated to use a different API that implies no thread.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24768#discussion_r2052929415
