From 69f543a29ba99a516fcf3827eebdebd25fa11bb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Art=C5=ABras=20=C5=A0lajus?= Date: Tue, 24 Jun 2025 15:51:01 +0300 Subject: [PATCH 1/4] Improvements to `WatchInputTests` and integration tests in general. --- .../src/WatchSourceInputTests.scala | 46 ++++---- .../src/mill/testkit/IntegrationTester.scala | 106 +++++++++++++++--- .../testkit/UtestIntegrationTestSuite.scala | 2 + testkit/src/mill/testkit/helpers.scala | 54 +++++++++ 4 files changed, 173 insertions(+), 35 deletions(-) create mode 100644 testkit/src/mill/testkit/helpers.scala diff --git a/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala b/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala index bbedde4cba86..0c6918b24bc7 100644 --- a/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala +++ b/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala @@ -21,9 +21,10 @@ import scala.concurrent.ExecutionContext.Implicits.global */ trait WatchTests extends UtestIntegrationTestSuite { - val maxDuration = 120000 + val maxDurationMillis: Int = if (sys.env.contains("CI")) 120000 else 5000 + def awaitCompletionMarker(tester: IntegrationTester, name: String): Unit = { - val maxTime = System.currentTimeMillis() + maxDuration + val maxTime = System.currentTimeMillis() + maxDurationMillis while (!os.exists(tester.workspacePath / "out" / name)) { if (System.currentTimeMillis() > maxTime) { sys.error(s"awaitCompletionMarker($name) timed out") @@ -32,11 +33,11 @@ trait WatchTests extends UtestIntegrationTestSuite { } } - def testBase(show: Boolean)(f: ( - mutable.Buffer[String], - mutable.Buffer[String], - mutable.Buffer[String] - ) => IntegrationTester.EvalResult): Unit = { + def testBase(preppedEval: IntegrationTester.PreparedEval, show: Boolean)(f: ( + expectedOut: mutable.Buffer[String], + expectedErr: mutable.Buffer[String], + expectedShows: mutable.Buffer[String] + ) => IntegrationTester.EvalResult): Unit = withTestClues(preppedEval.clues*) { val expectedOut = mutable.Buffer.empty[String] // Most of these are normal `println`s, so they go to `stdout` by // default unless you use `show` in which case they go to `stderr`. @@ -61,11 +62,13 @@ trait WatchTests extends UtestIntegrationTestSuite { object WatchSourceTests extends WatchTests { val tests: Tests = Tests { - def testWatchSource(tester: IntegrationTester, show: Boolean) = - testBase(show) { (expectedOut, expectedErr, expectedShows) => - val showArgs = if (show) Seq("show") else Nil - import tester._ - val evalResult = Future { eval(("--watch", showArgs, "qux"), timeout = maxDuration) } + def testWatchSource(tester: IntegrationTester, show: Boolean): Unit = { + import tester.* + val showArgs = if (show) Seq("show") else Nil + val preppedEval = prepEval(("--watch", showArgs, "qux"), timeout = maxDurationMillis) + + testBase(preppedEval, show) { (expectedOut, expectedErr, expectedShows) => + val evalResult = Future { preppedEval.run() } awaitCompletionMarker(tester, "initialized0") awaitCompletionMarker(tester, "quxRan0") @@ -125,8 +128,10 @@ object WatchSourceTests extends WatchTests { awaitCompletionMarker(tester, "initialized2") expectedOut.append("Setting up build.mill") - Await.result(evalResult, Duration.apply(maxDuration, SECONDS)) + Await.result(evalResult, Duration.apply(maxDurationMillis, SECONDS)) } + } + test("sources") { // Make sure we clean up the workspace between retries @@ -151,11 +156,13 @@ object WatchSourceTests extends WatchTests { object WatchInputTests extends WatchTests { val tests: Tests = Tests { - def testWatchInput(tester: IntegrationTester, show: Boolean) = - testBase(show) { (expectedOut, expectedErr, expectedShows) => - val showArgs = if (show) Seq("show") else Nil - import tester._ - val evalResult = Future { eval(("--watch", showArgs, "lol"), timeout = maxDuration) } + def testWatchInput(tester: IntegrationTester, show: Boolean) = { + val showArgs = if (show) Seq("show") else Nil + import tester.* + val preppedEval = prepEval(("--watch", showArgs, "lol"), timeout = maxDurationMillis) + + testBase(preppedEval, show) { (expectedOut, expectedErr, expectedShows) => + val evalResult = Future { preppedEval.run() } awaitCompletionMarker(tester, "initialized0") awaitCompletionMarker(tester, "lolRan0") @@ -181,8 +188,9 @@ object WatchInputTests extends WatchTests { if (show) expectedOut.append("{}") expectedOut.append("Setting up build.mill") - Await.result(evalResult, Duration.apply(maxDuration, SECONDS)) + Await.result(evalResult, Duration.apply(maxDurationMillis, SECONDS)) } + } test("input") { diff --git a/testkit/src/mill/testkit/IntegrationTester.scala b/testkit/src/mill/testkit/IntegrationTester.scala index b53dc7840115..621eda7506a2 100644 --- a/testkit/src/mill/testkit/IntegrationTester.scala +++ b/testkit/src/mill/testkit/IntegrationTester.scala @@ -6,6 +6,8 @@ import mill.define.Cached import mill.define.SelectMode import ujson.Value +import scala.concurrent.duration.* + /** * Helper meant for executing Mill integration tests, which runs Mill in a subprocess * against a folder with a `build.mill` and project files. Provides APIs such as [[eval]] @@ -41,6 +43,30 @@ object IntegrationTester { def isSuccess = exitCode == 0 } + /** An [[Impl.eval]] that is prepared for execution but haven't been executed yet. Run it with [[run]]. */ + case class PreparedEval( + cmd: os.Shellable, + env: Map[String, String], + cwd: os.Path, + timeout: Duration, + check: Boolean, + propagateEnv: Boolean = true, + shutdownGracePeriod: Long = 100, + run: () => EvalResult + ) { + /** Clues to use for with [[withTestClues]]. */ + def clues: Seq[utest.TestValue] = Seq( + cmd.asTestValue("eval.cmd.shellable"), + cmd.value.iterator.map(s => s"\"$s\"").mkString(" ").asTestValue("eval.cmd"), + env.asTestValue("eval.env"), + cwd.asTestValue("eval.cwd"), + timeout.asTestValue("eval.timeout"), + check.asTestValue("eval.check"), + propagateEnv.asTestValue("eval.propagateEnv"), + shutdownGracePeriod.asTestValue("eval.shutdownGracePeriod") + ) + } + trait Impl extends AutoCloseable with IntegrationTesterBase { def millExecutable: os.Path @@ -51,12 +77,11 @@ object IntegrationTester { def debugLog = false /** - * Evaluates a Mill command. Essentially the same as `os.call`, except it - * provides the Mill executable and some test flags and environment variables - * for you, and wraps the output in a [[IntegrationTester.EvalResult]] for - * convenience. + * Prepares to evaluate a Mill command. Run it with [[IntegrationTester.PreparedEval.run]]. + * + * Useful when you need the [[IntegrationTester.PreparedEval.clues]]. */ - def eval( + def prepEval( cmd: os.Shellable, env: Map[String, String] = Map.empty, cwd: os.Path = workspacePath, @@ -68,7 +93,7 @@ object IntegrationTester { check: Boolean = false, propagateEnv: Boolean = true, timeoutGracePeriod: Long = 100 - ): IntegrationTester.EvalResult = { + ): IntegrationTester.PreparedEval = { val serverArgs = Option.when(!daemonMode)("--no-daemon") val debugArgs = Option.when(debugLog)("--debug") @@ -76,9 +101,64 @@ object IntegrationTester { val shellable: os.Shellable = (millExecutable, serverArgs, "--ticker", "false", debugArgs, cmd) - val res0 = os.call( + val callEnv = millTestSuiteEnv ++ env + + def run() = { + val res0 = os.call( + cmd = shellable, + env = callEnv, + cwd = cwd, + stdin = stdin, + stdout = stdout, + stderr = stderr, + mergeErrIntoOut = mergeErrIntoOut, + timeout = timeout, + check = check, + propagateEnv = propagateEnv, + shutdownGracePeriod = timeoutGracePeriod + ) + + IntegrationTester.EvalResult( + res0.exitCode, + fansi.Str(res0.out.text(), errorMode = fansi.ErrorMode.Strip).plainText.trim, + fansi.Str(res0.err.text(), errorMode = fansi.ErrorMode.Strip).plainText.trim + ) + } + + PreparedEval( cmd = shellable, - env = millTestSuiteEnv ++ env, + env = callEnv, + cwd = cwd, + timeout = if (timeout == -1) Duration.Inf else timeout.millis, + check = check, + propagateEnv = propagateEnv, + shutdownGracePeriod = timeoutGracePeriod, + run = run + ) + } + + /** + * Evaluates a Mill command. Essentially the same as `os.call`, except it + * provides the Mill executable and some test flags and environment variables + * for you, and wraps the output in a [[IntegrationTester.EvalResult]] for + * convenience. + */ + def eval( + cmd: os.Shellable, + env: Map[String, String] = Map.empty, + cwd: os.Path = workspacePath, + stdin: os.ProcessInput = os.Pipe, + stdout: os.ProcessOutput = os.Pipe, + stderr: os.ProcessOutput = os.Pipe, + mergeErrIntoOut: Boolean = false, + timeout: Long = -1, + check: Boolean = false, + propagateEnv: Boolean = true, + timeoutGracePeriod: Long = 100 + ): IntegrationTester.EvalResult = { + prepEval( + cmd = cmd, + env = env, cwd = cwd, stdin = stdin, stdout = stdout, @@ -87,14 +167,8 @@ object IntegrationTester { timeout = timeout, check = check, propagateEnv = propagateEnv, - shutdownGracePeriod = timeoutGracePeriod - ) - - IntegrationTester.EvalResult( - res0.exitCode, - fansi.Str(res0.out.text(), errorMode = fansi.ErrorMode.Strip).plainText.trim, - fansi.Str(res0.err.text(), errorMode = fansi.ErrorMode.Strip).plainText.trim - ) + timeoutGracePeriod = timeoutGracePeriod + ).run() } /** diff --git a/testkit/src/mill/testkit/UtestIntegrationTestSuite.scala b/testkit/src/mill/testkit/UtestIntegrationTestSuite.scala index 63b7625ff13d..9f4f41247aaf 100644 --- a/testkit/src/mill/testkit/UtestIntegrationTestSuite.scala +++ b/testkit/src/mill/testkit/UtestIntegrationTestSuite.scala @@ -1,6 +1,8 @@ package mill.testkit abstract class UtestIntegrationTestSuite extends utest.TestSuite with IntegrationTestSuite { + export mill.testkit.{asTestValue, withTestClues} + protected def workspaceSourcePath: os.Path = os.Path(sys.env("MILL_TEST_RESOURCE_DIR")) protected def daemonMode: Boolean = sys.env("MILL_INTEGRATION_DAEMON_MODE").toBoolean diff --git a/testkit/src/mill/testkit/helpers.scala b/testkit/src/mill/testkit/helpers.scala new file mode 100644 index 000000000000..78e8a46fb0a1 --- /dev/null +++ b/testkit/src/mill/testkit/helpers.scala @@ -0,0 +1,54 @@ +package mill.testkit + +import utest.TestValue + +import scala.annotation.tailrec +import scala.language.implicitConversions +import scala.reflect.NameTransformer +import scala.quoted.* + +extension [A](a: A) { + inline def asTestValue: TestValue = + ${ TestValueConversion.impl('{a}, customName = '{None}) } + + inline def asTestValue(name: String): TestValue = + ${ TestValueConversion.impl('{a}, customName = '{Some(name)}) } +} + +object TestValueConversion { + def impl[A](a: Expr[A], customName: Expr[Option[String]])(using t: Type[A], ctx: Quotes): Expr[TestValue] = { + import ctx.reflect.* + + // Taken from https://github.com/Somainer/scala3-nameof/blob/ffe2e3c258171e2a70ccd5aa436063911fc3cc91/src/main/scala/com/somainer/nameof/NameOfMacros.scala#L8 + @tailrec + def extract(term: Term): String = term match + case Inlined(Some(term: Term), _, _) => extract(term) + case Inlined(None, _, inlined) => extract(inlined) + case Ident(n) => NameTransformer.decode(n) + case Select(_, n) => NameTransformer.decode(n) + case Block(DefDef(_, _, _, Some(term)) :: Nil, _) => extract(term) + case Block(_, expr) => extract(expr) + case Apply(func, _) => extract(func) + case Typed(expr, _) => extract(expr) + case TypeApply(func, _) => extract(func) + case _ => report.errorAndAbort(s"Can not know how to get name of ${a.show}: ${term}") + + val name = customName.valueOrAbort match { + case None => Expr(extract(a.asTerm)) + case Some(name) => Expr(name) + } + val typeName = Expr(Type.show[A]) + '{ TestValue(${name}, ${typeName}, $a) } + } +} + +/** Adds the provided clues to the thrown [[utest.AssertionError]]. */ +def withTestClues[A](clues: TestValue*)(f: => A): A = { + try f + catch { + case e: utest.AssertionError => + val newException = e.copy(captured = clues ++ e.captured) + newException.setStackTrace(e.getStackTrace) + throw newException + } +} From ccec515fc3aad185d612b250d7a48cbf9c6f11d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Art=C5=ABras=20=C5=A0lajus?= Date: Tue, 24 Jun 2025 17:38:49 +0300 Subject: [PATCH 2/4] Increase the timeout, as the tests otherwise fail. --- .../watch-source-input/src/WatchSourceInputTests.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala b/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala index 0c6918b24bc7..e1bd8238ca99 100644 --- a/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala +++ b/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala @@ -21,7 +21,7 @@ import scala.concurrent.ExecutionContext.Implicits.global */ trait WatchTests extends UtestIntegrationTestSuite { - val maxDurationMillis: Int = if (sys.env.contains("CI")) 120000 else 5000 + val maxDurationMillis: Int = if (sys.env.contains("CI")) 120000 else 15000 def awaitCompletionMarker(tester: IntegrationTester, name: String): Unit = { val maxTime = System.currentTimeMillis() + maxDurationMillis From 11c3695a12e58ed462fccd10e3163a2f8e83066d Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 24 Jun 2025 14:55:51 +0000 Subject: [PATCH 3/4] [autofix.ci] apply automated fixes --- .../src/mill/testkit/IntegrationTester.scala | 17 +++++++++-------- testkit/src/mill/testkit/helpers.scala | 11 +++++++---- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/testkit/src/mill/testkit/IntegrationTester.scala b/testkit/src/mill/testkit/IntegrationTester.scala index 621eda7506a2..8445d6d59540 100644 --- a/testkit/src/mill/testkit/IntegrationTester.scala +++ b/testkit/src/mill/testkit/IntegrationTester.scala @@ -45,15 +45,16 @@ object IntegrationTester { /** An [[Impl.eval]] that is prepared for execution but haven't been executed yet. Run it with [[run]]. */ case class PreparedEval( - cmd: os.Shellable, - env: Map[String, String], - cwd: os.Path, - timeout: Duration, - check: Boolean, - propagateEnv: Boolean = true, - shutdownGracePeriod: Long = 100, - run: () => EvalResult + cmd: os.Shellable, + env: Map[String, String], + cwd: os.Path, + timeout: Duration, + check: Boolean, + propagateEnv: Boolean = true, + shutdownGracePeriod: Long = 100, + run: () => EvalResult ) { + /** Clues to use for with [[withTestClues]]. */ def clues: Seq[utest.TestValue] = Seq( cmd.asTestValue("eval.cmd.shellable"), diff --git a/testkit/src/mill/testkit/helpers.scala b/testkit/src/mill/testkit/helpers.scala index 78e8a46fb0a1..b5a88d32611f 100644 --- a/testkit/src/mill/testkit/helpers.scala +++ b/testkit/src/mill/testkit/helpers.scala @@ -9,14 +9,17 @@ import scala.quoted.* extension [A](a: A) { inline def asTestValue: TestValue = - ${ TestValueConversion.impl('{a}, customName = '{None}) } + ${ TestValueConversion.impl('{ a }, customName = '{ None }) } inline def asTestValue(name: String): TestValue = - ${ TestValueConversion.impl('{a}, customName = '{Some(name)}) } + ${ TestValueConversion.impl('{ a }, customName = '{ Some(name) }) } } object TestValueConversion { - def impl[A](a: Expr[A], customName: Expr[Option[String]])(using t: Type[A], ctx: Quotes): Expr[TestValue] = { + def impl[A](a: Expr[A], customName: Expr[Option[String]])(using + t: Type[A], + ctx: Quotes + ): Expr[TestValue] = { import ctx.reflect.* // Taken from https://github.com/Somainer/scala3-nameof/blob/ffe2e3c258171e2a70ccd5aa436063911fc3cc91/src/main/scala/com/somainer/nameof/NameOfMacros.scala#L8 @@ -38,7 +41,7 @@ object TestValueConversion { case Some(name) => Expr(name) } val typeName = Expr(Type.show[A]) - '{ TestValue(${name}, ${typeName}, $a) } + '{ TestValue(${ name }, ${ typeName }, $a) } } } From 6750d0767a6e9f643e83aeb3140220b7f7052434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Art=C5=ABras=20=C5=A0lajus?= Date: Thu, 26 Jun 2025 11:05:26 +0300 Subject: [PATCH 4/4] Code review. --- .../src/mill/testkit/IntegrationTester.scala | 19 ++++---- testkit/src/mill/testkit/helpers.scala | 46 ++----------------- 2 files changed, 15 insertions(+), 50 deletions(-) diff --git a/testkit/src/mill/testkit/IntegrationTester.scala b/testkit/src/mill/testkit/IntegrationTester.scala index 8445d6d59540..bf94ee9b587a 100644 --- a/testkit/src/mill/testkit/IntegrationTester.scala +++ b/testkit/src/mill/testkit/IntegrationTester.scala @@ -57,15 +57,16 @@ object IntegrationTester { /** Clues to use for with [[withTestClues]]. */ def clues: Seq[utest.TestValue] = Seq( - cmd.asTestValue("eval.cmd.shellable"), - cmd.value.iterator.map(s => s"\"$s\"").mkString(" ").asTestValue("eval.cmd"), - env.asTestValue("eval.env"), - cwd.asTestValue("eval.cwd"), - timeout.asTestValue("eval.timeout"), - check.asTestValue("eval.check"), - propagateEnv.asTestValue("eval.propagateEnv"), - shutdownGracePeriod.asTestValue("eval.shutdownGracePeriod") - ) + // Copy-pastable shell command that you can run in bash/zsh/whatever + asTestValue("cmd", cmd.value.iterator.map(pprint.Util.literalize(_)).mkString(" ")), + asTestValue("cmd.shellable", cmd), + asTestValue(env), + asTestValue(cwd), + asTestValue(timeout), + asTestValue(check), + asTestValue(propagateEnv), + asTestValue(shutdownGracePeriod) + ).map(tv => tv.copy(name = "eval." + tv.name)) } trait Impl extends AutoCloseable with IntegrationTesterBase { diff --git a/testkit/src/mill/testkit/helpers.scala b/testkit/src/mill/testkit/helpers.scala index b5a88d32611f..35b5e56df062 100644 --- a/testkit/src/mill/testkit/helpers.scala +++ b/testkit/src/mill/testkit/helpers.scala @@ -1,49 +1,13 @@ package mill.testkit +import pprint.{TPrint, TPrintColors} import utest.TestValue -import scala.annotation.tailrec -import scala.language.implicitConversions -import scala.reflect.NameTransformer -import scala.quoted.* +def asTestValue[A](a: sourcecode.Text[A])(using typeName: TPrint[A]): TestValue = + TestValue(a.source, typeName.render(using TPrintColors.BlackWhite).plainText, a.value) -extension [A](a: A) { - inline def asTestValue: TestValue = - ${ TestValueConversion.impl('{ a }, customName = '{ None }) } - - inline def asTestValue(name: String): TestValue = - ${ TestValueConversion.impl('{ a }, customName = '{ Some(name) }) } -} - -object TestValueConversion { - def impl[A](a: Expr[A], customName: Expr[Option[String]])(using - t: Type[A], - ctx: Quotes - ): Expr[TestValue] = { - import ctx.reflect.* - - // Taken from https://github.com/Somainer/scala3-nameof/blob/ffe2e3c258171e2a70ccd5aa436063911fc3cc91/src/main/scala/com/somainer/nameof/NameOfMacros.scala#L8 - @tailrec - def extract(term: Term): String = term match - case Inlined(Some(term: Term), _, _) => extract(term) - case Inlined(None, _, inlined) => extract(inlined) - case Ident(n) => NameTransformer.decode(n) - case Select(_, n) => NameTransformer.decode(n) - case Block(DefDef(_, _, _, Some(term)) :: Nil, _) => extract(term) - case Block(_, expr) => extract(expr) - case Apply(func, _) => extract(func) - case Typed(expr, _) => extract(expr) - case TypeApply(func, _) => extract(func) - case _ => report.errorAndAbort(s"Can not know how to get name of ${a.show}: ${term}") - - val name = customName.valueOrAbort match { - case None => Expr(extract(a.asTerm)) - case Some(name) => Expr(name) - } - val typeName = Expr(Type.show[A]) - '{ TestValue(${ name }, ${ typeName }, $a) } - } -} +def asTestValue[A](name: String, a: A)(using typeName: TPrint[A]): TestValue = + TestValue(name, typeName.render(using TPrintColors.BlackWhite).plainText, a) /** Adds the provided clues to the thrown [[utest.AssertionError]]. */ def withTestClues[A](clues: TestValue*)(f: => A): A = {