-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Java 8 CI #14126
Fix Java 8 CI #14126
Conversation
@@ -403,7 +403,7 @@ jobs: | |||
|
|||
- name: Test | |||
run: | | |||
./project/scripts/sbt ";scala3-bootstrapped/compile ;scala3-bootstrapped/test;sjsSandbox/run;sjsSandbox/test;sjsJUnitTests/test;sjsCompilerTests/test ;sbt-test/scripted scala2-compat/* ;configureIDE ;stdlib-bootstrapped/test:run ;stdlib-bootstrapped-tasty-tests/test" | |||
./project/scripts/sbt ";dist/pack ;scala3-bootstrapped/compile ;scala3-bootstrapped/test;sjsSandbox/run;sjsSandbox/test;sjsJUnitTests/test;sjsCompilerTests/test ;sbt-test/scripted scala2-compat/* ;configureIDE ;stdlib-bootstrapped/test:run ;stdlib-bootstrapped-tasty-tests/test" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this just needed for the bootstrapCmdTests? If so, couldn't these tests run dist/pack themselves so we don't need to remember to do pack by hand before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid that tests that need packing are coming from scala3-bootstrapped/test
. I know they could and probably should be in a different configuration, but I didn't create them. Actually, I saw that these tests were silently failing for a long long time (e. g. https://github.com/lampepfl/dotty/runs/4470356079?check_suite_focus=true#step:9:764) and turned them on in #14073 not knowing that they will do such turmoil in the whole CI (most of the checks are not run on a simple PR). Maybe we could move them to a separate isolated configuration for that, though @philwalk can bring more insight on that.
Here is a screenshot from old action where there are millions of stderrs printed out, yet the overall process was succeeding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case we should make sure dist is run automatically when we run scala3-bootstrapped/test
, for example using dependsOn(dist/pack)
a bit like what we do for the coursier tests: https://github.com/lampepfl/dotty/blob/7a473349b3fb64e7c6c9dfcfb02758f4a2e3756c/project/Build.scala#L777
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging this to fix tonight's CI.
[test_java8]