Skip to content

8372831: [lworld] com/sun/jdi/EATests.java#id0 fails with --enable-preview#2313

Open
plummercj wants to merge 3 commits intoopenjdk:lworldfrom
plummercj:8372831_eatests
Open

8372831: [lworld] com/sun/jdi/EATests.java#id0 fails with --enable-preview#2313
plummercj wants to merge 3 commits intoopenjdk:lworldfrom
plummercj:8372831_eatests

Conversation

@plummercj
Copy link
Copy Markdown
Contributor

@plummercj plummercj commented Apr 8, 2026

Don't run the EARelockingValueBased subtest when using --enable-preview.

The switch from driver to main is needed in order to get the test to run with preview enabled when it is specified.

When value types move out of preview, the EARelockingValueBased test will need to be removed.


Progress

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

Error

 ⚠️ Pull request body is missing required line: - [x] I confirm that I make this contribution in accordance with the [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).

Issue

  • JDK-8372831: [lworld] com/sun/jdi/EATests.java#id0 fails with --enable-preview (Bug - P4)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2313

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2313.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link
Copy Markdown

bridgekeeper bot commented Apr 8, 2026

👋 Welcome back cjplummer! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link
Copy Markdown

openjdk bot commented Apr 8, 2026

@plummercj This change is no longer ready for integration - check the PR body for details.

@openjdk openjdk bot changed the title 8372831 8372831: [lworld] com/sun/jdi/EATests.java#id0 fails with --enable-preview Apr 8, 2026
@plummercj plummercj marked this pull request as ready for review April 8, 2026 19:48
@openjdk openjdk bot added the rfr Pull request is ready for review label Apr 8, 2026
@mlbridge
Copy link
Copy Markdown

mlbridge bot commented Apr 8, 2026

Webrevs

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2025 SAP SE. All rights reserved.
* Copyright (c) 2020, 2026 SAP SE. All rights reserved.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unless the changes were contributed by SAP, I think we need an Oracle copyright line here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Thanks.

Copy link
Copy Markdown
Member

@Arraying Arraying left a comment

Choose a reason for hiding this comment

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

Except for the pending copyright comment, the test changes look fine. Thanks for investigating and fixing!

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Apr 9, 2026
@plummercj
Copy link
Copy Markdown
Contributor Author

@dean-long @Arraying For anyone reviewing this PR, please also look at openjdk/jdk#30560. There are a few ways of addressing this issue and there isn't consensus yet on which way to go about it. openjdk/jdk#30560 (comment) is a good summary of the 4 possible ways I can envision addressing this issue. They all have different pros and cons.

@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants