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

Excluding ResetPeakMemoryUsage on openj9 #2328

Merged
merged 2 commits into from
Mar 17, 2021
Merged

Excluding ResetPeakMemoryUsage on openj9 #2328

merged 2 commits into from
Mar 17, 2021

Conversation

psoujany
Copy link
Contributor

@psoujany psoujany commented Mar 2, 2021

This PR is to exclude hotspot related jdk_management test on openj9.
java/lang/management/MemoryMXBean/ResetPeakMemoryUsage.java - exclusion on openj9 for jdk11

@smlambert smlambert requested a review from llxia March 2, 2021 15:06
@smlambert
Copy link
Contributor

smlambert commented Mar 2, 2021

Thanks @psoujany, some initial questions:

  • is this specific to jdk11 (if applicable also to jdk16+, should also update those files)?
  • is there an open issue (at openj9 project) that can be referenced (as in future we will have automation to verify if issue is closed, and can re-include test. If the URL is unreachable/internal, automation will not work)?

@llxia
Copy link
Contributor

llxia commented Mar 2, 2021

Is ResetPeakMemoryUsage a part of jdk_management test? This test target is excluded for openj9 https://github.com/AdoptOpenJDK/openjdk-tests/blob/master/openjdk/playlist.xml#L753 as part of eclipse-openj9/openj9#10757

Do we have a plan to re-enable jdk_management after this PR? If not, maybe we do not need the problem list exclusion.

@psoujany
Copy link
Contributor Author

psoujany commented Mar 3, 2021

Hi @ShelleyLambert

  1. This test exclusion is specific to openj9. So, yes this change would be applicable to other releases as well. I'll make changes to releases.
  2. Yes, there's an open issue which is raised in internal repo i.e, openj9-openjdk-jdk11-zos/issues/454.

Thank you.

@psoujany
Copy link
Contributor Author

psoujany commented Mar 3, 2021

Hi @llxia
Yes, ResetPeakMemoryUsage is a part of jdk_management test. We don't have any plan to re-enable jdk_management after this PR. There's an open issue for this testcase in openj9-openjdk-jdk11-zos/issues/454, as part of fixing the issue we need this test exclusion.

This testcase is also not supported on openj9, so even in future enabling jdk_management this testcase would fail. Hence exclusion would be valid fix.

@smlambert
Copy link
Contributor

smlambert commented Mar 3, 2021

This testcase is also not supported on openj9, so even in future enabling jdk_management this testcase would fail. Hence exclusion would be valid fix.

So it sounds like in this case that this is a permanent exclude (the test case is not valid for openj9). For permanent excludes, then please change the URL from the internal one (runtimes/openj9-openjdk-jdk11-zos/issues/454) that we have no visibility of to https://github.com/AdoptOpenJDK/openjdk-tests/issues/1297.

I see the confusion, when I referred to 'open issue' I meant an issue in an open-source repo (not an internal repo) that is visible to everyone (and can be queried by automation we will run from the open-source projects). :)

@@ -133,7 +133,7 @@ sun/management/HotspotRuntimeMBean/GetSafepointCount.java https://github.com/Ado
sun/management/HotspotRuntimeMBean/GetSafepointSyncTime.java https://github.com/AdoptOpenJDK/openjdk-tests/issues/932 generic-all
sun/management/HotspotRuntimeMBean/GetTotalSafepointTime.java https://github.com/AdoptOpenJDK/openjdk-tests/issues/932 generic-all
sun/management/HotspotThreadMBean/GetInternalThreads.java https://github.com/AdoptOpenJDK/openjdk-tests/issues/932 generic-all

java/lang/management/MemoryMXBean/ResetPeakMemoryUsage.java https://github.ibm.com/runtimes/openj9-openjdk-jdk11-zos/issues/454 generic-all
Copy link
Contributor

@smlambert smlambert Mar 4, 2021

Choose a reason for hiding this comment

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

@psoujany - please use https://github.com/AdoptOpenJDK/openjdk-tests/issues/1297 instead of the https://github.ibm.com/runtimes/openj9-openjdk-jdk11-zos/issues/454 internal issue

@psoujany
Copy link
Contributor Author

psoujany commented Mar 5, 2021

Hi, @ShelleyLambert. I applied change as you mentioned to all releases i.e, jdk11+. Please review the changes. Thank you.

Copy link
Contributor

@smlambert smlambert left a comment

Choose a reason for hiding this comment

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

thanks @psoujany

@karianna karianna added this to the March 2021 milestone Mar 9, 2021
@smlambert
Copy link
Contributor

@llxia - anything further updates you would like to see on this PR, or are we good ?

Copy link
Contributor

@llxia llxia left a comment

Choose a reason for hiding this comment

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

LGTM

@llxia llxia merged commit 84224b4 into adoptium:master Mar 17, 2021
@psoujany psoujany deleted the jtreg-exclude branch May 25, 2021 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants