On Tue, 7 Apr 2026 06:39:09 GMT, David Holmes <[email protected]> wrote:
>> If the test is no longer using a `@ValueBased` class then there is no need >> for this regression test, as it no longer exercises the relevant runtime >> code paths. However this means that we also no longer test that EA with >> `DiagnoseSyncOnValueBasedClasses` on non-preview runs. >> >> From what I can see, there is no plan to remove >> `DiagnoseSyncOnValueBasedClasses` with JEP 401. So I wonder if we could not >> just have this specific `ValueBased` test case be conditional enabled based >> on enable-preview. >> >> Removing the `@ValueBased` testing test case regardless of >> `DiagnoseSyncOnValueBasedClasses` means we have less coverage of any >> regressions that might be introduced after JEP 401 in non-preview mode >> w.r.t. EA and `@ValueBased`. >> >> But if the consensus is that we want to remove the preview-mode illegal code >> from (non-preview mode) tests, then I see no reason to keep this test case >> either. > > Isn't this just a test that should not be run in preview mode? By the time > value types are out of preview DiagnoseSyncOnValueBasedClasses should be gone > too, at which time the test can be removed. I think the best solution for now is to bracket the EARelockingValueBased test case with a `!PreviewFeatures.isEnabled()` check and then completely remove when out of preview. And I think given the nature of this change (checking if preview is enabled) it might be best to push it to the valhalla repo instead. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30560#discussion_r3047455139
