Skip to content

Enable Windows CI #9666

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 22 commits into from
Sep 7, 2020
Merged

Enable Windows CI #9666

merged 22 commits into from
Sep 7, 2020

Conversation

liufengyun
Copy link
Contributor

@liufengyun liufengyun commented Aug 28, 2020 •
edited
Loading

Enable run and pos tests on windows

@liufengyun liufengyun changed the title Enable run and pos tests Enable run and pos tests for windows CI Aug 28, 2020
@liufengyun liufengyun force-pushed the fix-4588 branch 2 times, most recently from 602f0e8 to 0fe5e28 Compare September 1, 2020 12:02
sjrd
sjrd approved these changes Sep 2, 2020
View reviewed changes
Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

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

Thank you so much @liufengyun for making this work on Windows!

sjrd
sjrd approved these changes Sep 2, 2020
View reviewed changes
Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

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

Thank you so much @liufengyun for making this work on Windows!

@odersky
Copy link
Contributor

odersky commented Sep 2, 2020

It seems the CI is down or hangs somehwere. All open PRs I see are queued and none is making progress.

@liufengyun
Copy link
Contributor Author

It seems the CI is down or hangs somehwere. All open PRs I see are queued and none is making progress.

It is still running, just that there is only one worker, thus moves slowly.

After everything is green, I'll investigate how to move Windows CI from appveyor to Github actions and have multiple workers.

@liufengyun liufengyun force-pushed the fix-4588 branch 2 times, most recently from e868b2f to 0bdab09 Compare September 3, 2020 08:35
@liufengyun
Copy link
Contributor Author

Now both sbt test and sbt bootstrapped/test should succeed on Windows.

  • sbt test
    It has been green on appveyor (but it's unstable)
  • sbt bootstrapped/test
    I haven't be able to test it on the Windows virtual machine due to memory quota. The 3 language server test cases that fail are disabled on Windows.

However, we should not merge this PR until we migrate to a infrastructure that can run the jobs fast & reliably.

@liufengyun liufengyun changed the title Enable run and pos tests for windows CI Enable Windows CI Sep 3, 2020
@liufengyun
Enable run and pos tests
17303cf
@liufengyun
Fix path in neg tests
9ed534a
@liufengyun
Tolerate windows path in check files
f611147
@liufengyun
Ignore windows line endings in line comparison
18fe28d 
We need to compare by first stripping the line ends

tests\neg\unicodeEscapes.scala failed
tests\neg\unicodeEscapes-interpolations.scala failed
@liufengyun
Code refactor
6e41c5b
@liufengyun
Properly close file
ab0b792 
Otherwise, we encounter the following error on windows:

java.nio.file.FileSystemException: out\rewrites\i8982\rewrites\i8982\i8982.scala: The process cannot access the file because it is being used by another process.
	at sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:86)
	at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:97)
	at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:102)
	at sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:269)
	at sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:103)
	at java.nio.file.Files.delete(Files.java:1126)
	at dotty.tools.vulpix.ParallelTesting$CompilationTest.delete(ParallelTesting.scala:1061)
	at dotty.tools.vulpix.ParallelTesting$CompilationTest.delete$$anonfun$2(ParallelTesting.scala:1060)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:15)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.ArrayOps$.foreach$extension(ArrayOps.scala:1323)
	at dotty.tools.vulpix.ParallelTesting$CompilationTest.delete(ParallelTesting.scala:1060)
	at dotty.tools.vulpix.ParallelTesting$CompilationTest.delete$$anonfun$1(ParallelTesting.scala:1057)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:15)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.immutable.List.foreach(List.scala:333)
	at dotty.tools.vulpix.ParallelTesting$CompilationTest.delete(ParallelTesting.scala:1057)
	at dotty.tools.vulpix.ParallelTesting$CompilationTest.cleanup(ParallelTesting.scala:985)
	at dotty.tools.vulpix.ParallelTesting$CompilationTest.checkCompile(ParallelTesting.scala:892)
	at dotty.tools.vulpix.ParallelTesting$RewriteTest.verifyOutput(ParallelTesting.scala:614)
	at dotty.tools.vulpix.ParallelTesting$RewriteTest.onSuccess(ParallelTesting.scala:618)
	at dotty.tools.vulpix.ParallelTesting$CompilationLogic.dotty$tools$vulpix$ParallelTesting$CompilationLogic$$onComplete(ParallelTesting.scala:275)
	at dotty.tools.vulpix.ParallelTesting$$anon$2.checkTestSource$$anonfun$1(ParallelTesting.scala:264)
	at dotty.runtime.function.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:12)
	at dotty.tools.vulpix.ParallelTesting$Test.tryCompile(ParallelTesting.scala:431)
	at dotty.tools.vulpix.ParallelTesting$$anon$2.checkTestSource(ParallelTesting.scala:266)
	at dotty.tools.vulpix.ParallelTesting$Test$LoggedRunnable.run(ParallelTesting.scala:334)
	at dotty.tools.vulpix.ParallelTesting$Test$LoggedRunnable.run$(ParallelTesting.scala:316)
	at dotty.tools.vulpix.ParallelTesting$$anon$2.run(ParallelTesting.scala:261)
@liufengyun
Fix path in timeout test
2653ba9
@liufengyun
Add unicode test
f624b70
@liufengyun
Fix unicode support in REPL on Windows
3f33dd4 
The following test now passes:

compiler/test-resources/repl/unicodeChars
@liufengyun
Tolerate printing different on different platforms
273068c 
Note that Mac agrees with Windows in printing.
The compiler generated signature is the same across platforms:

signature for method m = ()LTest$Foo$A<LTest$Foo<TT1;>.B$;>;
@liufengyun
Disable two meta tests on Windows
df977e9 
The appveyor has problem with the tests:

- timeout: unknown error
- stackoverflow: VM crashes due to memory quota

Both tests succeed on local Windows machine.
@liufengyun
Fix typo
e4a6166
@liufengyun
Disable failing language server tests on Windows
6d2932d
@liufengyun
Address review
f61a524
@liufengyun
More UTF-8 string fixes
96e9d21
@liufengyun
Use suggested fix for file close
7a7bf71
@liufengyun
test bootstrapped/test
36404ac 
Appveyor has 1 hour limit:

https://www.appveyor.com/docs/build-configuration/#time-limitations

Disable other tests to see if we can get `bootstrapped/test` green.
@sjrd
Copy link
Member

sjrd commented Sep 4, 2020 •
edited
Loading

dotty-bootstrapped/test completed in 16 minutes on my Windows machine, but some tests failed. The full transcript is in the following gist: https://gist.github.com/sjrd/cdb570d5a3fef11edc5dcb013a5e3df4
FTR that sbt instance gets allocated about 1GB of RAM.

@liufengyun
Copy link
Contributor Author

Thanks a lot @sjrd !

@liufengyun
Copy link
Contributor Author

@sjrd : what about we merge the PR, and figure out the infrastructure in another PR?

@sjrd
Copy link
Member

sjrd commented Sep 7, 2020

Yes, I think we can merge it as is. At least now test and dotty-bootstrap/test pass on my Windows machine, which is a huge improvement over the status quo.

@liufengyun liufengyun mentioned this pull request Sep 7, 2020
Closed
@liufengyun liufengyun merged commit d11e137 into scala:master Sep 7, 2020
@liufengyun liufengyun deleted the fix-4588 branch September 7, 2020 11:44
@Kordyjan Kordyjan added this to the 3.0.0 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjrd sjrd sjrd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@liufengyun @odersky @sjrd @Kordyjan