Skip to content

Skip test of legacy compiler if not JDK 17#25452

Open
som-snytt wants to merge 5 commits intoscala:mainfrom
som-snytt:issue/23090-bad-AccessFlag-test
Open

Skip test of legacy compiler if not JDK 17#25452
som-snytt wants to merge 5 commits intoscala:mainfrom
som-snytt:issue/23090-bad-AccessFlag-test

Conversation

@som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Mar 6, 2026

Fixes #23090

How much have your relied on LLM-based tools in this contribution?

LLM-free.

That is reminiscent of Ollie, ollie, in come free, which is how we said it.

How was the solution tested?

Manually tested locally under JDK 25, to show that

tests/run/backwardsCompat-implicitParens/A_1_c3.0.2.scala

causes that test to be skipped.

Single test files were not compiled correctly (with the legacy compiler), so also test that this fails without the fix and is skipped with the fix:

tests/run/lazyVals_c3.1.0.scala

Additional notes

Left the unused import of chaining because lacking it results in a tangential rabbit hole:

#25451

@som-snytt
Copy link
Contributor Author

som-snytt commented Mar 6, 2026

It should also skip "individual" test files.

Edit: tests such as tests/run/lazyVals_c3.1.0.scala did not compile with the intended legacy compiler. It would be nice to have test infrastructure for these behaviors, of course.

@som-snytt som-snytt marked this pull request as ready for review March 6, 2026 23:17
@Gedochao Gedochao requested a review from SolalPirelli March 9, 2026 14:12
val CompilerVersion = """c([\d\.]+)""".r
val HasCompilerVersion = """_c([\d\.]+)""".r.unanchored
val GroupOrdinal = """(\d+)""".r
val usingCanonicalJava = javaSpecVersion.startsWith("17")
Copy link
Contributor

Choose a reason for hiding this comment

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

Does "canonical" have a specific meaning in context here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank-you for asking the hard questions. I will answer when I have a canonical answer.

@som-snytt
Copy link
Contributor Author

The failure is coverage blah blah the single test with legacy compiler tests/run/lazyVals_c3.1.0.scala.

Aggravated by bootstrapped test not taking a single test name, so I'm running the entire suite per cycle. Maybe I'll figure out why the test rig is broken.

@som-snytt
Copy link
Contributor Author

Realized that I have to use JDK 17 to run that test, stop laughing.

The bigger joke is that bootstrapped test suite crashed my WSL process.

Catastrophic failure
Error code: Wsl/Service/E_UNEXPECTED
Press any key to continue...

@som-snytt
Copy link
Contributor Author

The error when trying to select tests is

[error] `test / test` tasks in a Scala.js project require `test / fork := false`.

@som-snytt
Copy link
Contributor Author

After running the successful suite, I only just noticed:

image

Tossing in options is unexpected. Why isn't it part of the test suite itself?

@som-snytt som-snytt force-pushed the issue/23090-bad-AccessFlag-test branch from 4f77381 to c8538b2 Compare March 12, 2026 13:24
@som-snytt
Copy link
Contributor Author

Really any test relying on legacy compilation should be excluded from coverage; there aren't many, so I'll look at why more tests don't fail.

@som-snytt
Copy link
Contributor Author

I was too "lazy" to check for more tests to exclude, viz,

Test 'tests/run/lazyVals_c3.0.0.scala' failed with output:                      
java.lang.ClassNotFoundException: Test

so obviously I wish I'd just implemented my previous suggestion.

@som-snytt som-snytt force-pushed the issue/23090-bad-AccessFlag-test branch 2 times, most recently from bfee2e3 to b1c1f76 Compare March 12, 2026 21:17
@som-snytt
Copy link
Contributor Author

Why I'm not a fan of "list of things everyone must continually edit".

image

If my test can ask to be excluded from Scala-js scalajs: --skip then surely it can do the same for coverage or any facility.

@som-snytt som-snytt force-pushed the issue/23090-bad-AccessFlag-test branch from b1c1f76 to c32b611 Compare March 15, 2026 09:01
Copy link
Contributor

@SolalPirelli SolalPirelli left a comment

Choose a reason for hiding this comment

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

looks nice now, can we future-proof it?


object TestConfiguration {

val usingBaselineJava = javaSpecVersion.startsWith("17")
Copy link
Contributor

Choose a reason for hiding this comment

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

any way we can reuse https://github.com/dotty-staging/dotty/blob/main/compiler/src/dotty/tools/dotc/config/ScalaSettingsProperties.scala#L13 to avoid introducing another place that needs changing when we bump the minimum JDK?

* is a number. These groups are then ordered in ascending order based on
* the value of `X` and each group is compiled one after the other.
* A file can request compilation by a legacy compiler via a version suffix:
* `A_1_c3.2.0.scala` in group 1 is compiled by 3.2.0 under canonical JDK 17.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* `A_1_c3.2.0.scala` in group 1 is compiled by 3.2.0 under canonical JDK 17.
* `A_1_c3.2.0.scala` in group 1 is compiled by 3.2.0 under the default JDK.

the exact version doesn't really matter here and this comment will become obsolete otherwise, IMHO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tests with latest master failed with JDK-21 on MacOS

2 participants