Skip to content

Commit

Permalink
Update thirdparty tests, make subprocess testrunner run user code in …
Browse files Browse the repository at this point in the history
…root classloader (#2561)

1. Move `thirdparty` from `integration/` to `example/`, and simplify
them down to `acyclic`, `fansi`, and `jimfs`. We can add more complex
builds later if we want, but for now just having some realistic builds
that are not bitrotted and old is already an improvement over what we
have now.

2. Flip the sub-process `TestRunner` classloaders on its head: rather
than runner `TestRunner.scala` in the boot classloader and user code in
a sub-classloader, we run user code in the boot classloader and
`TestRunner.scala` in a sub classloader. This is necessary to make JimFS
work, and would probably fix
#716 and related issues.

3. Broke up and cleaned up `TestRunner.scala`:
1. Separated `TestRunner` and `TestRunnerMain0`, the two entrypoints,
from `TestRunnerUtils`
2. Replaced all the complicated logic for converting logic to/from CLI
flags with JSON written to a file by `TestModule` and read from the file
by `TestRunner`
3. Replaced `ClosableIterator` with `geny.Generator`, which also
provides guaranteed closing after iteration

4. Marked most of `testrunner` as `@internal`. Most of these could be
`private[mill]`, but I think we don't have a clear idea of how this
stuff is meant to be used yet, and it's plausible that other people may
want to interact with the testrunner in their own custom test suites. I
think `@internal` is better suited for now until we either (a) know
better what a stable API looks like or (b) decide that this stuff should
not be used externally.

We have much more control over `TestRunner.scala` than we do over user
code, so we can make sure that `TestRunner.scala` works when running in
a semi-isolated classloader, whereas for user code we can offer no such
guarantees. We can't do anything for `.testLocal` that runs in-process,
but at least for `.test` we can try to offer them a clean classloader
environment.

The classloader for user code is not 100% clean: we still have a single
class `mill.testrunner.entrypoint.TestRunnerMain` added within it.
That's probably OK, since no matter what we do w.r.t. shading and stuff
we still need somewhere to put our own main method to bootstrap the
`mill.testrunner` classloader.

The testrunner changes are covered by existing unit and
integration/example tests, and the new "user code runs in clean
classloader" logic is covered by `example.thirdparty[jimfs].local`.
  • Loading branch information
lihaoyi authored Jun 4, 2023
1 parent 87cd32a commit f944ceb
Show file tree
Hide file tree
Showing 32 changed files with 668 additions and 1,745 deletions.
4 changes: 0 additions & 4 deletions .github/workflows/actions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ jobs:
millargs: "integration.feature.__.local.test"
- java-version: 17
millargs: "integration.failure.__.test"
- java-version: 17
millargs: "integration.thirdparty.__.test"
- java-version: 17
millargs: docs.githubPages

Expand Down Expand Up @@ -128,8 +126,6 @@ jobs:
millargs: "integration.feature.__.server.test"
- java-version: 17
millargs: "integration.failure.__.fork.test"
- java-version: 17
millargs: "integration.thirdparty.__.fork.test"
- java-version: 17
millargs: "contrib.__.test"

Expand Down
6 changes: 3 additions & 3 deletions bsp/worker/src/mill/bsp/worker/MillScalaBuildServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import mill.{Agg, T}
import mill.bsp.worker.Utils.sanitizeUri
import mill.util.Jvm
import mill.scalalib.{JavaModule, ScalaModule, SemanticDbJavaModule, TestModule}
import mill.testrunner.TestRunner
import mill.testrunner.{Framework, TestRunner, TestRunnerUtils}
import sbt.testing.Fingerprint

import java.util.concurrent.CompletableFuture
Expand Down Expand Up @@ -107,8 +107,8 @@ private trait MillScalaBuildServer extends ScalaBuildServer { this: MillBuildSer
isolated = true,
closeContextClassLoaderWhenDone = false,
cl => {
val framework = TestRunner.framework(testFramework)(cl)
val discoveredTests = TestRunner.discoverTests(
val framework = Framework.framework(testFramework)(cl)
val discoveredTests = TestRunnerUtils.discoverTests(
cl,
framework,
Agg(compResult.classes.path)
Expand Down
115 changes: 38 additions & 77 deletions build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,15 @@ trait MillJavaModule extends JavaModule {
def testArgs: T[Seq[String]] = T { Seq("-Djna.nosys=true") }
def testDepPaths = T { upstreamAssemblyClasspath() ++ Seq(compile().classes) ++ resources() }

def testTransitiveDeps: T[Map[String, String]] = T {
val upstream = T.traverse(moduleDeps ++ compileModuleDeps) {
case m: MillJavaModule => m.testTransitiveDeps.map(Some(_))
case _ => T.task(None)
}().flatten.flatten
val current = Seq(testDep())
upstream.toMap ++ current
}

def repositoriesTask = T.task {
super.repositoriesTask() ++
Seq(MavenRepository("https://oss.sonatype.org/content/repositories/releases"))
Expand Down Expand Up @@ -281,15 +290,6 @@ trait MillScalaModule extends ScalaModule with MillJavaModule { outer =>
def scalaVersion = Deps.scalaVersion
def scalacOptions = super.scalacOptions() ++ Seq("-deprecation", "-P:acyclic:force")

def testTransitiveDeps: T[Map[String, String]] = T {
val upstream = T.traverse(outer.moduleDeps ++ outer.compileModuleDeps) {
case m: MillScalaModule => m.testTransitiveDeps.map(Some(_))
case _ => T.task(None)
}().flatten.flatten
val current = Seq(outer.testDep())
upstream.toMap ++ current
}

def testIvyDeps: T[Agg[Dep]] = Agg(Deps.utest)
def testModuleDeps: Seq[JavaModule] =
if (this == main) Seq(main)
Expand Down Expand Up @@ -507,7 +507,9 @@ object main extends MillStableScalaModule with BuildInfo{
}

object testrunner extends MillPublishScalaModule {
def moduleDeps = Seq(scalalib.api, main.util)
object entrypoint extends MillPublishJavaModule

def moduleDeps = Seq(scalalib.api, main.util, entrypoint)
}

object scalalib extends MillStableScalaModule {
Expand Down Expand Up @@ -616,7 +618,8 @@ object contrib extends Module {
object testng extends JavaModule with ContribModule {

def testTransitiveDeps =
super.testTransitiveDeps() ++ Seq(scalalib.testDep(), scalalib.worker.testDep())
super.testTransitiveDeps() ++
Seq(scalalib.testDep(), scalalib.worker.testDep(), testrunner.entrypoint.testDep())

// pure Java implementation
def artifactSuffix: T[String] = ""
Expand Down Expand Up @@ -926,7 +929,6 @@ object example extends MillScalaModule {
object web extends Cross[ExampleCrossModule](listIn(millSourcePath / "web"))

trait ExampleCrossModule extends IntegrationTestCrossModule {
def sources = T.sources()
def testRepoRoot: T[PathRef] = T.source(millSourcePath)
def compile = example.compile()
def forkEnv = super.forkEnv() ++ Map("MILL_EXAMPLE_PARSED" -> upickle.default.write(parsed()))
Expand Down Expand Up @@ -1003,6 +1005,30 @@ object example extends MillScalaModule {
PathRef(T.dest / "example.adoc")
}
}

def repoInfo = Map(
"acyclic" -> ("com-lihaoyi/acyclic", "1ec221f377794db39e8ff9b43415f81c703c202f"),
"fansi" -> ("com-lihaoyi/fansi", "169ac96d7c6761a72590d312a433cf12c572573c"),
"jimfs" -> ("google/jimfs", "5b60a42eb9d3cd7a2073d549bd0cb833f5a7e7e9")
)
object thirdparty extends Cross[ThirdPartyModule](listIn(millSourcePath / "thirdparty"))
trait ThirdPartyModule extends ExampleCrossModule {
val (repoPath, repoHash) = repoInfo(crossValue)
def repoSlug = repoPath.split("/").last

def testRepoRoot = T {
shared.downloadTestRepo(repoPath, repoHash, T.dest)
val wrapperFolder = T.dest / s"$repoSlug-$repoHash"

os.makeDir(T.dest / "merged")
os.copy(wrapperFolder, T.dest / "merged", mergeFolders = true)
os.remove.all(wrapperFolder)
os.copy(super.testRepoRoot().path, T.dest / "merged", mergeFolders = true, replaceExisting = true)
os.remove.all(T.dest / "merged" / ".mill-version")

PathRef(T.dest / "merged")
}
}
}

object integration extends MillScalaModule {
Expand All @@ -1017,71 +1043,6 @@ object integration extends MillScalaModule {
val name = if (scala.util.Properties.isWin) "mill.bat" else "mill"
T { PathRef(installLocalTask(binFile = T.task((T.dest / name).toString()))()) }
}

// Test of various third-party repositories
object thirdparty extends MillScalaModule {
def moduleDeps = Seq(integration)

object acyclic extends ThirdPartyModule {
def repoPath = "lihaoyi/acyclic"
def repoHash = "bc41cd09a287e2c270271e27ccdb3066173a8598"
}

object jawn extends ThirdPartyModule {
def repoPath = "non/jawn"
def repoHash = "fd8dc2b41ce70269889320aeabf8614fe1e8fbcb"
}

object ammonite extends ThirdPartyModule {
def repoPath = "lihaoyi/Ammonite"
def repoHash = "26b7ebcace16b4b5b4b68f9344ea6f6f48d9b53e"
}

object upickle extends ThirdPartyModule {
def repoPath = "lihaoyi/upickle"
def repoHash = "7f33085c890db7550a226c349832eabc3cd18769"
}

object caffeine extends ThirdPartyModule {
def repoPath = "ben-manes/caffeine"
def repoHash = "c02c623aedded8174030596989769c2fecb82fe4"

def runClasspath: T[Seq[PathRef]] = T {
// we need to trigger installation of testng-contrib for Caffeine
contrib.testng.publishLocal()()
super.runClasspath()
}
}

trait ThirdPartyModule extends IntegrationTestModule {
def moduleDeps = super.moduleDeps ++ Seq(thirdparty)
def repoPath: String
def repoHash: String
def repoSlug = repoPath.split("/").last

def testRepoRoot = T {
shared.downloadTestRepo(repoPath, repoHash, T.dest)
val wrapperFolder = T.dest / s"$repoSlug-$repoHash"
os.list(wrapperFolder).foreach(os.move.into(_, T.dest))
os.remove(wrapperFolder)

os.list(super.testRepoRoot().path)
.foreach(os.copy.into(_, T.dest, replaceExisting = true))

PathRef(T.dest)
}

object local extends ModeModule {
def runClasspath: T[Seq[PathRef]] = T {
// we need to trigger installation of testng-contrib for Caffeine
contrib.testng.publishLocal()()
super.runClasspath()
}
}
object fork extends ModeModule
object server extends ModeModule
}
}
}

def launcherScript(shellJvmArgs: Seq[String],
Expand Down
2 changes: 1 addition & 1 deletion contrib/testng/test/src/mill/testng/TestNGTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ object TestNGTests extends TestSuite {
}
"Test case lookup from inherited annotations" - workspaceTest(demo) { eval =>
val Right((result, evalCount)) = eval.apply(demo.test.test())
val tres = result.asInstanceOf[(String, Seq[mill.testrunner.TestRunner.Result])]
val tres = result.asInstanceOf[(String, Seq[mill.testrunner.TestResult])]
assert(
tres._2.size == 8
)
Expand Down
75 changes: 75 additions & 0 deletions example/thirdparty/acyclic/build.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import mill._, scalalib._, publish._

object Deps {
def acyclic = ivy"com.lihaoyi:::acyclic:0.3.6"
def scalaCompiler(scalaVersion: String) = ivy"org.scala-lang:scala-compiler:$scalaVersion"
val utest = ivy"com.lihaoyi::utest:0.8.1"
}

val crosses =
Seq("2.11.12") ++
Range.inclusive(8, 17).map("2.12." + _) ++
Range.inclusive(0, 10).map("2.13." + _)

object acyclic extends Cross[AcyclicModule](crosses)
trait AcyclicModule extends CrossScalaModule with PublishModule {
def crossFullScalaVersion = true
def artifactName = "acyclic"
def publishVersion = "1.3.3.7"

def pomSettings = PomSettings(
description = artifactName(),
organization = "com.lihaoyi",
url = "https://github.com/com-lihaoyi/acyclic",
licenses = Seq(License.MIT),
versionControl = VersionControl.github(owner = "com-lihaoyi", repo = "acyclic"),
developers = Seq(
Developer("lihaoyi", "Li Haoyi", "https://github.com/lihaoyi")
)
)

def compileIvyDeps = Agg(Deps.scalaCompiler(crossScalaVersion))

object test extends ScalaModuleTests with TestModule.Utest {
def sources = T.sources(millSourcePath / "src", millSourcePath / "resources")
def ivyDeps = Agg(Deps.utest, Deps.scalaCompiler(crossScalaVersion))
}
}

// Acyclic is an example of a very small project that is a Scala compiler
// plugin. It is cross-built against all point versions of Scala from 2.11.12
// to 2.13.10, and has a dependency on the `org.scala-lang:scala-compiler`

/** Usage
> ./mill resolve acyclic[_].compile
acyclic[2.11.12].compile
acyclic[2.12.10].compile
acyclic[2.12.11].compile
acyclic[2.12.12].compile
acyclic[2.12.13].compile
acyclic[2.12.14].compile
acyclic[2.12.15].compile
acyclic[2.12.16].compile
acyclic[2.12.8].compile
acyclic[2.12.9].compile
acyclic[2.13.0].compile
acyclic[2.13.1].compile
acyclic[2.13.2].compile
acyclic[2.13.3].compile
acyclic[2.13.4].compile
acyclic[2.13.5].compile
acyclic[2.13.6].compile
acyclic[2.13.7].compile
acyclic[2.13.8].compile
acyclic[2.13.9].compile
> ./mill acyclic[2.12.17].compile
compiling 6 Scala sources...
...
> ./mill acyclic[2.13.10].test.testLocal # acyclic tests need testLocal due to classloader assumptions
-------------------------------- Running Tests --------------------------------
...
*/
93 changes: 93 additions & 0 deletions example/thirdparty/fansi/build.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import mill._, scalalib._, scalajslib._, scalanativelib._, publish._

val dottyCommunityBuildVersion = sys.props.get("dottyVersion").toList

val scalaVersions = Seq("2.12.17", "2.13.8", "2.11.12", "3.1.3") ++ dottyCommunityBuildVersion

trait FansiModule extends PublishModule with CrossScalaModule with PlatformScalaModule {
def publishVersion = "1.3.3.7"

def pomSettings = PomSettings(
description = artifactName(),
organization = "com.lihaoyi",
url = "https://github.com/com-lihaoyi/Fansi",
licenses = Seq(License.MIT),
versionControl = VersionControl.github(owner = "com-lihaoyi", repo = "fansi"),
developers = Seq(
Developer("lihaoyi", "Li Haoyi", "https://github.com/lihaoyi")
)
)

def ivyDeps = Agg(ivy"com.lihaoyi::sourcecode::0.3.0")

trait FansiTestModule extends ScalaModuleTests with TestModule.Utest {
def ivyDeps = Agg(ivy"com.lihaoyi::utest::0.8.1")
}
}

object fansi extends Module {
object jvm extends Cross[JvmFansiModule](scalaVersions)
trait JvmFansiModule extends FansiModule with ScalaModule {
object test extends FansiTestModule with ScalaModuleTests
}

object js extends Cross[JsFansiModule](scalaVersions)
trait JsFansiModule extends FansiModule with ScalaJSModule {
def scalaJSVersion = "1.10.1"
object test extends FansiTestModule with ScalaJSModuleTests
}

object native extends Cross[NativeFansiModule](scalaVersions)
trait NativeFansiModule extends FansiModule with ScalaNativeModule {
def scalaNativeVersion = "0.4.5"
object test extends FansiTestModule with ScalaNativeModuleTests
}
}

// Fansi is an example of a small library that is cross built against every
// minor version of Scala (including Scala 3.x) and every platform: JVM, JS,
// and Native. Both the library and the test suite are duplicated across all
// entries in the {version}x{platform} matrix, and you can select which one you
// want to compile, test, or publish

/** Usage
> ./mill resolve __.compile
fansi.js[2.11.12].test.compile
fansi.js[2.12.17].compile
fansi.js[2.12.17].test.compile
fansi.js[2.13.8].compile
fansi.js[2.13.8].test.compile
fansi.js[3.1.3].compile
fansi.js[3.1.3].test.compile
fansi.jvm[2.11.12].compile
fansi.jvm[2.11.12].test.compile
fansi.jvm[2.12.17].compile
fansi.jvm[2.12.17].test.compile
fansi.jvm[2.13.8].compile
fansi.jvm[2.13.8].test.compile
fansi.jvm[3.1.3].compile
fansi.jvm[3.1.3].test.compile
fansi.native[2.11.12].compile
fansi.native[2.11.12].test.compile
fansi.native[2.12.17].compile
fansi.native[2.12.17].test.compile
fansi.native[2.13.8].compile
fansi.native[2.13.8].test.compile
fansi.native[3.1.3].compile
fansi.native[3.1.3].test.compile
> ./mill fansi.jvm[2.12.17].compile
compiling 1 Scala source...
...
> ./mill fansi.js[2.13.8].test
Starting process: node
-------------------------------- Running Tests --------------------------------
...
> ./mill fansi.native[3.1.3].publishLocal
Publishing Artifact(com.lihaoyi,fansi_native0.4_3,1.3.3.7) to ivy repo...
...
*/
Loading

0 comments on commit f944ceb

Please sign in to comment.