scala / scala-dev

Scala 2 team issues. Not for user-facing bugs or directly actionable user-facing improvements. For build/test/infra and for longer-term planning and idea tracking. Our bug tracker is at https://github.com/scala/bug/issues
Apache License 2.0
130 stars 15 forks source link

Test failures on JDK 22 (on both 2.12.x and 2.13.x) #867

Closed SethTisue closed 3 months ago

SethTisue commented 3 months ago

seen at e.g. https://github.com/scala/scala/actions/runs/8636734847/job/23677330475

[error] 3 of 20 test tasks failed:
[error] - junit/testOnly -- +v
[error]   - junit/test:testOnly failed: sbt.TestsFailedException: Tests unsuccessful
[error] - partest run
[error]   - test/it:testOnly failed: sbt.TestsFailedException: Tests unsuccessful
[error] - partest pos neg jvm
[error]   - test/it:testOnly failed: sbt.TestsFailedException: Tests unsuccessful

I re-reran it a few times to be sure they aren't spurious.

example 2.13 job: https://github.com/scala/scala/actions/runs/8636734847/job/23677330475

one weird thing is that the Ubuntu jobs fail but the Windows jobs are green, wtf?

som-snytt commented 3 months ago

Scala interpreter should evaluate arithmetic expression failed is not promising.

On a dark background it looks positively diabolical.

image

Oh, I see we were supposed to zap the invader before it reached the bottom.

som-snytt commented 3 months ago

the ReplTest output is in color. That's run/StringConcat. Also, partest tab complete doesn't match case-correcting like zsh.

SethTisue commented 3 months ago

Scala interpreter should evaluate arithmetic expression failed is not promising

that one (scala.tools.xsbt.InteractiveConsoleInterfaceTest) is reproducible locally and is also a colorized-vs-not difference. perhaps that's the sole cause of the various failures?

SethTisue commented 3 months ago

Properties#coloredOutputEnabled is coming out differently because (in the context of InteractiveConsoleInterfaceTest) System.console() returns null on JDK 21 but on JDK 22 it returns a java.io.ProxyingConsole

SethTisue commented 3 months ago

https://jdk.java.net/22/release-notes says:

JLine As The Default Console Provider (JDK-8308591) System.console() has changed in this release to return a Console with enhanced editing features that improve the experience of programs that use the Console API. In addition, System.console() now returns a Console object when the standard streams are redirected or connected to a virtual terminal. In prior releases, System.console() returned null for these cases. This change may impact code that uses the return from System.console() to test if the VM is connected to a terminal. If needed, running with -Djdk.console=java.base will restore older behavior where the console is only returned when it is connected to a terminal.

A new method Console.isTerminal() has been added to test if console is connected to a terminal.

SethTisue commented 3 months ago

I guess we should call Console.isTerminal reflectively if it's available, and otherwise fallback to the old-style null test.

som-snytt commented 3 months ago

They also mention class file API as more stable & timely base for 3rd party libs.

https://openjdk.org/jeps/457

The console check is at https://github.com/scala/scala/pull/10751

SethTisue commented 3 months ago

merged https://github.com/scala/scala/pull/10751 but there are still test failures at https://github.com/scala/scala/actions/runs/8665032593

SethTisue commented 3 months ago

junit/test passes now, it's the partests that still fail

lrytz commented 3 months ago

-Vimplicits is still colored somehow, which is weird - it seems to go through consoleIsTerminal added in https://github.com/scala/scala/pull/10751.

I noticed https://github.com/scala/scala/pull/10751#pullrequestreview-2003122535 but that doesn't look like a possible cause for having too much color on linux.

som-snytt commented 3 months ago

More tweak-driven development https://github.com/scala/scala/pull/10758

I haven't double-checked -Vimplicits but will take up manual checks again. I also was trying to figure out what disables color besides the -Dscala.color:false. scala < text.scala for instance.

lrytz commented 3 months ago

merged scala/scala#10751 but there are still test failures at https://github.com/scala/scala/actions/runs/8665032593

@SethTisue I just saw now that this is not for current 2.13.x, I guess https://github.com/scala/scala/pull/10753 fixed it.

I can reproduce the failure with b68ac48 and it passes with current 2.13.x (6b68db9)

som-snytt commented 3 months ago

I removed the stray null in my getMethod, which was finger memory for the null array in that api. Then I went full Seth. I haven't looked whether it should catch more reflective errors; also IIRC Seth allowed true "is terminal" on failure, FSR I was pessimistic.

Meanwhile, in order to raise more dust, I just submitted a PR to switch partests to using syntax. The quickie sed was the fastest command I've run all year.

Edit: that was partly to become more scala-cli-aware. In the same vein, last night I read some of the jvm.analysis doc comments. I've intended to familiarize myself for several years, and decided I'd better do it before scala 2 is EOL.

lrytz commented 3 months ago

Since it's a bit of back and forth by now, here's the list of PRs with JDK 22 adjustments

Changes since JDK 22 addition in 2.13: https://github.com/scala/scala/compare/80ef12f932935b5d9f832150029afeadcacd6109...a5270194b6eec9678a79e2f88a83d83aa6b92de1

2.12: https://github.com/scala/scala/compare/a5df1e511ab49c33993df6a159d0bc4c43ca0abb...a9c9962190b5dd513cff5319e0b0ca1706d3cbee