Update sbt-sourcegraph and re-enable SBT build tool tests - #636
Conversation
|
Note that I have some changes locally to be able to run build tool tests in parallel, but it seems to suffer from a concurrency bug, where MUnit prints all suite names first and then all test results FTR, the diff is diff --git a/build.sbt b/build.sbt
index 4b2a864..52066b7 100644
--- a/build.sbt
+++ b/build.sbt
@@ -509,7 +509,8 @@ lazy val buildTools = project
Map(
"SCIP_JAVA_CLI" -> ((cli / pack).value / "bin" / "scip-java").toString
),
- Test / fork := true
+ Test / fork := true,
+ Test / testForkedParallel := true
)
.dependsOn(agent, unit)
diff --git a/tests/buildTools/src/test/scala/tests/BaseBuildToolSuite.scala b/tests/buildTools/src/test/scala/tests/BaseBuildToolSuite.scala
index 80fe64a..d4401fb 100644
--- a/tests/buildTools/src/test/scala/tests/BaseBuildToolSuite.scala
+++ b/tests/buildTools/src/test/scala/tests/BaseBuildToolSuite.scala
@@ -21,6 +21,7 @@ import os.Shellable
object Java8Only extends munit.Tag("Java8Only")
abstract class BaseBuildToolSuite extends MopedSuite(ScipJava.app) {
+ self =>
override def environmentVariables: Map[String, String] = sys.env
def tags = List.empty[Tag]
@@ -49,8 +50,15 @@ abstract class BaseBuildToolSuite extends MopedSuite(ScipJava.app) {
// NOTE(olafur): workaround for https://github.com/scalameta/moped/issues/18
override val temporaryDirectory: DirectoryFixture =
new DirectoryFixture {
- private val path = BuildInfo.temporaryDirectory.toPath
- override def apply(): Path = path
+ private val path = BuildInfo
+ .temporaryDirectory
+ .toPath
+ .resolve(self.getClass().getSimpleName())
+
+ override def apply(): Path = {
+ Files.createDirectories(path)
+ path
+ }
override def beforeEach(context: BeforeEach): Unit = {
DeleteVisitor.deleteRecursively(path)
} |
|
@keynmol what happens if you remove the |
|
That seems to have helped - I made buildTools the only module running in parallel |
| private val path = BuildInfo | ||
| .temporaryDirectory | ||
| .toPath | ||
| .resolve(self.getClass().getSimpleName()) |
There was a problem hiding this comment.
Doing this so that individual suites don't clobber each other (otherwise you get issues with parallel folder deletion and all that).
Hacky but works for now.
GHA doesn't have the oomph to give us any benefit there
|
Ok, I made the tests sequential again on CI only - GHA workers have such tiny vCPUs and barely two of them, that it's not worth the increased GC overhead |
| ), | ||
| Test / fork := true | ||
| Test / fork := true, | ||
| // Our CI set up is a couple of measly vCPUs so parallelising tests there makes |
There was a problem hiding this comment.
We can probably get access to more powerful buildkite machines if we want to. I never looked into it.
2 participants
Now that we can run auto-indexing for SBT and support Scala 3, it's time to re-enable the tests
Test plan