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

jaikiran · 2024-03-10T01:54:33Z

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)
Change must not contain extraneous whitespace
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

Christian Stein (@sormuras - Committer)

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 · 2024-03-10T01:55:00Z

/issue 7903645

bridgekeeper · 2024-03-10T01:55:00Z

👋 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.

openjdk · 2024-03-10T01:55:29Z

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

jaikiran · 2024-03-10T01:55:56Z

/issue 7903646

openjdk · 2024-03-10T01:56:22Z

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

mlbridge · 2024-03-10T01:59:11Z

Webrevs

LudwikJaniuk · 2024-03-11T12:24:01Z

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

jaikiran · 2024-03-11T12:52:24Z

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

openjdk · 2024-03-13T20:06:38Z

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

jonathan-gibbons

It would be better to come up with a better way for jtreg to deal with the lack of a security manager.

That being, said, we obviously are using jtreg to run on recent versions of JDK, and this PR is just about fixing the tests, not about changing jtreg functionality itself, at this time.

jonathan-gibbons · 2024-03-19T19:43:09Z

make/build.sh

@@ -784,6 +785,7 @@ make ASMTOOLS_JAR="${ASMTOOLS_JAR}"                           \\
     JUNIT_NOTICES="${JUNIT_NOTICES}"                         \\
     TESTNG_JARS="${TESTNG_JARS}"                             \\
     TESTNG_NOTICES="${TESTNG_NOTICES}"                       \\
+     JAVA_SPECIFICATION_VERSION="${JAVA_SPECIFICATION_VERSION}" \\


Note the list of variables is sorted alphabetically.

jonathan-gibbons · 2024-03-19T19:47:52Z

test/basic/ReportOnlyTest.gmk

@@ -40,6 +35,7 @@ $(BUILDTESTDIR)/ReportOnlyTest.ok: $(BUILDTESTDIR)/Basic.othervm.ok \
 		   $(JTREG_IMAGEDIR)/bin/jtreg
 	JAVA_HOME=$(JDKHOME) CLASSPATH=$(ABSCLASSDIR) \
 	    $(JTREG_IMAGEDIR)/bin/jtreg $(JTREG_OPTS) \
+	    -k:!needDisplay \


This seems unrelated to the security manager changes.

Sorry Jon, I forgot to reply to this comment. I'll refresh this PR against latest master branch and refresh my memory about this change and respond shortly.

With the other PR #223 now integrated, this part of the change in this PR is no longer needed. So I've removed it in the latest updates to this PR.

bridgekeeper · 2024-04-17T00:25:42Z

@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 · 2024-05-15T05:12:12Z

@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.

jaikiran · 2024-08-24T07:03:29Z

/open

openjdk · 2024-08-24T07:03:37Z

@jaikiran This pull request is now open

jaikiran · 2024-08-24T07:06:12Z

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.

…n build script

sormuras · 2024-09-20T05:27:23Z

Heads-up for after #217 has been integrated, the basic test number counters need an update, again.

jaikiran · 2024-09-22T10:58:50Z

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.

sormuras

Looks good to me - @jonathan-gibbons do you agree?

sormuras · 2024-10-17T13:27:22Z

@jaikiran Can you please replace the 17 with a 21 in https://github.com/jaikiran/jtreg/blob/60809c4da0f2ef18540df4328113f898d032497b/.github/workflows/test.yml#L24

That should make a) the test workflow download an Oracle JDK and b) also run the build (compilation and tests) on a JDK 18+.

Background: https://www.oracle.com/java/technologies/jdk-script-friendly-urls/

JDK 17 LTS and GraalVM for JDK 17 LTS latest URLs are no longer available. They were available for three years after GA and one year after the next JDK LTS release. They stopped working on October of 2024. Java SE Subscribers and customers using Oracle Cloud Infrastructure can continue getting updates for JDK 17 and GraalVM for JDK 17; other users should migrate to the next LTS.

The archive URLs for each release will continue to work for at least a year after the corresponding latest URLs stop being available.

Although the latest URLs for JDK 17 are no longer available, the archive URLs for JDK 17 LTS releases 17.0.12 and earlier continue to work until October of 2025; one year after the the corresponding latest URLs were disabled.

jaikiran · 2024-10-17T13:33:02Z

@jaikiran Can you please replace the 17 with a 21 in https://github.com/jaikiran/jtreg/blob/60809c4da0f2ef18540df4328113f898d032497b/.github/workflows/test.yml#L24

That should make a) the test workflow download an Oracle JDK and b) also run the build (compilation and tests) on a JDK 18+.

I wasn't sure if we should do that change as part of this PR, so I created a separate branch which includes the changes in this PR plus the change to the JDK version in the github workflow file. That branch's job is currently in progress here https://github.com/jaikiran/jtreg/actions/runs/11385998691/job/31677151971

jaikiran · 2024-10-17T13:34:43Z

