8319673: Few security tests ignore VM flags

[sendaoYan]

Hi all,

This pull request contains a backport of commit 43029006 from the openjdk/jdk repository. Backport parity with 21.0.5-oracle, the change has been verified, no risk.

The commit being backported was authored by Matthew Donovan on 29 Feb 2024 and was reviewed by Weijun Wang and Hai-May Chao.

The new API Path resolve(String first, String... more) was not supported in jdk21u-dev, so I change the line code from COPY_JDK_DIR.resolve("bin", "java") to COPY_JDK_DIR.resolve("bin").resolve("java") to avoid jdk21 javac generate compile error, and make this backport uncleanly, all other parts are backported cleanly.

The change has been verified locally, test-fix only, no risk.

Thanks!

---

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
• JDK-8319673 needs maintainer approval

Issue

• JDK-8319673: Few security tests ignore VM flags (Sub-task - P4 - Approved)

Reviewers

• Lutz Schmidt (@RealLucy - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk21u-dev.git pull/830/head:pull/830
$ git checkout pull/830

Update a local copy of the PR:
$ git checkout pull/830
$ git pull https://git.openjdk.org/jdk21u-dev.git pull/830/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 830

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk21u-dev/pull/830.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back syan! 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]

@sendaoYan This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8319673: Few security tests ignore VM flags

Reviewed-by: lucy

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 17 new commits pushed to the master branch:

• bcc85d9: 8318442: java/net/httpclient/ManyRequests2.java fails intermittently on Linux
• 6affc09: 8340801: Disable ubsan checks in some awt/2d coding
• 1af737e: 8339892: Several security shell tests don't set TESTJAVAOPTS
• d0abff2: 8207908: JMXStatusTest.java fails assertion intermittently
• c5d75ef: 8337410: The makefiles should set problemlist and adjust timeout basing on the given VM flags
• d81d301: 8342181: Update tests to use stronger Key and Salt size
• 22d5e0d: 8343474: [updates] Customize README.md to specifics of update project
• 9bdf2a6: 8341927: Replace hardcoded security providers with new test.provider.name system property
• 7869713: 8335142: compiler/c1/TestTraceLinearScanLevel.java occasionally times out with -Xcomp
• 2702f67: 8333235: vmTestbase/nsk/jdb/kill/kill001/kill001.java fails with C1
• ... and 7 more: https://git.openjdk.org/jdk21u-dev/compare/4a5c578bb092cde2c1cb40a451de6be63a9026f4...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

This backport pull request has now been updated with issue from the original commit.

[openjdk]

⚠️ @sendaoYan This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

[mlbridge]

Webrevs

• 02: Full - Incremental (29167217)
• 01: Full - Incremental (e479a477)
• 00: Full (199b8d3b)

[bridgekeeper]

@sendaoYan This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 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!

[sendaoYan]

wait

[bridgekeeper]

@sendaoYan This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 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!

[sendaoYan]

wait

[GoeLin]

Hi @sendaoYan, what's up with this backport?

[sendaoYan]

Hi @sendaoYan, what's up with this backport?

I think I just mistake the PR status. I make this PR as ready now.

[sendaoYan]

I will verify the jtreg tests again then create approval request later.

[sendaoYan]

This backport will generate below javac compiler error, caused by new API Path resolve(String first, String... more) was not supported in jdk21u-dev
Should we integrate this PR first, and then create a new PR to fix the compiler error.

/tmp/tone/run/jtreg/jdk-repo/test/jdk/java/security/Security/ConfigFileTest.java:57: error: no suitable method found for resolve(String,String)
    private static Path COPIED_JAVA = COPY_JDK_DIR.resolve("bin", "java");

[sendaoYan]

GHA report install some dependencies failure in macos-aarch64, it seems like envirnmental issue, it's unrelrated to this PR.

[sendaoYan]

Hi @sendaoYan, what's up with this backport?

This backport will cause test java/security/Security/ConfigFileTest.java compile error, because new API Path resolve(String first, String... more) was not supported in jdk21u-dev.
Should I integrate this backport, and then create a new PR to fix the error right away.

[GoeLin]

Hi @sendaoYan,
this is a normal thing happening with backports. We don't file extra bugs for this.
If it was clean before, it is best to push an extra commit on top of the original backport, explain in detail in the PR what and why it was edited, and include a short remark in the approval comment that it had to be adapted to 21. (This hints the person backporting to 17 to base the backport on the 21 commit).

[sendaoYan]

Hi @sendaoYan, this is a normal thing happening with backports. We don't file extra bugs for this. If it was clean before, it is best to push an extra commit on top of the original backport, explain in detail in the PR what and why it was edited, and include a short remark in the approval comment that it had to be adapted to 21. (This hints the person backporting to 17 to base the backport on the 21 commit).

Thanks for the detail advice. I will create a new PR to backport plus fix the compile error.

[sendaoYan]

Thanks for the detail advice. I will create a new PR to backport plus fix the compile error.

The new PR #1142 has been created, close this as duplicated.

[GoeLin]

Hi, you don't need a new pr, just push a fix to this branch. The fix is simple enough, you can just edit it in the web. Locally, you can check out origin/pr/830 and double-check on it if needed.

[sendaoYan]

Hi, you don't need a new pr, just push a fix to this branch. The fix is simple enough, you can just edit it in the web. Locally, you can check out origin/pr/830 and double-check on it if needed.

Thanks for your patient advising. This PR has been updated.

[GoeLin]

GHA failure of latest run: infra issue, unrelated
error: invalid developer directory '/Applications/Xcode_14.3.1.app/Contents/Developer'

[RealLucy]

LGTM.

[sendaoYan]

LGTM.

Thanks for the review.

/approval request Backport to make several tests receive VM options from jtreg. The change has been verified locally, test-fix only, no risk.

[openjdk]

@sendaoYan
8319673: The approval request has been created successfully.

[sendaoYan]

Thanks all for the review and approved.

/integrate

[openjdk]

Going to push as commit fa07b3d.
Since your change was applied there have been 17 commits pushed to the master branch:

• bcc85d9: 8318442: java/net/httpclient/ManyRequests2.java fails intermittently on Linux
• 6affc09: 8340801: Disable ubsan checks in some awt/2d coding
• 1af737e: 8339892: Several security shell tests don't set TESTJAVAOPTS
• d0abff2: 8207908: JMXStatusTest.java fails assertion intermittently
• c5d75ef: 8337410: The makefiles should set problemlist and adjust timeout basing on the given VM flags
• d81d301: 8342181: Update tests to use stronger Key and Salt size
• 22d5e0d: 8343474: [updates] Customize README.md to specifics of update project
• 9bdf2a6: 8341927: Replace hardcoded security providers with new test.provider.name system property
• 7869713: 8335142: compiler/c1/TestTraceLinearScanLevel.java occasionally times out with -Xcomp
• 2702f67: 8333235: vmTestbase/nsk/jdb/kill/kill001/kill001.java fails with C1
• ... and 7 more: https://git.openjdk.org/jdk21u-dev/compare/4a5c578bb092cde2c1cb40a451de6be63a9026f4...master

Your commit was automatically rebased without conflicts.

[openjdk]

@sendaoYan Pushed as commit fa07b3d.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.