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.

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

Remove calls to test APIs unavailable in jdk17u #30

Conversation

swesonga
Copy link
Member

Some of the usage of the OutputAnalyzer in the TestAvailableProcessors class depends on APIs that are not in the jdk17u OutputAnalyzer. This PR cleans up these issues that prevent the class from compiling in jdk17u.

@brianjstafford
Copy link

How was this issue discovered?

@swesonga
Copy link
Member Author

How was this issue discovered?

The test failed in the release pipeline. I missed this in the backport because I mistakenly attributed this to a jtreg version issue.

@swesonga
Copy link
Member Author

The backport used the jdk21u APIs introduced in openjdk/jdk21u@262cacb#diff-13897cfb2b23f2a1424c9874bdb40f563b56cd01d2b560ace927fe240d15a5a9 and should have used the previous OutputAnalyzer.

@@ -115,7 +120,7 @@ private static boolean getSchedulesAllProcessorGroups(boolean isWindowsServer) t
private static OutputAnalyzer getAvailableProcessorsOutput(boolean productFlagEnabled) throws IOException {
String productFlag = productFlagEnabled ? "-XX:+UseAllWindowsProcessorGroups" : "-XX:-UseAllWindowsProcessorGroups";

ProcessBuilder processBuilder = ProcessTools.createLimitedTestJavaProcessBuilder(
ProcessBuilder processBuilder = ProcessTools.createJavaProcessBuilder(
Copy link

@brianjstafford brianjstafford Nov 1, 2024

Choose a reason for hiding this comment

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

Is this change also related to something that exists in 21 but isn't available in 17?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this change also related to something that exists in 21 but isn't available in 17?

Yes, jdk17u didn't have the fix for https://bugs.openjdk.org/browse/JDK-8315097 (although that is now being backported to jdk17u for the next release so we'll pick up that update automatically).

Copy link

@brianjstafford brianjstafford left a comment

Choose a reason for hiding this comment

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

LGTM

@swesonga swesonga merged commit 4babac7 into ms-patches/use-all-windows-processor-groups Nov 1, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants