Skip to content
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

Reenable and fix tests on Windows #1020

Merged
merged 5 commits into from Oct 28, 2019
Merged

Conversation

@tgodzik
Copy link
Collaborator

tgodzik commented Oct 27, 2019

After moving most test to GithubActions it turned out it might be possible to reenable the Windows tests. However, during the time we were not running the tests it seems some of them broke.

Fixes #878

Tomasz Godzik
@tgodzik

This comment has been minimized.

Copy link
Collaborator Author

tgodzik commented Oct 27, 2019

@marek1840 It seems that running and testing doesn't work on windows.

@@ -188,7 +188,7 @@ object CodeLensesLspSuite extends BaseLspSuite("codeLenses") {
def check(name: String, library: String = "")(expected: String): Unit = {
testAsync(name) {
cleanWorkspace()
val original = expected.replaceAll("<<.*>>\\W", "")
val original = expected.replaceAll("<<.*>>\\W+", "")

This comment has been minimized.

Copy link
@tgodzik

tgodzik Oct 27, 2019

Author Collaborator

This broke on windows due to "\n\r"

@@ -5,8 +5,23 @@ import java.util.concurrent.TimeUnit.SECONDS
import ch.epfl.scala.bsp4j.ScalaMainClass
import ch.epfl.scala.bsp4j.DebugSessionParamsDataKind
import scala.meta.internal.metals.MetalsEnrichments._
import scala.concurrent.duration.Duration
import scala.concurrent.Future

object DebugProtocolSuite extends BaseLspSuite("debug-protocol") {

This comment has been minimized.

Copy link
@tgodzik

tgodzik Oct 27, 2019

Author Collaborator

Debug is not working - ignored

@@ -25,7 +25,7 @@ object DocumentHighlightLspSuite extends BaseLspSuite("documentHighlight") {
"""
|object Main {
| val abc = 123
| abc.<<toInt@@>>
| abc.<<to@@Int>>

This comment has been minimized.

Copy link
@tgodzik

tgodzik Oct 27, 2019

Author Collaborator

Seems that doesn't work with cursor at last position - doesn't seem like a big issue and might not make sense to debug until someone reports. Looks to be working outside of tests.

This comment has been minimized.

Copy link
@olafurpg

olafurpg Oct 28, 2019

Member

Hmm, I wonder what's going on here 🤔

@tgodzik tgodzik force-pushed the tgodzik:reeanble-windows branch from 56fd8c8 to e4e3020 Oct 27, 2019
@tgodzik tgodzik force-pushed the tgodzik:reeanble-windows branch from e4e3020 to 7277b73 Oct 27, 2019
@tgodzik tgodzik requested review from olafurpg and marek1840 Oct 28, 2019
@tgodzik tgodzik force-pushed the tgodzik:reeanble-windows branch from c2c4139 to 1325fb9 Oct 28, 2019
…y created by the test.
@tgodzik tgodzik force-pushed the tgodzik:reeanble-windows branch 2 times, most recently from 10b9ba9 to ac24d19 Oct 28, 2019
@@ -173,8 +175,9 @@ final class FoldingRangeExtractor(
startToken <- term.expr.findFirstTrailing(_.is[Token.KwCatch])
lastCase <- term.catchp.lastOption
endToken <- lastCase.findFirstTrailing(_.is[Token.RightBrace])
} yield
Position.Range(tree.pos.input, startToken.pos.end, endToken.pos.end)
tryPos = Position

This comment has been minimized.

Copy link
@tgodzik

tgodzik Oct 28, 2019

Author Collaborator

Formatting here was a bit crazy and worked differently on Windows.

This comment has been minimized.

Copy link
@olafurpg

olafurpg Oct 28, 2019

Member

What's going on 😨

@tgodzik tgodzik force-pushed the tgodzik:reeanble-windows branch from ac24d19 to 5b7d85b Oct 28, 2019
@tgodzik tgodzik force-pushed the tgodzik:reeanble-windows branch from 5b7d85b to bf83d58 Oct 28, 2019
@tgodzik

This comment has been minimized.

Copy link
Collaborator Author

tgodzik commented Oct 28, 2019

I think this should be ready to merge. I ignored a couple of tests, but even having windows tests in a limited capacity will be helpful (and it's not a crazy amount of ignored tests).

I will work on fixing any flakiness that comes up.

Copy link
Member

olafurpg left a comment

LGTM 🚀 It's amazing to finally have the Windows CI enabled again, not to mention all jobs running in the same CI provider 😻

@@ -173,8 +175,9 @@ final class FoldingRangeExtractor(
startToken <- term.expr.findFirstTrailing(_.is[Token.KwCatch])
lastCase <- term.catchp.lastOption
endToken <- lastCase.findFirstTrailing(_.is[Token.RightBrace])
} yield
Position.Range(tree.pos.input, startToken.pos.end, endToken.pos.end)
tryPos = Position

This comment has been minimized.

Copy link
@olafurpg

olafurpg Oct 28, 2019

Member

What's going on 😨

@@ -25,7 +25,7 @@ object DocumentHighlightLspSuite extends BaseLspSuite("documentHighlight") {
"""
|object Main {
| val abc = 123
| abc.<<toInt@@>>
| abc.<<to@@Int>>

This comment has been minimized.

Copy link
@olafurpg

olafurpg Oct 28, 2019

Member

Hmm, I wonder what's going on here 🤔

@olafurpg olafurpg merged commit 9eddd91 into scalameta:master Oct 28, 2019
9 checks passed
9 checks passed
Windows unit tests
Details
Linux unit tests
Details
Sbt integration
Details
Maven integration
Details
Gradle integration
Details
Mill integration
Details
Slow tests
Details
Scala cross tests
Details
Scalafmt/Scalacheck/Docs
Details
@tgodzik tgodzik deleted the tgodzik:reeanble-windows branch Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.