jtreg icon indicating copy to clipboard operation
jtreg copied to clipboard

7903193: [jtreg] build and test failures using JDK 18

Open jaikiran opened this issue 1 year ago • 16 comments

Can I please get a review of this test only changes, which proposes to address the current failure in jtreg self tests when Java 18 or higher is used to build and test jtreg?

As noted in the following issues: https://bugs.openjdk.org/browse/CODETOOLS-7903193 https://bugs.openjdk.org/browse/CODETOOLS-7903646 https://bugs.openjdk.org/browse/CODETOOLS-7903645

these self tests in jtreg which rely on SecurityManager, no longer pass when used with Java 18 or higher, since starting Java 18 the setting of SecurityManager throws an UnsupportedOperationException.

Changes in this PR, include updates to test files which check for the Java version being used to run these tests and then decide whether or not to include some specific tests that only pass when a SecurityManager is set.

I've run these changes locally (on macos M1) and on a linux setup, both with Java 17 and Java 21. The tests all pass on these versions.

I've also run this on a headless system to make sure the ReportOnlyTest.gmk does indeed properly check the correct values on a headless system (both Java 17 and 21). I think this change should address the issue that Ludvig @LudwikJaniuk had run into.


Progress

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

Issues

  • CODETOOLS-7903193: [jtreg] build and test failures using JDK 18 (Bug - P3)
  • CODETOOLS-7903645: tests that set SecurityManager fail due to UnsupportedOperationException in Java 21 (Bug - P4)
  • CODETOOLS-7903646: test/BasicAgent.vm fails when run with Java 21 (Bug - P3)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 190

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jtreg/pull/190.diff

Webrev

Link to Webrev Comment

jaikiran avatar Mar 10 '24 01:03 jaikiran

/issue 7903645

jaikiran avatar Mar 10 '24 01:03 jaikiran

:wave: Welcome back jpai! 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.

bridgekeeper[bot] avatar Mar 10 '24 01:03 bridgekeeper[bot]

@jaikiran Adding additional issue to issue list: 7903645: tests that set SecurityManager fail due to UnsupportedOperationException in Java 21.

openjdk[bot] avatar Mar 10 '24 01:03 openjdk[bot]

/issue 7903646

jaikiran avatar Mar 10 '24 01:03 jaikiran

@jaikiran Adding additional issue to issue list: 7903646: test/BasicAgent.vm fails when run with Java 21.

openjdk[bot] avatar Mar 10 '24 01:03 openjdk[bot]

Webrevs

mlbridge[bot] avatar Mar 10 '24 01:03 mlbridge[bot]

With your changes, I get All (196) selected tests completed successfully after running the test make target. So it seems to work for me!

LudwikJaniuk avatar Mar 11 '24 12:03 LudwikJaniuk

Thank you Ludvig for testing this. I'm happy to hear that it now passes.

jaikiran avatar Mar 11 '24 12:03 jaikiran

❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.

openjdk[bot] avatar Mar 13 '24 20:03 openjdk[bot]

@jaikiran This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Apr 17 '24 00:04 bridgekeeper[bot]

@jaikiran This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

bridgekeeper[bot] avatar May 15 '24 05:05 bridgekeeper[bot]

/open

jaikiran avatar Aug 24 '24 07:08 jaikiran

@jaikiran This pull request is now open

openjdk[bot] avatar Aug 24 '24 07:08 openjdk[bot]

I've now updated the PR to only contain the Java 18+ changes. I have verified the latest changes against Java versions less than 18 as well as greater than 18, both when headless is true and when headless is false. All self tests continue to pass and jtreg builds successfully in all these combinations.

jaikiran avatar Aug 24 '24 07:08 jaikiran

Heads-up for after https://github.com/openjdk/jtreg/pull/217 has been integrated, the basic test number counters need an update, again.

sormuras avatar Sep 20 '24 05:09 sormuras

I've now updated the PR after merging against latest master. I have verified that with these changes, all self tests continue to pass with Java versions less than 18 as well as greater than 18, both when headless is true and when headless is false.

jaikiran avatar Sep 22 '24 10:09 jaikiran