If that build succeeds and if you think that we should include that change in this PR, let me know and I'll update this PR to include that change.

sormuras · 2024-10-17T13:38:55Z

If that build succeeds and if you think that we should include that change in this PR, let me know and I'll update this PR to include that change.

Sounds good to me.

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

da76c5e

openjdk bot added the rfr Pull request is ready for review label Mar 10, 2024

jonathan-gibbons reviewed Mar 19, 2024

View reviewed changes

bridgekeeper bot closed this May 15, 2024

jaikiran mentioned this pull request Aug 20, 2024

7903792: ReportOnlyTest may fail in "headless" environments #223

Closed

3 tasks

jaikiran added 3 commits August 24, 2024 12:27

merge latest from master branch

d53f2fa

consider java version >= 18

8b05649

better code comment

0aa1776

openjdk bot reopened this Aug 24, 2024

jaikiran added 2 commits August 24, 2024 21:08

Jon's review - maintain alphabetical order of environment variables i…

5efd255

…n build script

merge latest from master branch

52626eb

jaikiran added 2 commits September 22, 2024 13:33

merge latest from master branch

e992250

Fix test pass/fail count checks

db980db

sormuras approved these changes Sep 23, 2024

View reviewed changes

merge latest from master

60809c4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jaikiran commented Mar 10, 2024 •

edited by openjdk bot

Loading

jaikiran commented Mar 10, 2024

bridgekeeper bot commented Mar 10, 2024

openjdk bot commented Mar 10, 2024

jaikiran commented Mar 10, 2024

openjdk bot commented Mar 10, 2024

mlbridge bot commented Mar 10, 2024 •

edited

Loading

LudwikJaniuk commented Mar 11, 2024

jaikiran commented Mar 11, 2024

openjdk bot commented Mar 13, 2024

jonathan-gibbons left a comment •

edited by bridgekeeper bot

Loading

jonathan-gibbons Mar 19, 2024 •

edited by bridgekeeper bot

Loading

jonathan-gibbons Mar 19, 2024 •

edited by bridgekeeper bot

Loading

jaikiran May 15, 2024

jaikiran Aug 24, 2024

bridgekeeper bot commented Apr 17, 2024

bridgekeeper bot commented May 15, 2024

jaikiran commented Aug 24, 2024

openjdk bot commented Aug 24, 2024

jaikiran commented Aug 24, 2024

sormuras commented Sep 20, 2024

jaikiran commented Sep 22, 2024

sormuras left a comment

sormuras commented Oct 17, 2024 •

edited

Loading

jaikiran commented Oct 17, 2024

jaikiran commented Oct 17, 2024

sormuras commented Oct 17, 2024

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

Are you sure you want to change the base?

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

Conversation

jaikiran commented Mar 10, 2024 • edited by openjdk bot Loading

Progress

Issues

Reviewers

Reviewing

Webrev

jaikiran commented Mar 10, 2024

bridgekeeper bot commented Mar 10, 2024

openjdk bot commented Mar 10, 2024

jaikiran commented Mar 10, 2024

openjdk bot commented Mar 10, 2024

mlbridge bot commented Mar 10, 2024 • edited Loading

Webrevs

LudwikJaniuk commented Mar 11, 2024

jaikiran commented Mar 11, 2024

openjdk bot commented Mar 13, 2024

jonathan-gibbons left a comment • edited by bridgekeeper bot Loading

Choose a reason for hiding this comment

jonathan-gibbons Mar 19, 2024 • edited by bridgekeeper bot Loading

Choose a reason for hiding this comment

jonathan-gibbons Mar 19, 2024 • edited by bridgekeeper bot Loading

Choose a reason for hiding this comment

jaikiran May 15, 2024

Choose a reason for hiding this comment

jaikiran Aug 24, 2024

Choose a reason for hiding this comment

bridgekeeper bot commented Apr 17, 2024

bridgekeeper bot commented May 15, 2024

jaikiran commented Aug 24, 2024

openjdk bot commented Aug 24, 2024

jaikiran commented Aug 24, 2024

sormuras commented Sep 20, 2024

jaikiran commented Sep 22, 2024

sormuras left a comment

Choose a reason for hiding this comment

sormuras commented Oct 17, 2024 • edited Loading

jaikiran commented Oct 17, 2024

jaikiran commented Oct 17, 2024

sormuras commented Oct 17, 2024

jaikiran commented Mar 10, 2024 •

edited by openjdk bot

Loading

mlbridge bot commented Mar 10, 2024 •

edited

Loading

jonathan-gibbons left a comment •

edited by bridgekeeper bot

Loading

jonathan-gibbons Mar 19, 2024 •

edited by bridgekeeper bot

Loading

jonathan-gibbons Mar 19, 2024 •

edited by bridgekeeper bot

Loading

sormuras commented Oct 17, 2024 •

edited

Loading