Skip to content

Commit

Permalink
Fix PromptLogger 60s delay, improve test grouping prompt lines, bre…
Browse files Browse the repository at this point in the history
…akup large `scalalib` test suites (#3649)

* Break up some of the long-poll tests in scalalib.test into small
classes, in preparation for `testForkGrouping`
* Fix issue with `PromptLogger` waiting an extra 60s when paused before
exiting when calling `shutdown`
* Improve the prompt line message for the test groups to show a
collapsed list of test class names (which will get further collapsed by
the prompt as necessary)
  • Loading branch information
lihaoyi authored Oct 2, 2024
1 parent 637732a commit 1708307
Show file tree
Hide file tree
Showing 16 changed files with 958 additions and 722 deletions.
3 changes: 2 additions & 1 deletion main/util/src/mill/util/PromptLogger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ private[mill] class PromptLogger(
}

def ticker(s: String): Unit = ()
override def setPromptDetail(key: Seq[String], s: String): Unit = {
override def setPromptDetail(key: Seq[String], s: String): Unit = synchronized {
state.updateDetail(key, s)
}

Expand Down Expand Up @@ -138,6 +138,7 @@ private[mill] class PromptLogger(
else {
pauseNoticed = false
paused = true
promptUpdaterThread.interrupt()
try {
// After the prompt gets paused, wait until the `promptUpdaterThread` marks
// `pauseNoticed = true`, so we can be sure it's done printing out prompt updates for
Expand Down
43 changes: 32 additions & 11 deletions scalalib/src/mill/scalalib/TestModuleUtil.scala
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ private[scalalib] object TestModuleUtil {
EnvVars.MILL_WORKSPACE_ROOT -> T.workspace.toString
)

def runTestSubprocess(selectors2: Seq[String], base: os.Path) = {
def runTestRunnerSubprocess(selectors2: Seq[String], base: os.Path) = {
val outputPath = base / "out.json"
val testArgs = TestArgs(
framework = testFramework,
Expand Down Expand Up @@ -93,20 +93,23 @@ private[scalalib] object TestModuleUtil {
val subprocessResult: Either[String, (String, Seq[TestResult])] = filteredClassLists match {
// When no tests at all are discovered, run at least one test JVM
// process to go through the test framework setup/teardown logic
case Nil => runTestSubprocess(Nil, T.dest)
case Seq(singleTestClassList) => runTestSubprocess(singleTestClassList, T.dest)
case Nil => runTestRunnerSubprocess(Nil, T.dest)
case Seq(singleTestClassList) => runTestRunnerSubprocess(singleTestClassList, T.dest)
case multipleTestClassLists =>
val hasMultiClassGroup = multipleTestClassLists.exists(_.length > 1)
val futures = multipleTestClassLists.zipWithIndex.map { case (testClassList, i) =>
val groupLabel = testClassList match {
case Seq(single) =>
if (hasMultiClassGroup) s"group-$i-$single"
else single
case multiple => s"group-$i"
val groupPromptMessage = testClassList match {
case Seq(single) => single
case multiple =>
collapseTestClassNames(multiple).mkString(", ") + s", ${multiple.length} suites"
}

T.fork.async(T.dest / groupLabel, "" + i, groupLabel) {
(groupLabel, runTestSubprocess(testClassList, T.dest / groupLabel))
val folderName = testClassList match {
case Seq(single) => single
case multiple => s"group-$i-${multiple.head}"
}

T.fork.async(T.dest / folderName, "" + i, groupPromptMessage) {
(folderName, runTestRunnerSubprocess(testClassList, T.dest / folderName))
}
}

Expand Down Expand Up @@ -295,4 +298,22 @@ private[scalalib] object TestModuleUtil {
case _ => None
}
}

/**
* Shorten the long list of fully qualified class names by truncating
* repetitive segments so we can see more stuff on a single line
*/
def collapseTestClassNames(names0: Seq[String]): Seq[String] = {
val names = names0.sorted
Seq(names.head) ++ names.sliding(2).map {
case Seq(prev, next) =>
val prevSegments = prev.split('.')
val nextSegments = next.split('.')

nextSegments
.zipWithIndex
.map { case (s, i) => if (prevSegments.lift(i).contains(s)) s.head else s }
.mkString(".")
}
}
}
45 changes: 45 additions & 0 deletions scalalib/test/src/mill/scalalib/AssemblyExeTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package mill.scalalib

import scala.util.Properties
import mill.api.Result
import mill.testkit.{UnitTester, TestBaseModule}
import utest._

// Ensure the assembly is runnable, even if we have assembled lots of dependencies into it
// Reproduction of issues:
// - https://github.com/com-lihaoyi/mill/issues/528
// - https://github.com/com-lihaoyi/mill/issues/2650

object AssemblyExeTests extends TestSuite with AssemblyTestUtils {

def tests: Tests = Tests {
test("Assembly") {
test("exe") {
test("small") - UnitTester(TestCase, sourceRoot = sources).scoped { eval =>
val Right(result) = eval(TestCase.exe.small.assembly)
val originalPath = result.value.path
val resolvedPath =
if (Properties.isWin) {
val winPath = originalPath / os.up / s"${originalPath.last}.bat"
os.copy(originalPath, winPath)
winPath
} else originalPath
runAssembly(resolvedPath, TestCase.millSourcePath, checkExe = true)
}

test("large-should-fail") - UnitTester(TestCase, sourceRoot = sources).scoped { eval =>
val Left(Result.Failure(msg, Some(res))) = eval(TestCase.exe.large.assembly)
val expectedMsg =
"""The created assembly jar contains more than 65535 ZIP entries.
|JARs of that size are known to not work correctly with a prepended shell script.
|Either reduce the entries count of the assembly or disable the prepended shell script with:
|
| def prependShellScript = ""
|""".stripMargin
assert(msg == expectedMsg)

}
}
}
}
}
24 changes: 24 additions & 0 deletions scalalib/test/src/mill/scalalib/AssemblyNoExeTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package mill.scalalib

import mill.testkit.{UnitTester, TestBaseModule}
import utest._

object AssemblyNoExeTests extends TestSuite with AssemblyTestUtils {

def tests: Tests = Tests {
test("Assembly") {
test("noExe") {
test("small") - UnitTester(TestCase, sourceRoot = sources).scoped { eval =>
val Right(result) = eval(TestCase.noExe.small.assembly)
runAssembly(result.value.path, TestCase.millSourcePath)

}
test("large") - UnitTester(TestCase, sourceRoot = sources).scoped { eval =>
val Right(result) = eval(TestCase.noExe.large.assembly)
runAssembly(result.value.path, TestCase.millSourcePath)

}
}
}
}
}
66 changes: 66 additions & 0 deletions scalalib/test/src/mill/scalalib/AssemblyTestUtils.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package mill.scalalib

import mill._
import mill.util.Jvm
import mill.testkit.{UnitTester, TestBaseModule}

trait AssemblyTestUtils {

object TestCase extends TestBaseModule {
trait Setup extends ScalaModule {
def scalaVersion = "2.13.11"

def sources = Task.Sources(T.workspace / "src")

def ivyDeps = super.ivyDeps() ++ Agg(
ivy"com.lihaoyi::scalatags:0.8.2",
ivy"com.lihaoyi::mainargs:0.4.0",
ivy"org.apache.avro:avro:1.11.1"
)
}

trait ExtraDeps extends ScalaModule {
def ivyDeps = super.ivyDeps() ++ Agg(
ivy"dev.zio::zio:2.0.15",
ivy"org.typelevel::cats-core:2.9.0",
ivy"org.apache.spark::spark-core:3.4.0",
ivy"dev.zio::zio-metrics-connectors:2.0.8",
ivy"dev.zio::zio-http:3.0.0-RC2"
)
}

object noExe extends Module {
object small extends Setup {
override def prependShellScript: T[String] = ""
}

object large extends Setup with ExtraDeps {
override def prependShellScript: T[String] = ""
}
}

object exe extends Module {
object small extends Setup

object large extends Setup with ExtraDeps
}

}

val sources = os.Path(sys.env("MILL_TEST_RESOURCE_DIR")) / "assembly"
def runAssembly(file: os.Path, wd: os.Path, checkExe: Boolean = false): Unit = {
println(s"File size: ${os.stat(file).size}")
Jvm.runSubprocess(
commandArgs = Seq(Jvm.javaExe, "-jar", file.toString(), "--text", "tutu"),
envArgs = Map.empty[String, String],
workingDir = wd
)
if (checkExe) {
Jvm.runSubprocess(
commandArgs = Seq(file.toString(), "--text", "tutu"),
envArgs = Map.empty[String, String],
workingDir = wd
)
}
}
}
117 changes: 0 additions & 117 deletions scalalib/test/src/mill/scalalib/AssemblyTests.scala

This file was deleted.

Loading

0 comments on commit 1708307

Please sign in to comment.