On Mon, 15 Dec 2025 19:22:29 GMT, Paul Hübner <[email protected]> wrote:
>> Hi all, >> >> Many tier 1 tests fail when running with >> `JTREG="JAVA_OPTIONS=--enable-preview"`. This PR does trivial fixes to test >> configurations and problemlists the rest of the failing tests. >> >> This PR is necessary to complete >> [JDK-8371357](https://bugs.openjdk.org/browse/JDK-8371357). >> >> Testing: tier1, with and without `--enable-preview`. > > Paul Hübner has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains five additional > commits since the last revision: > > - Merge branch 'lworld' into JDK-8373575 > - Move some tests to Xcomp list. > - Add failing test to problemlist. > - Add one of the AOT tests. > - Problemlist and fix. I'm okay with these changes, but I've embedded a few questions. test/jdk/ProblemList-Xcomp.txt line 35: > 33: valhalla/valuetypes/ObjectNewInstance.java 8369045 > macosx-all > 34: valhalla/valuetypes/ProxyTest.java 8369045 > macosx-all > 35: valhalla/valuetypes/WeakReferenceTest.java 8369045 > macosx-all Do these tests only fail with `-Xcomp` when `--enable-preview` is used? Or do these tests only fail with `-Xcomp` in `lworld` regardless of the `--enable-preview` value? If it's the latter case, then `ProblemList-Xcomp.txt` is the right place. Reading through [JDK-8369045](https://bugs.openjdk.org/browse/JDK-8369045), it sounds like `--enable-preview` is necessary so I don't think `ProblemList-Xcomp.txt` is the right place. test/jdk/java/lang/reflect/AccessFlag/SuperAccessFlagTest.java line 28: > 26: * @bug 8301720 > 27: * @summary Test expected value of SUPER AccessFlag for pre-ValueClass > .class file > 28: * @requires !java.enablePreview The reasons for `@requires !java.enablePreview` is clear because the summary mentions `pre-ValueClass`. test/langtools/TEST.ROOT line 53: > 51: vm.continuations \ > 52: vm.debug \ > 53: java.enablePreview This just requires the `java.enablePreview` property to be set to `true` or `false`. It does not require `java.enablePreview` == `true`, correct? test/langtools/tools/javac/AnonymousClass/AnonymousClassFlags.java line 29: > 27: * @summary Verify that anonymous class binaries have the correct flags > set > 28: * @modules java.base/jdk.internal.misc > 29: * @requires !java.enablePreview Why is `@requires !java.enablePreview` needed here? The test's id value is NoPreview so that might indicate that the test doesn't support --enable-preview bits, but I had to expand the review's context to see that id value. test/langtools/tools/javap/T4975569.java line 30: > 28: * @modules jdk.jdeps/com.sun.tools.javap > 29: * @modules java.base/jdk.internal.misc > 30: * @requires !java.enablePreview Why is `@requires !java.enablePreview` needed here? The test's `id` value is `NoPreview` so that might indicate that the test doesn't support `--enable-preview` bits, but I had to expand the review's context to see that `id` value. ------------- Marked as reviewed by dcubed (Committer). PR Review: https://git.openjdk.org/valhalla/pull/1793#pullrequestreview-3579844876 PR Review Comment: https://git.openjdk.org/valhalla/pull/1793#discussion_r2620608150 PR Review Comment: https://git.openjdk.org/valhalla/pull/1793#discussion_r2620633275 PR Review Comment: https://git.openjdk.org/valhalla/pull/1793#discussion_r2620627741 PR Review Comment: https://git.openjdk.org/valhalla/pull/1793#discussion_r2620635666 PR Review Comment: https://git.openjdk.org/valhalla/pull/1793#discussion_r2620643507
