8356811: Some nsk/jdi tests can fetch ThreadReference from static field in the debuggee: part 4

[plummercj]

This batch of changes mostly concerns the remaining uses of threadByName() and converting them to use threadByFieldNameOrThrow() or the new threadByFieldName(). The latter is used if the caller has code to handle a null result. The former is when an exception is needed to get the test to terminate properly. I did fix a few long standing cases where threadyByName() was being called and not checking the result. These call sites now use threadByFieldNameOrThrow() instead of threadByFieldName().

Note there is a minor semantic change in doing this. threadByName() has some extra code to check that the named thread is only found once, and will throw an exception if it is. I think this was just some extra checking that was being done during test development, and is not needed for proper test execution. I've run all the tests without this check and they still pass. I plan on removing this check at some point.

Tested by running all tier5 svc tests, which includes the nsk/jdi tests. Also ran tier1 and ran locally.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8356811: Some nsk/jdi tests can fetch ThreadReference from static field in the debuggee: part 4 (Enhancement - P4)

Reviewers

• Serguei Spitsyn (@sspitsyn - Reviewer)
• Alex Menkov (@alexmenkov - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25190/head:pull/25190
$ git checkout pull/25190

Update a local copy of the PR:
$ git checkout pull/25190
$ git pull https://git.openjdk.org/jdk.git pull/25190/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25190

View PR using the GUI difftool:
$ git pr show -t 25190

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25190.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back cjplummer! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@plummercj This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8356811: Some nsk/jdi tests can fetch ThreadReference from static field in the debuggee: part 4

Reviewed-by: sspitsyn, amenkov

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 148 new commits pushed to the master branch:

• 3dd3451: 8357075: Remove leftover COMPAT locale data tests
• 63ef90b: 8357084: Zero build fails after JDK-8354887
• d524509: 8356702: CTW: Update modules
• ... and 145 more: https://git.openjdk.org/jdk/compare/927309453822183bbfa9af278043fe0dca17031c...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@plummercj The following label will be automatically applied to this pull request:

• serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (f5d18f8a)
• 00: Full (6ab78d6d)

[alexmenkov]

Maybe just move this into try and replace debuggee.threadByFieldName with debuggee.threadByFieldNameOrThrow?

[plummercj]

@alexmenkov It looks like this could be done in almost every test in this PR. In some the try just needs to move about 8 lines up, but in others it's more like 20. There will be a lot of indentation diffs. Do you still want to see this change?

[plummercj]

It looks like there were some changes in part 3 that could also benefit from this. Perhaps this would be better off done in a followup PR. It will keep this PR easier to review, will make moving the try easier to review, and will pickup some additional tests that could use this.

[plummercj]

I filed JDK-8357172

[sspitsyn]

I also noticed this issue with exceptions but waited you to sort this out with Alex first.
I'm okay to separate it from this PR. Current PR looks good then.

[plummercj]

Thanks for the reviews Alex and Serguei!

/integrate

[openjdk]

Going to push as commit 10258dc.
Since your change was applied there have been 148 commits pushed to the master branch:

• 3dd3451: 8357075: Remove leftover COMPAT locale data tests
• 63ef90b: 8357084: Zero build fails after JDK-8354887
• d524509: 8356702: CTW: Update modules
• ... and 145 more: https://git.openjdk.org/jdk/compare/927309453822183bbfa9af278043fe0dca17031c...master

Your commit was automatically rebased without conflicts.

[openjdk]

@plummercj Pushed as commit 10258dc.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.