Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8327054: DiagnosticCommand Compiler.perfmap does not log on output() #20257

Closed
wants to merge 4 commits into from
+18 −6
Closed

8327054: DiagnosticCommand Compiler.perfmap does not log on output() #20257

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
484f25e
8327054: DiagnosticCommand Compiler.perfmap does not log on output()
SoniaZaldana Jul 19, 2024
6129d87
Ensuring test case deletes file in case of exception
SoniaZaldana Jul 19, 2024
237f751
Adding comment
SoniaZaldana Jul 22, 2024
a2e4617
Updating warning message
SoniaZaldana Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions test/hotspot/jtreg/serviceability/dcmd/compiler/PerfMapTest.java
View file
Original file line number Diff line number Diff line change
@@ -118,9 +118,12 @@ public void specifiedDefaultMapFile() throws IOException {
public void logErrorsDcmdOutputStream() throws IOException {
String test_dir = System.getProperty("test.dir", ".");
Path path = Paths.get("nonexistent", test_dir);
OutputAnalyzer output = new JMXExecutor().execute("Compiler.perfmap %s".formatted(path));
output.shouldContain("Failed to create nonexistent/%s for perf map".formatted(test_dir));
output.shouldNotHaveExitValue(0);
Files.deleteIfExists(path);
try {
OutputAnalyzer output = new JMXExecutor().execute("Compiler.perfmap %s".formatted(path));
output.shouldContain("Failed to create nonexistent/%s for perf map".formatted(test_dir));
output.shouldNotHaveExitValue(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if this exit value check works, as jcmd failures like this show "Command executed successfully" and return 0 for success.
These compiler tests have chosen JMXExecutor and PidJcmdExecutor which might be relevant. Interested to know if JMXExecutor returns a non-zero exit value for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Kevin,

Yes, I noticed the test was exiting with a non-zero value when I was testing. After giving it a bit more thought, that check is not the main purpose of the test and I'm not entirely sure why the JMXExecutor behaves that way. I'll just remove the exit value check to avoid confusion.

Copy link
Contributor

@kevinjwalls kevinjwalls Jul 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was returning an "exit value" of -1, the test/lib/jdk/test/lib/process/OutputBuffer.java default.

JMXExecutor doesn't set one as there isn't an exit code... That could be clearer, actually there must be various issues in that area. But yes just don't check exit code for a JMX Executor, and jcmd would return zero but we don't want to embed that as a requirement, it's really a failure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was returning an "exit value" of -1, the test/lib/jdk/test/lib/process/OutputBuffer.java default.

Correct, that was the exit value.

But yes just don't check exit code for a JMX Executor, and jcmd would return zero but we don't want to embed that as a requirement, it's really a failure.

Agreed. I removed the exit status check.

} finally {
Files.deleteIfExists(path);
}
}
}