On Sat, 19 Apr 2025 05:19:22 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
> Remove the need for many nsk/jdi tests to discover the main thread, resulting
> in the test needing to be run with includevirtualthreads=y. Details in first
> comment.
>
> Tested with all tier2, tier3, and tier5 svc tests
Good to see more simplification of common code patterns.
See my small comment inline.
Also, you need to update a copyrights for changed files.
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.
-------------
Changes requested by lmesnik (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/24768#pullrequestreview-2782060944
PR Review Comment: https://git.openjdk.org/jdk/pull/24768#discussion_r2052889087
