Fix for 14106 cygwin failed tests #14113
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
|
The first problem in [test_windows_full] is this: 2021-12-14T19:21:44.1151367Z stderr[/cygdrive/c/Program Files (x86)/sbt/bin/sbt: line 2: $'\r': command not found]
2021-12-14T19:21:44.1152191Z stderr[/cygdrive/c/Program Files (x86)/sbt/bin/sbt: line 3: set: +]The cause of this symptom is described here: The fix (to be verified shortly) is to add SHELLOPTS with
The next problem is a path that needs to be normalized: I'll test and push these fixes asap. |
|
Some tests are failing because the test environment sometimes doesn't provide |
|
Thanks @philwalk for your input. Actually, these logs are from the second attempt (which I killed manually), here are from the first one: https://github.com/lampepfl/dotty/runs/4527616578?check_suite_focus=true Apparently, there is some deadlock/loop that blocks the tests execution (These were killed automatically after 6hours at the very same step, and these tasks takes usually 30-40 minutes to complete). My rough guess is that it could be entering REPL? I don't know if it is possible, but maybe this is hanging the tests execution. EDIT: I see that your latest commit got things working, though the tests are still failing, but now in a more controllable manner |
|
Ah, the failure is caused by ansi colors polluting the strings returned by the scripts, causing the comparison to fail. Here's the relevant lines from the test log: |
|
I'm optimistic about #14113, it's testing now, but it turns out that in windows console bash processes, ANSI color codes were polluting STDOUT, causing string comparisons to fail. |
|
The ANSI color problem seems to be fixed, so now the remaining issue has to do with system layout. The problem with A similar problem exists with [test_non_bootstrapped]: Here's where the test thinks Where do I look to discover the system layout of these tests? Apparently, it's different than the normal git workspace. |
Isn't it because no |
Ahh, that make sense. So we just need to add |
remove duplicate
Fix not-updating property
BarkingBad
approved these changes
Dec 16, 2021
There was a problem hiding this comment.
Thank @philwalk for your input and for resolving all the problems. Is it ready to be merged or do you want to change something?
No changes needed, AFAIK |
dwijnand
reviewed
Mar 2, 2022
| @@ -66,7 +66,7 @@ jobs: | |||
|
|
|||
| - name: Test | |||
| run: | | |||
| ./project/scripts/sbt ";compile ;test" | |||
| ./project/scripts/sbt ";dist/pack; compile ;test" | |||
There was a problem hiding this comment.
This re-introduces bootstrapping the compiler in "test_non_bootstrapped", which is annoying (can't see the results of running the tests in CI while the compiler fails to bootstrap).
If these tests need a bootstrapped compiler, can't they be in bootstrap-only?
There was a problem hiding this comment.
If these tests need a bootstrapped compiler, can't they be in bootstrap-only?
I don't know enough about the test architecture to offer a reply, perhaps this is a question for @BarkingBad ?
olsdavis
pushed a commit
to olsdavis/dotty
that referenced
this pull request
Apr 4, 2022
* refactor common code from scripting tests; invalid tests fail by default * dump stdout/stderr if BashScriptsTests.verifyScalaOpts fails * fix for failing cygwin tests for scala#14106 * normalize scala and scalac paths; set proper SHELLOPTS in cygwin bashCommandline env * improved detection of scalacPath and scalaPath; additional logging * print warnings; remove unused code * strip ansi colors from bash command line output, to fix windows tests * dist/pack before sbt test in test_windows_full and test_non_bootstrapped * squeeze redundancy from env-var-setting tests, add more log messages * further reduced redundancy; additional log messages * remove trailing java from JAVA_HOME value; shorten comand lines with relpath * Update BashScriptsTests.scala remove duplicate * Update BashScriptsTests.scala Fix not-updating property Co-authored-by: Andrzej Ratajczak <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[test_windows_full]
[test_non_bootstrapped]
Cygwin has an older version of
coreutilsand so is not compatible with/usr/bin/env -S bin/scalausage.To fix the problem of legacy
/usr/bin/envcombiningbin/scalawith arguments passed to it, the hashbang is split apart indist/bin/scala, similar to how it was done inscala3-3.0.2, although this variation is more flexible, as it's not limited to-classpathon the hashbang command line.For reference, here are the hashbang lines affected by this change: