8333813: Seviceability tests fail due to stderr containing Temporarily processing option UseNotificationThread

[plummercj]

Allow warning messages such as the following to appear in stderr:

Java HotSpot(TM) 64-Bit Server VM warning: Temporarily processing option UseNotificationThread; support is scheduled for removal in 24.0

Tested by running CI tier5, which reproduced the issue.

---

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

Integration blocker

 ⚠️ Title mismatch between PR and JBS for issue JDK-8333813

Issue

• JDK-8333813: Serviceability tests fail due to stderr containing Temporarily processing option UseNotificationThread (Bug - P4) ⚠️ Title mismatch between PR and JBS. ⚠️ Issue is already resolved. Consider making this a "backport pull request" by setting the PR title to Backport <hash> with the hash of the original commit. See Backports.

Reviewers

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

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19606

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

Using diff file

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

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.

[plummercj]

/label serviceability

[openjdk]

@plummercj This change is no longer ready for integration - check the PR body for details.

[openjdk]

@plummercj
The serviceability label was successfully added.

[mlbridge]

Webrevs

• 00: Full (761b02c2)

[sspitsyn]

Looks good.

[dholmes-ora]

Maybe this should be renamed to reflect that it is about VM option warnings.

[plummercj]

Possibly. It means updating 15 tests. Also need to come up with a new name. Any suggestions?

[jaikiran]

Hello Chris, given these similary named methods in this class, perhaps we should instead just have one single stderrShouldBeEmptyIgnoring(...) method which takes the messages that should be ignored. Something like:

public static final String VM_WARNING_MSG = ".* VM warning:.*";

public static final String VM_WARNING_DEPRECATED_MSG = ".* VM warning:.* deprecated.*";
...

public OutputAnalyzer stderrShouldBeEmptyIgnoring(String ignoreMsg, String... additionalIgnoreMsgs) {
    String stdErrContent = getStderr().replaceAll(ignoreMsg + "\\R", "");
    if (additionalIgnoreMsgs != null) {
    	for (String additionalIgnore : additionalIgnoreMsgs) {
    		stdErrContent = stdErrContent.replaceAll(additionalIgnore + "\\R", "");
    	}
    }
    if (!stdErrContent.isEmpty()) {
    	reportDiagnosticSummary();
    	throw new RuntimeException("stderr was not empty");
    }
    return this;
}

We make those private fields in OutputAnalyzer public and have the caller pass them:

oa.stderrShouldBeEmptyIgnoring(OutputAnalyzer.VM_WARNING_DEPRECATED_MSG)

[plummercj]

That seems like a lot of busy work and over abstraction for trying to solve a rather trivial issue that already has a very simple solution, but maybe could use a better API name.

[jaikiran]

In that case, would stderrShouldBeEmptyIgnoreVMOptionDeprecations be OK?

[dholmes-ora]

This is the wrong fix. The tests should not be being run with the problematic option. We should not be hiding that by extending the "ignore warnings" support. This is an issue with our CI task definitions.