From 1779b953c1babcb039fa7f410d8c193ca27dc054 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sat, 7 Sep 2024 18:12:11 +0800 Subject: [PATCH 01/14] . --- testkit/src/mill/testkit/IntegrationTesterBase.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testkit/src/mill/testkit/IntegrationTesterBase.scala b/testkit/src/mill/testkit/IntegrationTesterBase.scala index 9d98ed6b3b4..ab3a0e3b99b 100644 --- a/testkit/src/mill/testkit/IntegrationTesterBase.scala +++ b/testkit/src/mill/testkit/IntegrationTesterBase.scala @@ -10,7 +10,7 @@ trait IntegrationTesterBase { * Mill build being tested. Contains the `build.mill` file, any application code, and * the `out/` folder containing the build output */ - val workspacePath: os.Path = os.temp.dir(dir = os.pwd) + val workspacePath: os.Path = os.pwd /** * Initializes the workspace in preparation for integration testing From de06ff363c7ac3992d4b2b3cb2c478493fcfc45a Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 11:30:23 +0800 Subject: [PATCH 02/14] . --- build.mill | 1 - main/api/src/mill/api/Retry.scala | 51 ++++ .../src/mill/{util => api}/PathRefTests.scala | 0 main/api/test/src/mill/api/RetryTests.scala | 74 ++++++ .../src/mill/main/client/ClientTests.java | 5 +- .../main/client/FileToStreamTailerTest.java | 227 +++++++++--------- .../src/mill/main/client/MillEnvTests.java | 43 ++-- .../test/src/mill/main/client/RetryRule.java | 36 +++ main/package.mill | 1 - main/util/src/mill/util/CoursierSupport.scala | 42 ++-- testkit/src/mill/testkit/ExampleTester.scala | 30 +-- .../mill/testkit/IntegrationTesterBase.scala | 10 +- 12 files changed, 326 insertions(+), 194 deletions(-) create mode 100644 main/api/src/mill/api/Retry.scala rename main/api/test/src/mill/{util => api}/PathRefTests.scala (100%) create mode 100644 main/api/test/src/mill/api/RetryTests.scala create mode 100644 main/client/test/src/mill/main/client/RetryRule.java diff --git a/build.mill b/build.mill index 1a02d6470eb..70b0f9c8810 100644 --- a/build.mill +++ b/build.mill @@ -147,7 +147,6 @@ object Deps { val junitInterface = ivy"com.github.sbt:junit-interface:0.13.3" val commonsIO = ivy"commons-io:commons-io:2.16.1" - val lambdaTest = ivy"de.tototec:de.tobiasroeser.lambdatest:0.8.0" val log4j2Core = ivy"org.apache.logging.log4j:log4j-core:2.23.1" val osLib = ivy"com.lihaoyi::os-lib:0.10.5" val pprint = ivy"com.lihaoyi::pprint:0.9.0" diff --git a/main/api/src/mill/api/Retry.scala b/main/api/src/mill/api/Retry.scala new file mode 100644 index 00000000000..88a495cb25c --- /dev/null +++ b/main/api/src/mill/api/Retry.scala @@ -0,0 +1,51 @@ +package mill.api + +import java.util.concurrent.TimeUnit +import scala.concurrent.duration.Duration +import scala.concurrent.{Await, Promise} + +object Retry { + /** + * Generic retry functionality + * + * @param count How many times to retry before giving up + * @param backoffMillis What is the initial backoff time + * @param backoffMultiplier How much to multiply the initial backoff each time + * @param timeoutMillis How much time we want to allow [[t]] to run. If passed, + * runs [[t]] in a separate thread and throws a `TimeoutException` + * if it takes too long + * @param filter Whether or not we want to retry a given exception at a given retryCount; + * defaults to `true` to retry all exceptions, but can be made more fine-grained + * to only retry specific exceptions, or log them together with the retryCount + * @param t The code block that we want to retry + * @return the value of evaluating [[t]], or throws an exception if evaluating + * [[t]] fails more than [[count]] times + */ + def apply[T](count: Int = 5, + backoffMillis: Long = 10, + backoffMultiplier: Double = 2.0, + timeoutMillis: Long = -1, + filter: (Int, Throwable) => Boolean = (_, _) => true)(t: => T): T = { + def rec(retryCount: Int, currentBackoffMillis: Long): T = { + try { + if(timeoutMillis == -1) t + else{ + val result = Promise[T] + val thread = new Thread(() => { + result.complete(scala.util.Try(t)) + }) + thread.start() + Await.result(result.future, Duration.apply(timeoutMillis, TimeUnit.MILLISECONDS)) + } + } + catch { + case e: Throwable if retryCount < count && filter(retryCount + 1, e) => + Thread.sleep(currentBackoffMillis) + rec(retryCount + 1, (currentBackoffMillis * backoffMultiplier).toInt) + } + } + + rec(0, backoffMillis) + } +} + diff --git a/main/api/test/src/mill/util/PathRefTests.scala b/main/api/test/src/mill/api/PathRefTests.scala similarity index 100% rename from main/api/test/src/mill/util/PathRefTests.scala rename to main/api/test/src/mill/api/PathRefTests.scala diff --git a/main/api/test/src/mill/api/RetryTests.scala b/main/api/test/src/mill/api/RetryTests.scala new file mode 100644 index 00000000000..bf59a1f4688 --- /dev/null +++ b/main/api/test/src/mill/api/RetryTests.scala @@ -0,0 +1,74 @@ +package mill.api + +import utest._ + + +object RetryTests extends TestSuite { + val tests: Tests = Tests { + test("fail") { + var count = 0 + try { + Retry(){ + count += 1 + throw new Exception("boom") + } + } catch{case ex => + assert(ex.getMessage == "boom") + } + + assert(count == 6) // 1 original try + 5 retries + } + test("succeed") { + var count = 0 + Retry(){ + count += 1 + if (count < 3) throw new Exception("boom") + } + assert(count == 3) + } + test("filter") { + var count = 0 + try { + Retry( + filter = { + case (i, ex: RuntimeException) => true + case _ => false + } + ){ + count += 1 + if (count < 3) throw new RuntimeException("boom") + else throw new Exception("foo") + } + } catch{case e: Exception => + assert(e.getMessage == "foo") + } + assert(count == 3) + } + test("timeout") { + test("fail") { + var count = 0 + try { + Retry(timeoutMillis = 100) { + count += 1 + Thread.sleep(1000) + } + } catch { + case e: Exception => + assert(e.getMessage == "Future timed out after [100 milliseconds]") + } + + assert(count == 6) // 1 original try + 5 retries + } + test("success") { + var count = 0 + Retry(timeoutMillis = 100) { + count += 1 + if (count < 3) Thread.sleep(1000) + } + + assert(count == 3) + } + + } + } +} diff --git a/main/client/test/src/mill/main/client/ClientTests.java b/main/client/test/src/mill/main/client/ClientTests.java index 42f47180164..d8bf292e20b 100644 --- a/main/client/test/src/mill/main/client/ClientTests.java +++ b/main/client/test/src/mill/main/client/ClientTests.java @@ -1,6 +1,5 @@ package mill.main.client; -import static de.tobiasroeser.lambdatest.Expect.expectEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -58,8 +57,8 @@ public void checkStringRoundTrip(String example) throws Exception{ Util.writeString(o, example); ByteArrayInputStream i = new ByteArrayInputStream(o.toByteArray()); String s = Util.readString(i); - expectEquals(example, s, "String as bytes: ["+example.getBytes()+"] differs from expected: ["+s.getBytes()+"]"); - expectEquals(i.available(), 0); + assertEquals(example, s, "String as bytes: ["+example.getBytes()+"] differs from expected: ["+s.getBytes()+"]"); + assertEquals(i.available(), 0); } public byte[] readSamples(String ...samples) throws Exception{ diff --git a/main/client/test/src/mill/main/client/FileToStreamTailerTest.java b/main/client/test/src/mill/main/client/FileToStreamTailerTest.java index 0c68b5e12fe..0fb1bbc1bac 100644 --- a/main/client/test/src/mill/main/client/FileToStreamTailerTest.java +++ b/main/client/test/src/mill/main/client/FileToStreamTailerTest.java @@ -1,162 +1,163 @@ package mill.main.client; -import static de.tobiasroeser.lambdatest.Expect.expectEquals; -import static de.tobiasroeser.lambdatest.Expect.expectTrue; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileOutputStream; import java.io.PrintStream; -import de.tobiasroeser.lambdatest.generic.DefaultReporter; -import de.tobiasroeser.lambdatest.junit.FreeSpec; +import org.junit.Ignore; +import org.junit.Test; -public class FileToStreamTailerTest extends FreeSpec { - public FileToStreamTailerTest() { - setReporter(new DefaultReporter()); +import static org.junit.Assert.*; - test("Handle non-existing file", () -> { - ByteArrayOutputStream bas = new ByteArrayOutputStream(); - final PrintStream ps = new PrintStream(bas); +public class FileToStreamTailerTest { - final File file = File.createTempFile("tailer", ""); - expectTrue(file.delete()); + @org.junit.Rule + public RetryRule retryRule = new RetryRule(3); - try ( + @Test + public void handleNonExistingFile() throws Exception{ + ByteArrayOutputStream bas = new ByteArrayOutputStream(); + final PrintStream ps = new PrintStream(bas); + + final File file = File.createTempFile("tailer", ""); + assertTrue(file.delete()); + + try ( + final FileToStreamTailer tailer = new FileToStreamTailer(file, ps, 10); + ) { + tailer.start(); + Thread.sleep(200); + assertEquals(bas.toString(), ""); + } + } + @Test + public void handleNoExistingFileThatAppearsLater() throws Exception { + ByteArrayOutputStream bas = new ByteArrayOutputStream(); + final PrintStream ps = new PrintStream(bas); + + final File file = File.createTempFile("tailer", ""); + assertTrue(file.delete()); + + try ( final FileToStreamTailer tailer = new FileToStreamTailer(file, ps, 10); + ) { + tailer.start(); + Thread.sleep(100); + assertEquals(bas.toString(), ""); + + try ( + PrintStream out = new PrintStream(new FileOutputStream(file)); ) { - tailer.start(); - Thread.sleep(200); - expectEquals(bas.toString(), ""); + out.println("log line"); + assertTrue(file.exists()); + Thread.sleep(100); + assertEquals(bas.toString(), "log line" + System.lineSeparator()); } - }); + } + } + + @Test + public void handleExistingInitiallyEmptyFile() throws Exception{ + ByteArrayOutputStream bas = new ByteArrayOutputStream(); + final PrintStream ps = new PrintStream(bas); - test("Handle non-existing file that appears later", () -> { - ByteArrayOutputStream bas = new ByteArrayOutputStream(); - final PrintStream ps = new PrintStream(bas); + final File file = File.createTempFile("tailer", ""); + assertTrue(file.exists()); - final File file = File.createTempFile("tailer", ""); - expectTrue(file.delete()); + try ( + final FileToStreamTailer tailer = new FileToStreamTailer(file, ps, 10); + ) { + tailer.start(); + Thread.sleep(100); + + assertEquals(bas.toString(), ""); try ( - final FileToStreamTailer tailer = new FileToStreamTailer(file, ps, 10); + PrintStream out = new PrintStream(new FileOutputStream(file)); ) { - tailer.start(); + out.println("log line"); + assertTrue(file.exists()); Thread.sleep(100); - expectEquals(bas.toString(), ""); - - try ( - PrintStream out = new PrintStream(new FileOutputStream(file)); - ) { - out.println("log line"); - expectTrue(file.exists()); - Thread.sleep(100); - expectEquals(bas.toString(), "log line" + System.lineSeparator()); - } + assertEquals(bas.toString(), "log line" + System.lineSeparator()); } - }); + } + } - test("Handle existing and initially empty file", () -> { - ByteArrayOutputStream bas = new ByteArrayOutputStream(); - final PrintStream ps = new PrintStream(bas); + @Test + public void handleExistingFileWithOldContent() throws Exception{ + ByteArrayOutputStream bas = new ByteArrayOutputStream(); + final PrintStream ps = new PrintStream(bas); - final File file = File.createTempFile("tailer", ""); - expectTrue(file.exists()); + final File file = File.createTempFile("tailer", ""); + assertTrue(file.exists()); + try ( + PrintStream out = new PrintStream(new FileOutputStream(file)); + ) { + out.println("old line 1"); + out.println("old line 2"); try ( final FileToStreamTailer tailer = new FileToStreamTailer(file, ps, 10); ) { tailer.start(); - Thread.sleep(100); - - expectEquals(bas.toString(), ""); - - try ( - PrintStream out = new PrintStream(new FileOutputStream(file)); - ) { - out.println("log line"); - expectTrue(file.exists()); - Thread.sleep(100); - expectEquals(bas.toString(), "log line" + System.lineSeparator()); - } + Thread.sleep(500); + assertEquals(bas.toString(), ""); + out.println("log line"); + assertTrue(file.exists()); + Thread.sleep(500); + assertEquals(bas.toString().trim(), "log line"); } - }); + } + } + @Ignore + @Test + public void handleExistingEmptyFileWhichDisappearsAndComesBack() throws Exception{ + ByteArrayOutputStream bas = new ByteArrayOutputStream(); + final PrintStream ps = new PrintStream(bas); + + final File file = File.createTempFile("tailer", ""); + assertTrue(file.exists()); - test("Handle existing file with old content", () -> { - ByteArrayOutputStream bas = new ByteArrayOutputStream(); - final PrintStream ps = new PrintStream(bas); + try ( + final FileToStreamTailer tailer = new FileToStreamTailer(file, ps, 10); + ) { + tailer.start(); + Thread.sleep(100); - final File file = File.createTempFile("tailer", ""); - expectTrue(file.exists()); + assertEquals(bas.toString(), ""); try ( PrintStream out = new PrintStream(new FileOutputStream(file)); ) { - out.println("old line 1"); - out.println("old line 2"); - try ( - final FileToStreamTailer tailer = new FileToStreamTailer(file, ps, 10); - ) { - tailer.start(); - Thread.sleep(500); - expectEquals(bas.toString(), ""); - out.println("log line"); - expectTrue(file.exists()); - Thread.sleep(500); - expectEquals(bas.toString().trim(), "log line"); - } + out.println("log line 1"); + out.println("log line 2"); + assertTrue(file.exists()); + Thread.sleep(100); + assertEquals(bas.toString(), + "log line 1" + System.lineSeparator() + "log line 2" + System.lineSeparator()); } - }); - test("Handle existing empty file which disappears later and comes back", () -> { - pending("Not a requirement"); + // Now delete file and give some time, then append new lines - ByteArrayOutputStream bas = new ByteArrayOutputStream(); - final PrintStream ps = new PrintStream(bas); - - final File file = File.createTempFile("tailer", ""); - expectTrue(file.exists()); + assertTrue(file.delete()); + Thread.sleep(100); try ( - final FileToStreamTailer tailer = new FileToStreamTailer(file, ps, 10); + PrintStream out = new PrintStream(new FileOutputStream(file)); ) { - tailer.start(); + out.println("new line"); + assertTrue(file.exists()); Thread.sleep(100); - - expectEquals(bas.toString(), ""); - - try ( - PrintStream out = new PrintStream(new FileOutputStream(file)); - ) { - out.println("log line 1"); - out.println("log line 2"); - expectTrue(file.exists()); - Thread.sleep(100); - expectEquals(bas.toString(), - "log line 1" + System.lineSeparator() + "log line 2" + System.lineSeparator()); - } - - // Now delete file and give some time, then append new lines - - expectTrue(file.delete()); - Thread.sleep(100); - - try ( - PrintStream out = new PrintStream(new FileOutputStream(file)); - ) { - out.println("new line"); - expectTrue(file.exists()); - Thread.sleep(100); - expectEquals(bas.toString(), - "log line 1" + System.lineSeparator() + - "log line 2" + System.lineSeparator() + - "new line" + System.lineSeparator()); - } - + assertEquals(bas.toString(), + "log line 1" + System.lineSeparator() + + "log line 2" + System.lineSeparator() + + "new line" + System.lineSeparator()); } - }); + } } } diff --git a/main/client/test/src/mill/main/client/MillEnvTests.java b/main/client/test/src/mill/main/client/MillEnvTests.java index bcb19366142..a744a7e8f92 100644 --- a/main/client/test/src/mill/main/client/MillEnvTests.java +++ b/main/client/test/src/mill/main/client/MillEnvTests.java @@ -1,31 +1,30 @@ package mill.main.client; -import de.tobiasroeser.lambdatest.junit.FreeSpec; + +import org.junit.Test; import java.io.File; import java.util.Arrays; import java.util.List; -import static de.tobiasroeser.lambdatest.Expect.expectEquals; - -public class MillEnvTests extends FreeSpec { - - @Override - protected void initTests() { - section("readOptsFileLines", () -> { - test("without final newline", () -> { - File file = new File( - getClass().getClassLoader().getResource("file-wo-final-newline.txt").toURI() - ); - List lines = Util.readOptsFileLines(file); - expectEquals( - lines, - Arrays.asList( - "-DPROPERTY_PROPERLY_SET_VIA_JVM_OPTS=value-from-file", - "-Xss120m" - )); - }); - }); - } +import static org.junit.Assert.assertEquals; + + +public class MillEnvTests { + @Test + public void readOptsFileLinesWithoutFInalNewline() throws Exception { + File file = new File( + getClass().getClassLoader().getResource("file-wo-final-newline.txt").toURI() + ); + List lines = Util.readOptsFileLines(file); + assertEquals( + lines, + Arrays.asList( + "-DPROPERTY_PROPERLY_SET_VIA_JVM_OPTS=value-from-file", + "-Xss120m" + )); + } } + + diff --git a/main/client/test/src/mill/main/client/RetryRule.java b/main/client/test/src/mill/main/client/RetryRule.java new file mode 100644 index 00000000000..c53dd332ad3 --- /dev/null +++ b/main/client/test/src/mill/main/client/RetryRule.java @@ -0,0 +1,36 @@ +// Taken from https://www.swtestacademy.com/rerun-failed-test-junit/ +package mill.main.client; +import java.util.Objects; +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; +public class RetryRule implements TestRule { + private int retryCount; + public RetryRule(int retryCount) { + this.retryCount = retryCount; + } + public Statement apply(Statement base, Description description) { + return statement(base, description); + } + private Statement statement(final Statement base, final Description description) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + Throwable caughtThrowable = null; + // implement retry logic here + for (int i = 0; i < retryCount; i++) { + try { + base.evaluate(); + return; + } + catch (Throwable t) { + caughtThrowable = t; + System.err.println(description.getDisplayName() + ": run " + (i + 1) + " failed."); + } + } + System.err.println(description.getDisplayName() + ": Giving up after " + retryCount + " failures."); + throw Objects.requireNonNull(caughtThrowable); + } + }; + } +} \ No newline at end of file diff --git a/main/package.mill b/main/package.mill index ec0a4152837..f7a2522255a 100644 --- a/main/package.mill +++ b/main/package.mill @@ -171,7 +171,6 @@ object `package` extends RootModule with build.MillStableScalaModule with BuildI object test extends JavaModuleTests with TestModule.Junit4 { def ivyDeps = Agg( build.Deps.junitInterface, - build.Deps.lambdaTest, build.Deps.commonsIO ) } diff --git a/main/util/src/mill/util/CoursierSupport.scala b/main/util/src/mill/util/CoursierSupport.scala index d8bc242dd99..4bf0c1c05bd 100644 --- a/main/util/src/mill/util/CoursierSupport.scala +++ b/main/util/src/mill/util/CoursierSupport.scala @@ -4,8 +4,9 @@ import coursier.cache.ArtifactError import coursier.parse.RepositoryParser import coursier.util.{Gather, Task} import coursier.{Dependency, Repository, Resolution} -import mill.api.{Ctx, PathRef, Result} +import mill.api.{Ctx, PathRef, Result, Retry} import mill.api.Loose.Agg + import java.io.File import scala.annotation.tailrec import scala.collection.mutable @@ -39,34 +40,25 @@ trait CoursierSupport { * @tparam T The result type of the computation * @return The result of the computation. If the computation was retries and finally succeeded, proviously occured errors will not be included in the result. */ - @tailrec private def retry[T]( retryCount: Int = CoursierRetryCount, debug: String => Unit, errorMsgExtractor: T => Seq[String] - )(f: () => T): T = { - val tried = Try(f()) - tried match { - case Failure(e) if retryCount > 0 && retryableCoursierError(e.getMessage) => - // this one is not detected by coursier itself, so we try-catch handle it - // I assume, this happens when another coursier thread already moved or rename dthe temporary file - debug(s"Attempting to retry coursier failure (${retryCount} left): ${e.getMessage}") - Thread.sleep(CoursierRetryWait) - retry(retryCount - 1, debug, errorMsgExtractor)(f) - - case Success(res) if retryCount > 0 => - val errors = errorMsgExtractor(res) - errors.filter(retryableCoursierError) match { - case Nil => res - case retryable => - for (r <- retryable) { - debug(s"Attempting to retry coursier failure (${retryCount} left): $r") - } - Thread.sleep(CoursierRetryWait) - retry(retryCount - 1, debug, errorMsgExtractor)(f) - } - - case r => r.get + )(f: () => T): T = Retry( + count = retryCount, + filter = { (i, ex) => + if (!retryableCoursierError(ex.getMessage))false + else { + debug(s"Attempting to retry coursier failure (${i} left): ${ex.getMessage}") + true + } + } + ) { + val res = f() + val errors = errorMsgExtractor(res) + errors.filter(retryableCoursierError) match { + case Nil => res + case retryable => throw new Exception(retryable.mkString("\n")) } } diff --git a/testkit/src/mill/testkit/ExampleTester.scala b/testkit/src/mill/testkit/ExampleTester.scala index c01490aff5e..33cd597b37b 100644 --- a/testkit/src/mill/testkit/ExampleTester.scala +++ b/testkit/src/mill/testkit/ExampleTester.scala @@ -1,4 +1,5 @@ package mill.testkit +import mill.api.Retry import mill.util.Util import utest._ @@ -75,32 +76,11 @@ class ExampleTester( millExecutable: os.Path, bashExecutable: String = ExampleTester.defaultBashExecutable() ) extends IntegrationTesterBase { - initWorkspace() os.copy.over(millExecutable, workspacePath / "mill") val testTimeout: FiniteDuration = 5.minutes - // Integration tests sometime hang on CI - // The idea is to just abort and retry them after a reasonable amount of time - @tailrec final def retryOnTimeout[T](n: Int)(body: => T): T = { - - // We use Java Future here, as it supports cancellation - val executor = Executors.newFixedThreadPool(1) - val fut = executor.submit { () => body } - - try fut.get(testTimeout.length, testTimeout.unit) - catch { - case e: TimeoutException => - fut.cancel(true) - if (n > 0) { - Console.err.println(s"Timeout occurred (${testTimeout}). Retrying..") - retryOnTimeout(n - 1)(body) - } else throw e - } - - } - def processCommandBlock(commandBlock: String): Unit = { val commandBlockLines = commandBlock.linesIterator.toVector @@ -212,13 +192,9 @@ class ExampleTester( val usageComment = parsed.collect { case ("example", txt) => txt }.mkString("\n\n") val commandBlocks = ("\n" + usageComment.trim).split("\n> ").filter(_.nonEmpty) - retryOnTimeout(3) { + Retry(count = 3, timeoutMillis = testTimeout.toMillis) { try { - try os.remove.all(workspacePath / "out") - catch { - case e: Throwable => /*do nothing*/ - } - + initWorkspace() for (commandBlock <- commandBlocks) processCommandBlock(commandBlock) } finally { if (clientServerMode) processCommand(Vector(), "./mill shutdown", check = false) diff --git a/testkit/src/mill/testkit/IntegrationTesterBase.scala b/testkit/src/mill/testkit/IntegrationTesterBase.scala index ab3a0e3b99b..9e7ac5ab9c3 100644 --- a/testkit/src/mill/testkit/IntegrationTesterBase.scala +++ b/testkit/src/mill/testkit/IntegrationTesterBase.scala @@ -1,5 +1,6 @@ package mill.testkit -import mill.main.client.OutFiles.{out, millWorker} +import mill.api.Retry +import mill.main.client.OutFiles.{millWorker, out} import mill.main.client.ServerFiles.serverId trait IntegrationTesterBase { @@ -18,9 +19,14 @@ trait IntegrationTesterBase { def initWorkspace(): Unit = { println(s"Copying integration test sources from $workspaceSourcePath to $workspacePath") os.makeDir.all(workspacePath) + Retry(){ + val tmp = os.temp.dir() + if (os.exists(workspacePath / out)) os.move.into(workspacePath / out, tmp) + os.remove.all(tmp) + } + os.list(workspacePath).foreach(os.remove.all(_)) os.list(workspaceSourcePath).filter(_.last != out).foreach(os.copy.into(_, workspacePath)) - os.remove.all(workspacePath / "out") } /** From 540c5487e479811cd6350e57c59959239b0e1687 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 11:52:42 +0800 Subject: [PATCH 03/14] . --- main/api/src/mill/api/Retry.scala | 20 ++++++----- testkit/src/mill/testkit/ExampleTester.scala | 34 +++++++++---------- .../mill/testkit/IntegrationTestSuite.scala | 25 +++++++++++--- .../testkit/IntegrationTestSuiteBase.scala | 18 ---------- .../src/mill/testkit/IntegrationTester.scala | 3 +- .../mill/testkit/IntegrationTesterBase.scala | 2 +- .../mill/testkit/UtestExampleTestSuite.scala | 12 ++++++- .../testkit/UtestIntegrationTestSuite.scala | 8 +++++ 8 files changed, 71 insertions(+), 51 deletions(-) delete mode 100644 testkit/src/mill/testkit/IntegrationTestSuiteBase.scala create mode 100644 testkit/src/mill/testkit/UtestIntegrationTestSuite.scala diff --git a/main/api/src/mill/api/Retry.scala b/main/api/src/mill/api/Retry.scala index 88a495cb25c..1f3cc6957fc 100644 --- a/main/api/src/mill/api/Retry.scala +++ b/main/api/src/mill/api/Retry.scala @@ -4,7 +4,11 @@ import java.util.concurrent.TimeUnit import scala.concurrent.duration.Duration import scala.concurrent.{Await, Promise} -object Retry { +case class Retry(count: Int = 5, + backoffMillis: Long = 10, + backoffMultiplier: Double = 2.0, + timeoutMillis: Long = -1, + filter: (Int, Throwable) => Boolean = (_, _) => true) { /** * Generic retry functionality * @@ -21,18 +25,18 @@ object Retry { * @return the value of evaluating [[t]], or throws an exception if evaluating * [[t]] fails more than [[count]] times */ - def apply[T](count: Int = 5, - backoffMillis: Long = 10, - backoffMultiplier: Double = 2.0, - timeoutMillis: Long = -1, - filter: (Int, Throwable) => Boolean = (_, _) => true)(t: => T): T = { + def apply[T](t: => T): T = { + indexed(i => t) + } + + def indexed[T](t: Int => T): T = { def rec(retryCount: Int, currentBackoffMillis: Long): T = { try { - if(timeoutMillis == -1) t + if(timeoutMillis == -1) t(retryCount) else{ val result = Promise[T] val thread = new Thread(() => { - result.complete(scala.util.Try(t)) + result.complete(scala.util.Try(t(retryCount))) }) thread.start() Await.result(result.future, Duration.apply(timeoutMillis, TimeUnit.MILLISECONDS)) diff --git a/testkit/src/mill/testkit/ExampleTester.scala b/testkit/src/mill/testkit/ExampleTester.scala index 33cd597b37b..7bdb7cd8097 100644 --- a/testkit/src/mill/testkit/ExampleTester.scala +++ b/testkit/src/mill/testkit/ExampleTester.scala @@ -55,14 +55,18 @@ object ExampleTester { clientServerMode: Boolean, workspaceSourcePath: os.Path, millExecutable: os.Path, - bashExecutable: String = defaultBashExecutable() - ): Unit = + bashExecutable: String = defaultBashExecutable(), + workspacePath: os.Path = os.pwd + ): Unit = { new ExampleTester( clientServerMode, workspaceSourcePath, millExecutable, - bashExecutable - ).run() + bashExecutable, + workspacePath + ) + } + def defaultBashExecutable(): String = { if (!mill.main.client.Util.isWindows) "bash" @@ -74,13 +78,10 @@ class ExampleTester( clientServerMode: Boolean, val workspaceSourcePath: os.Path, millExecutable: os.Path, - bashExecutable: String = ExampleTester.defaultBashExecutable() + bashExecutable: String = ExampleTester.defaultBashExecutable(), + val workspacePath: os.Path ) extends IntegrationTesterBase { - os.copy.over(millExecutable, workspacePath / "mill") - - val testTimeout: FiniteDuration = 5.minutes - def processCommandBlock(commandBlock: String): Unit = { val commandBlockLines = commandBlock.linesIterator.toVector @@ -188,18 +189,17 @@ class ExampleTester( } def run(): Any = { + os.copy.over(millExecutable, workspacePath / "mill") val parsed = ExampleParser(workspaceSourcePath) val usageComment = parsed.collect { case ("example", txt) => txt }.mkString("\n\n") val commandBlocks = ("\n" + usageComment.trim).split("\n> ").filter(_.nonEmpty) - Retry(count = 3, timeoutMillis = testTimeout.toMillis) { - try { - initWorkspace() - for (commandBlock <- commandBlocks) processCommandBlock(commandBlock) - } finally { - if (clientServerMode) processCommand(Vector(), "./mill shutdown", check = false) - removeServerIdFile() - } + try { + initWorkspace() + for (commandBlock <- commandBlocks) processCommandBlock(commandBlock) + } finally { + if (clientServerMode) processCommand(Vector(), "./mill shutdown", check = false) + removeServerIdFile() } } } diff --git a/testkit/src/mill/testkit/IntegrationTestSuite.scala b/testkit/src/mill/testkit/IntegrationTestSuite.scala index e488a51542f..fce86b782de 100644 --- a/testkit/src/mill/testkit/IntegrationTestSuite.scala +++ b/testkit/src/mill/testkit/IntegrationTestSuite.scala @@ -1,8 +1,23 @@ package mill.testkit -abstract class UtestIntegrationTestSuite extends utest.TestSuite with IntegrationTestSuite { - protected def workspaceSourcePath: os.Path = os.Path(sys.env("MILL_TEST_RESOURCE_FOLDER")) - protected def clientServerMode: Boolean = sys.env("MILL_INTEGRATION_SERVER_MODE").toBoolean - protected def millExecutable: os.Path = - os.Path(System.getenv("MILL_INTEGRATION_LAUNCHER"), os.pwd) +import os.Path + +trait IntegrationTestSuite { + protected def workspaceSourcePath: os.Path + protected def clientServerMode: Boolean + + protected def millExecutable: Path + + def debugLog: Boolean = false + def integrationTest[T](t: IntegrationTester => T): T = { + val tester = new IntegrationTester( + clientServerMode, + workspaceSourcePath, + millExecutable, + debugLog, + workspacePath = os.pwd + ) + try t(tester) + finally tester.close() + } } diff --git a/testkit/src/mill/testkit/IntegrationTestSuiteBase.scala b/testkit/src/mill/testkit/IntegrationTestSuiteBase.scala deleted file mode 100644 index 24abc745b62..00000000000 --- a/testkit/src/mill/testkit/IntegrationTestSuiteBase.scala +++ /dev/null @@ -1,18 +0,0 @@ -package mill.testkit - -import os.Path - -trait IntegrationTestSuite { - protected def workspaceSourcePath: os.Path - protected def clientServerMode: Boolean - - protected def millExecutable: Path - - def debugLog: Boolean = false - def integrationTest[T](t: IntegrationTester => T): T = { - val tester = - new IntegrationTester(clientServerMode, workspaceSourcePath, millExecutable, debugLog) - try t(tester) - finally tester.close() - } -} diff --git a/testkit/src/mill/testkit/IntegrationTester.scala b/testkit/src/mill/testkit/IntegrationTester.scala index 803ad5f6a06..b55bb335be0 100644 --- a/testkit/src/mill/testkit/IntegrationTester.scala +++ b/testkit/src/mill/testkit/IntegrationTester.scala @@ -22,7 +22,8 @@ class IntegrationTester( val clientServerMode: Boolean, val workspaceSourcePath: os.Path, val millExecutable: os.Path, - override val debugLog: Boolean = false + override val debugLog: Boolean = false, + val workspacePath: os.Path ) extends IntegrationTester.Impl { initWorkspace() } diff --git a/testkit/src/mill/testkit/IntegrationTesterBase.scala b/testkit/src/mill/testkit/IntegrationTesterBase.scala index 9e7ac5ab9c3..45f626cd0d9 100644 --- a/testkit/src/mill/testkit/IntegrationTesterBase.scala +++ b/testkit/src/mill/testkit/IntegrationTesterBase.scala @@ -11,7 +11,7 @@ trait IntegrationTesterBase { * Mill build being tested. Contains the `build.mill` file, any application code, and * the `out/` folder containing the build output */ - val workspacePath: os.Path = os.pwd + val workspacePath: os.Path /** * Initializes the workspace in preparation for integration testing diff --git a/testkit/src/mill/testkit/UtestExampleTestSuite.scala b/testkit/src/mill/testkit/UtestExampleTestSuite.scala index 994a4995a04..bbf7b316ba3 100644 --- a/testkit/src/mill/testkit/UtestExampleTestSuite.scala +++ b/testkit/src/mill/testkit/UtestExampleTestSuite.scala @@ -1,6 +1,9 @@ package mill.testkit +import mill.api.Retry import utest._ +import scala.concurrent.duration.DurationInt + object UtestExampleTestSuite extends TestSuite { val workspaceSourcePath: os.Path = os.Path(sys.env("MILL_TEST_RESOURCE_FOLDER")) val clientServerMode: Boolean = sys.env("MILL_INTEGRATION_SERVER_MODE").toBoolean @@ -9,7 +12,14 @@ object UtestExampleTestSuite extends TestSuite { val tests: Tests = Tests { test("exampleTest") { - new ExampleTester(clientServerMode, workspaceSourcePath, millExecutable).run() + Retry(count = 3, timeoutMillis = 5.minutes.toMillis).indexed { i => + ExampleTester.run( + clientServerMode, + workspaceSourcePath, + millExecutable, + workspacePath = os.pwd / s"run-$i" + ) + } } } } diff --git a/testkit/src/mill/testkit/UtestIntegrationTestSuite.scala b/testkit/src/mill/testkit/UtestIntegrationTestSuite.scala new file mode 100644 index 00000000000..e488a51542f --- /dev/null +++ b/testkit/src/mill/testkit/UtestIntegrationTestSuite.scala @@ -0,0 +1,8 @@ +package mill.testkit + +abstract class UtestIntegrationTestSuite extends utest.TestSuite with IntegrationTestSuite { + protected def workspaceSourcePath: os.Path = os.Path(sys.env("MILL_TEST_RESOURCE_FOLDER")) + protected def clientServerMode: Boolean = sys.env("MILL_INTEGRATION_SERVER_MODE").toBoolean + protected def millExecutable: os.Path = + os.Path(System.getenv("MILL_INTEGRATION_LAUNCHER"), os.pwd) +} From 3419ddbca1be7e28e35b6c0ddf2a4ae406c0429b Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 11:58:20 +0800 Subject: [PATCH 04/14] . --- .github/workflows/publish-artifacts.yml | 4 ++-- .github/workflows/publish-bridges.yml | 2 +- .github/workflows/publish-docs.yml | 2 +- .github/workflows/run-tests.yml | 24 ++++++++++++------------ 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/.github/workflows/publish-artifacts.yml b/.github/workflows/publish-artifacts.yml index 10adfde8e6f..7ba9d5e0b05 100644 --- a/.github/workflows/publish-artifacts.yml +++ b/.github/workflows/publish-artifacts.yml @@ -51,7 +51,7 @@ jobs: - uses: actions/setup-java@v4 with: - java-version: 11 + java-version: '11' distribution: temurin - run: ci/release-maven.sh @@ -73,7 +73,7 @@ jobs: - uses: actions/setup-java@v4 with: - java-version: 11 + java-version: '11' distribution: temurin - run: ./mill -i uploadToGithub $REPO_ACCESS_TOKEN diff --git a/.github/workflows/publish-bridges.yml b/.github/workflows/publish-bridges.yml index 1a5fd0ecac0..27e44d848aa 100644 --- a/.github/workflows/publish-bridges.yml +++ b/.github/workflows/publish-bridges.yml @@ -34,7 +34,7 @@ jobs: - uses: actions/setup-java@v4 with: - java-version: 8 + java-version: '11' distribution: temurin - run: ci/release-bridge-maven.sh diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index 8fcb95fe38b..78dc63b46ae 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -22,7 +22,7 @@ jobs: - uses: actions/setup-java@v4 with: - java-version: 8 + java-version: '11' distribution: temurin - run: ci/publish-docs.sh diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 8dc79491e29..e6f9dae0bf9 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -29,7 +29,7 @@ jobs: build-linux: uses: ./.github/workflows/run-mill-action.yml with: - java-version: 11 + java-version: '11' millargs: __.compile populate_cache: true @@ -37,7 +37,7 @@ jobs: uses: ./.github/workflows/run-mill-action.yml with: os: windows-latest - java-version: 11 + java-version: '11' millargs: __.compile populate_cache: true @@ -53,7 +53,7 @@ jobs: needs: build-linux uses: ./.github/workflows/run-mill-action.yml with: - java-version: '8' + java-version: '11' millargs: bridge.__.publishLocal env-bridge-versions: 'essential' @@ -71,7 +71,7 @@ jobs: matrix: include: # bootstrap tests - - java-version: 11 # Have one job on oldest JVM + - java-version: '11' # Have one job on oldest JVM buildcmd: ci/test-mill-dev.sh && ci/test-mill-release.sh && ./mill -i -k __.ivyDepsTree && ./mill -i -k __.ivyDepsTree --withRuntime - java-version: 17 # Have one job on default JVM buildcmd: ci/test-mill-bootstrap.sh @@ -94,7 +94,7 @@ jobs: # We also try to group tests together to manuaully balance out the runtimes of each jobs - java-version: 17 millargs: "'{main,scalalib,testrunner,bsp,testkit}.__.testCached'" - - java-version: 11 + - java-version: '11' millargs: "'{scalajslib,scalanativelib}.__.testCached'" - java-version: 17 millargs: "contrib.__.testCached" @@ -103,16 +103,16 @@ jobs: millargs: "'example.javalib.__.local.testCached'" - java-version: 17 millargs: "'example.scalalib.__.local.testCached'" - - java-version: 11 + - java-version: '11' millargs: "'example.thirdparty[{mockito,acyclic,commons-io}].local.testCached'" - java-version: 17 millargs: "'example.thirdparty[{fansi,jimfs,netty,gatling}].local.testCached'" - - java-version: 11 + - java-version: '11' millargs: "'example.{depth,extending}.__.local.testCached'" # Most of these integration tests should not depend on which mode they # are run in, so just run them in `local` - - java-version: 11 + - java-version: '11' millargs: "'integration.{failure,feature,ide}.__.local.testCached'" # These invalidation tests need to be exercised in both execution modes @@ -135,15 +135,15 @@ jobs: include: # just run a subset of examples/ on Windows, because for some reason running # the whole suite can take hours on windows v.s. half an hour on linux - - java-version: 11 + - java-version: '11' millargs: '"{main,scalalib,bsp}.__.testCached"' - - java-version: 11 + - java-version: '11' millargs: '"example.scalalib.{basic,web}.__.fork.testCached"' - java-version: 17 millargs: "'integration.{feature,failure}[_].fork.testCached'" - - java-version: 11 + - java-version: '11' millargs: "'integration.invalidation[_].server.testCached'" - - java-version: 11 + - java-version: '11' millargs: "contrib.__.testCached" uses: ./.github/workflows/run-mill-action.yml From 4a16875748b3d50144e105a6b8cfb97431e48b4d Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 12:01:06 +0800 Subject: [PATCH 05/14] . --- .github/workflows/run-mill-action.yml | 4 ++-- main/api/src/mill/api/Retry.scala | 21 ++++++++++--------- main/api/test/src/mill/api/RetryTests.scala | 15 ++++++------- main/util/src/mill/util/CoursierSupport.scala | 6 ++---- testkit/src/mill/testkit/ExampleTester.scala | 6 ------ .../mill/testkit/IntegrationTesterBase.scala | 2 +- 6 files changed, 24 insertions(+), 30 deletions(-) diff --git a/.github/workflows/run-mill-action.yml b/.github/workflows/run-mill-action.yml index 7b62509a032..2a07747d447 100644 --- a/.github/workflows/run-mill-action.yml +++ b/.github/workflows/run-mill-action.yml @@ -68,11 +68,11 @@ jobs: if: inputs.buildcmd != '' - name: Run Mill '${{ inputs.millargs }}' - run: ./mill -i -k ${{ inputs.millargs }} + run: ./mill -i -j2 -k ${{ inputs.millargs }} if: inputs.millargs != '' && !startsWith(inputs.os, 'windows') - name: Run Mill (on Windows) '${{ inputs.millargs }}' - run: cmd /C %GITHUB_WORKSPACE%\ci\mill.bat -i -k ${{ inputs.millargs }} + run: cmd /C %GITHUB_WORKSPACE%\ci\mill.bat -i -j2 -k ${{ inputs.millargs }} if: inputs.millargs != '' && startsWith(inputs.os, 'windows') - name: Run Mill (on Windows) Worker Cleanup diff --git a/main/api/src/mill/api/Retry.scala b/main/api/src/mill/api/Retry.scala index 1f3cc6957fc..2398d4e87e7 100644 --- a/main/api/src/mill/api/Retry.scala +++ b/main/api/src/mill/api/Retry.scala @@ -4,11 +4,14 @@ import java.util.concurrent.TimeUnit import scala.concurrent.duration.Duration import scala.concurrent.{Await, Promise} -case class Retry(count: Int = 5, - backoffMillis: Long = 10, - backoffMultiplier: Double = 2.0, - timeoutMillis: Long = -1, - filter: (Int, Throwable) => Boolean = (_, _) => true) { +case class Retry( + count: Int = 5, + backoffMillis: Long = 10, + backoffMultiplier: Double = 2.0, + timeoutMillis: Long = -1, + filter: (Int, Throwable) => Boolean = (_, _) => true +) { + /** * Generic retry functionality * @@ -32,8 +35,8 @@ case class Retry(count: Int = 5, def indexed[T](t: Int => T): T = { def rec(retryCount: Int, currentBackoffMillis: Long): T = { try { - if(timeoutMillis == -1) t(retryCount) - else{ + if (timeoutMillis == -1) t(retryCount) + else { val result = Promise[T] val thread = new Thread(() => { result.complete(scala.util.Try(t(retryCount))) @@ -41,8 +44,7 @@ case class Retry(count: Int = 5, thread.start() Await.result(result.future, Duration.apply(timeoutMillis, TimeUnit.MILLISECONDS)) } - } - catch { + } catch { case e: Throwable if retryCount < count && filter(retryCount + 1, e) => Thread.sleep(currentBackoffMillis) rec(retryCount + 1, (currentBackoffMillis * backoffMultiplier).toInt) @@ -52,4 +54,3 @@ case class Retry(count: Int = 5, rec(0, backoffMillis) } } - diff --git a/main/api/test/src/mill/api/RetryTests.scala b/main/api/test/src/mill/api/RetryTests.scala index bf59a1f4688..8ae4938db94 100644 --- a/main/api/test/src/mill/api/RetryTests.scala +++ b/main/api/test/src/mill/api/RetryTests.scala @@ -2,17 +2,17 @@ package mill.api import utest._ - object RetryTests extends TestSuite { val tests: Tests = Tests { test("fail") { var count = 0 try { - Retry(){ + Retry() { count += 1 throw new Exception("boom") } - } catch{case ex => + } catch { + case ex => assert(ex.getMessage == "boom") } @@ -20,7 +20,7 @@ object RetryTests extends TestSuite { } test("succeed") { var count = 0 - Retry(){ + Retry() { count += 1 if (count < 3) throw new Exception("boom") } @@ -34,13 +34,14 @@ object RetryTests extends TestSuite { case (i, ex: RuntimeException) => true case _ => false } - ){ + ) { count += 1 if (count < 3) throw new RuntimeException("boom") else throw new Exception("foo") } - } catch{case e: Exception => - assert(e.getMessage == "foo") + } catch { + case e: Exception => + assert(e.getMessage == "foo") } assert(count == 3) } diff --git a/main/util/src/mill/util/CoursierSupport.scala b/main/util/src/mill/util/CoursierSupport.scala index ce142e6a797..ceb2e09db9e 100644 --- a/main/util/src/mill/util/CoursierSupport.scala +++ b/main/util/src/mill/util/CoursierSupport.scala @@ -8,15 +8,13 @@ import mill.api.{Ctx, PathRef, Result, Retry} import mill.api.Loose.Agg import java.io.File -import scala.annotation.tailrec import scala.collection.mutable -import scala.util.{Failure, Success, Try} trait CoursierSupport { import CoursierSupport._ private val CoursierRetryCount = 5 - private val CoursierRetryWait = 100 + private def retryableCoursierError(s: String) = s match { case s"${_}concurrent download${_}" => true @@ -48,7 +46,7 @@ trait CoursierSupport { )(f: () => T): T = Retry( count = retryCount, filter = { (i, ex) => - if (!retryableCoursierError(ex.getMessage))false + if (!retryableCoursierError(ex.getMessage)) false else { debug(s"Attempting to retry coursier failure (${i} left): ${ex.getMessage}") true diff --git a/testkit/src/mill/testkit/ExampleTester.scala b/testkit/src/mill/testkit/ExampleTester.scala index 7bdb7cd8097..632b010e321 100644 --- a/testkit/src/mill/testkit/ExampleTester.scala +++ b/testkit/src/mill/testkit/ExampleTester.scala @@ -1,12 +1,7 @@ package mill.testkit -import mill.api.Retry import mill.util.Util import utest._ -import java.util.concurrent.{Executors, TimeoutException} -import scala.annotation.tailrec -import scala.concurrent.duration.DurationInt -import scala.concurrent.duration.FiniteDuration /** * A variant of [[IntegrationTester]], [[ExampleTester]] works the same way @@ -67,7 +62,6 @@ object ExampleTester { ) } - def defaultBashExecutable(): String = { if (!mill.main.client.Util.isWindows) "bash" else "C:\\Program Files\\Git\\usr\\bin\\bash.exe" diff --git a/testkit/src/mill/testkit/IntegrationTesterBase.scala b/testkit/src/mill/testkit/IntegrationTesterBase.scala index 45f626cd0d9..7a280be282e 100644 --- a/testkit/src/mill/testkit/IntegrationTesterBase.scala +++ b/testkit/src/mill/testkit/IntegrationTesterBase.scala @@ -19,7 +19,7 @@ trait IntegrationTesterBase { def initWorkspace(): Unit = { println(s"Copying integration test sources from $workspaceSourcePath to $workspacePath") os.makeDir.all(workspacePath) - Retry(){ + Retry() { val tmp = os.temp.dir() if (os.exists(workspacePath / out)) os.move.into(workspacePath / out, tmp) os.remove.all(tmp) From 4e29937da9d874242d02b7e9ea6d3def8bc7d47e Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 12:08:20 +0800 Subject: [PATCH 06/14] . --- main/util/src/mill/util/CoursierSupport.scala | 1 - testkit/src/mill/testkit/ExampleTester.scala | 6 +++--- .../src/mill/testkit/IntegrationTesterBase.scala | 14 ++++++++------ 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/main/util/src/mill/util/CoursierSupport.scala b/main/util/src/mill/util/CoursierSupport.scala index ceb2e09db9e..19631c63b50 100644 --- a/main/util/src/mill/util/CoursierSupport.scala +++ b/main/util/src/mill/util/CoursierSupport.scala @@ -14,7 +14,6 @@ trait CoursierSupport { import CoursierSupport._ private val CoursierRetryCount = 5 - private def retryableCoursierError(s: String) = s match { case s"${_}concurrent download${_}" => true diff --git a/testkit/src/mill/testkit/ExampleTester.scala b/testkit/src/mill/testkit/ExampleTester.scala index 632b010e321..536e8248186 100644 --- a/testkit/src/mill/testkit/ExampleTester.scala +++ b/testkit/src/mill/testkit/ExampleTester.scala @@ -2,7 +2,6 @@ package mill.testkit import mill.util.Util import utest._ - /** * A variant of [[IntegrationTester]], [[ExampleTester]] works the same way * except the commands used to test the project come from a `/** Usage ... */` @@ -59,7 +58,7 @@ object ExampleTester { millExecutable, bashExecutable, workspacePath - ) + ).run() } def defaultBashExecutable(): String = { @@ -183,13 +182,14 @@ class ExampleTester( } def run(): Any = { - os.copy.over(millExecutable, workspacePath / "mill") + os.makeDir.all(workspacePath) val parsed = ExampleParser(workspaceSourcePath) val usageComment = parsed.collect { case ("example", txt) => txt }.mkString("\n\n") val commandBlocks = ("\n" + usageComment.trim).split("\n> ").filter(_.nonEmpty) try { initWorkspace() + os.copy.over(millExecutable, workspacePath / "mill") for (commandBlock <- commandBlocks) processCommandBlock(commandBlock) } finally { if (clientServerMode) processCommand(Vector(), "./mill shutdown", check = false) diff --git a/testkit/src/mill/testkit/IntegrationTesterBase.scala b/testkit/src/mill/testkit/IntegrationTesterBase.scala index 7a280be282e..20826e009b2 100644 --- a/testkit/src/mill/testkit/IntegrationTesterBase.scala +++ b/testkit/src/mill/testkit/IntegrationTesterBase.scala @@ -33,12 +33,14 @@ trait IntegrationTesterBase { * Remove any ID files to try and force them to exit */ def removeServerIdFile(): Unit = { - val serverIdFiles = for { - outPath <- os.list.stream(workspacePath / out) - if outPath.last.startsWith(millWorker) - } yield outPath / serverId + if (os.exists(workspacePath / out)) { + val serverIdFiles = for { + outPath <- os.list.stream(workspacePath / out) + if outPath.last.startsWith(millWorker) + } yield outPath / serverId - serverIdFiles.foreach(os.remove(_)) - Thread.sleep(500) // give a moment for the server to notice the file is gone and exit + serverIdFiles.foreach(os.remove(_)) + Thread.sleep(500) // give a moment for the server to notice the file is gone and exit + } } } From da6db6f2bf380ac1d881c1b466263924c331f50f Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 12:12:05 +0800 Subject: [PATCH 07/14] . --- testkit/src/mill/testkit/IntegrationTester.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testkit/src/mill/testkit/IntegrationTester.scala b/testkit/src/mill/testkit/IntegrationTester.scala index b55bb335be0..1de1a9b152f 100644 --- a/testkit/src/mill/testkit/IntegrationTester.scala +++ b/testkit/src/mill/testkit/IntegrationTester.scala @@ -23,7 +23,7 @@ class IntegrationTester( val workspaceSourcePath: os.Path, val millExecutable: os.Path, override val debugLog: Boolean = false, - val workspacePath: os.Path + val workspacePath: os.Path = os.pwd ) extends IntegrationTester.Impl { initWorkspace() } From d0db6d95d5c908608c8a940c5d24ef55f6c1f2bf Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 12:18:42 +0800 Subject: [PATCH 08/14] . --- testkit/src/mill/testkit/ExampleTester.scala | 2 +- testkit/src/mill/testkit/IntegrationTestSuite.scala | 2 +- testkit/src/mill/testkit/IntegrationTester.scala | 2 +- testkit/src/mill/testkit/IntegrationTesterBase.scala | 10 +++++++++- testkit/src/mill/testkit/UtestExampleTestSuite.scala | 3 +-- 5 files changed, 13 insertions(+), 6 deletions(-) diff --git a/testkit/src/mill/testkit/ExampleTester.scala b/testkit/src/mill/testkit/ExampleTester.scala index 536e8248186..20b310a72d3 100644 --- a/testkit/src/mill/testkit/ExampleTester.scala +++ b/testkit/src/mill/testkit/ExampleTester.scala @@ -72,7 +72,7 @@ class ExampleTester( val workspaceSourcePath: os.Path, millExecutable: os.Path, bashExecutable: String = ExampleTester.defaultBashExecutable(), - val workspacePath: os.Path + val baseWorkspacePath: os.Path ) extends IntegrationTesterBase { def processCommandBlock(commandBlock: String): Unit = { diff --git a/testkit/src/mill/testkit/IntegrationTestSuite.scala b/testkit/src/mill/testkit/IntegrationTestSuite.scala index fce86b782de..d157c5a743b 100644 --- a/testkit/src/mill/testkit/IntegrationTestSuite.scala +++ b/testkit/src/mill/testkit/IntegrationTestSuite.scala @@ -15,7 +15,7 @@ trait IntegrationTestSuite { workspaceSourcePath, millExecutable, debugLog, - workspacePath = os.pwd + baseWorkspacePath = os.pwd ) try t(tester) finally tester.close() diff --git a/testkit/src/mill/testkit/IntegrationTester.scala b/testkit/src/mill/testkit/IntegrationTester.scala index 1de1a9b152f..01693ef0b4e 100644 --- a/testkit/src/mill/testkit/IntegrationTester.scala +++ b/testkit/src/mill/testkit/IntegrationTester.scala @@ -23,7 +23,7 @@ class IntegrationTester( val workspaceSourcePath: os.Path, val millExecutable: os.Path, override val debugLog: Boolean = false, - val workspacePath: os.Path = os.pwd + val baseWorkspacePath: os.Path = os.pwd ) extends IntegrationTester.Impl { initWorkspace() } diff --git a/testkit/src/mill/testkit/IntegrationTesterBase.scala b/testkit/src/mill/testkit/IntegrationTesterBase.scala index 20826e009b2..eb108041425 100644 --- a/testkit/src/mill/testkit/IntegrationTesterBase.scala +++ b/testkit/src/mill/testkit/IntegrationTesterBase.scala @@ -11,7 +11,15 @@ trait IntegrationTesterBase { * Mill build being tested. Contains the `build.mill` file, any application code, and * the `out/` folder containing the build output */ - val workspacePath: os.Path + val workspacePath: os.Path = { + Iterator + .iterate(1)(_+1) + .map(i => baseWorkspacePath / s"run-$i") + .find(!os.exists(_)) + .head + } + + val baseWorkspacePath: os.Path /** * Initializes the workspace in preparation for integration testing diff --git a/testkit/src/mill/testkit/UtestExampleTestSuite.scala b/testkit/src/mill/testkit/UtestExampleTestSuite.scala index bbf7b316ba3..3e7ab41a3d6 100644 --- a/testkit/src/mill/testkit/UtestExampleTestSuite.scala +++ b/testkit/src/mill/testkit/UtestExampleTestSuite.scala @@ -12,12 +12,11 @@ object UtestExampleTestSuite extends TestSuite { val tests: Tests = Tests { test("exampleTest") { - Retry(count = 3, timeoutMillis = 5.minutes.toMillis).indexed { i => + Retry(count = 3, timeoutMillis = 5.minutes.toMillis) { ExampleTester.run( clientServerMode, workspaceSourcePath, millExecutable, - workspacePath = os.pwd / s"run-$i" ) } } From 0672765c5256b630a1e0b9b6f16cc18e7c47726e Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 12:21:58 +0800 Subject: [PATCH 09/14] . --- .github/workflows/run-mill-action.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/run-mill-action.yml b/.github/workflows/run-mill-action.yml index 2a07747d447..6264143eacf 100644 --- a/.github/workflows/run-mill-action.yml +++ b/.github/workflows/run-mill-action.yml @@ -68,6 +68,7 @@ jobs: if: inputs.buildcmd != '' - name: Run Mill '${{ inputs.millargs }}' + # Mill tests are pretty heavy so run them only 2x parallel on 4 core Github Actions runners run: ./mill -i -j2 -k ${{ inputs.millargs }} if: inputs.millargs != '' && !startsWith(inputs.os, 'windows') From 6b487b01a102f5f8a07a9afc31cd1036b4a3e907 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 12:23:48 +0800 Subject: [PATCH 10/14] . --- testkit/src/mill/testkit/IntegrationTesterBase.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/testkit/src/mill/testkit/IntegrationTesterBase.scala b/testkit/src/mill/testkit/IntegrationTesterBase.scala index eb108041425..9494bfde8a1 100644 --- a/testkit/src/mill/testkit/IntegrationTesterBase.scala +++ b/testkit/src/mill/testkit/IntegrationTesterBase.scala @@ -10,6 +10,11 @@ trait IntegrationTesterBase { * The working directory of the integration test suite, which is the root of the * Mill build being tested. Contains the `build.mill` file, any application code, and * the `out/` folder containing the build output + * + * Each integration test that runs in the same [[baseWorkspacePath]] is given a new folder + * for isolation purposes; even though we try our best to clean up the processes and files + * from each Mill run, it still doesn't work 100%, and re-using the same folder can cause + * non-deterministic interference and flakiness */ val workspacePath: os.Path = { Iterator From 5114cddb5625c11cbe06617e6095fc9c1cc480c2 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 12:24:18 +0800 Subject: [PATCH 11/14] . --- .github/workflows/run-mill-action.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/run-mill-action.yml b/.github/workflows/run-mill-action.yml index 6264143eacf..9d667968e14 100644 --- a/.github/workflows/run-mill-action.yml +++ b/.github/workflows/run-mill-action.yml @@ -68,12 +68,12 @@ jobs: if: inputs.buildcmd != '' - name: Run Mill '${{ inputs.millargs }}' - # Mill tests are pretty heavy so run them only 2x parallel on 4 core Github Actions runners - run: ./mill -i -j2 -k ${{ inputs.millargs }} + # Mill tests are pretty heavy so run them only 3x parallel on 4 core Github Actions runners + run: ./mill -i -j3 -k ${{ inputs.millargs }} if: inputs.millargs != '' && !startsWith(inputs.os, 'windows') - name: Run Mill (on Windows) '${{ inputs.millargs }}' - run: cmd /C %GITHUB_WORKSPACE%\ci\mill.bat -i -j2 -k ${{ inputs.millargs }} + run: cmd /C %GITHUB_WORKSPACE%\ci\mill.bat -i -j3 -k ${{ inputs.millargs }} if: inputs.millargs != '' && startsWith(inputs.os, 'windows') - name: Run Mill (on Windows) Worker Cleanup From 07b76f68bc17ae89aa51e591a6887c2e2a9c62d6 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 12:26:18 +0800 Subject: [PATCH 12/14] . --- testkit/src/mill/testkit/IntegrationTesterBase.scala | 2 +- testkit/src/mill/testkit/UtestExampleTestSuite.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/testkit/src/mill/testkit/IntegrationTesterBase.scala b/testkit/src/mill/testkit/IntegrationTesterBase.scala index 9494bfde8a1..e3c25c641b6 100644 --- a/testkit/src/mill/testkit/IntegrationTesterBase.scala +++ b/testkit/src/mill/testkit/IntegrationTesterBase.scala @@ -18,7 +18,7 @@ trait IntegrationTesterBase { */ val workspacePath: os.Path = { Iterator - .iterate(1)(_+1) + .iterate(1)(_ + 1) .map(i => baseWorkspacePath / s"run-$i") .find(!os.exists(_)) .head diff --git a/testkit/src/mill/testkit/UtestExampleTestSuite.scala b/testkit/src/mill/testkit/UtestExampleTestSuite.scala index 3e7ab41a3d6..c2fac9be206 100644 --- a/testkit/src/mill/testkit/UtestExampleTestSuite.scala +++ b/testkit/src/mill/testkit/UtestExampleTestSuite.scala @@ -16,7 +16,7 @@ object UtestExampleTestSuite extends TestSuite { ExampleTester.run( clientServerMode, workspaceSourcePath, - millExecutable, + millExecutable ) } } From 05882ef6f094dbd3892645cbf14a05ce23dbdea1 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 12:43:39 +0800 Subject: [PATCH 13/14] . --- main/client/test/src/mill/main/client/ClientTests.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/main/client/test/src/mill/main/client/ClientTests.java b/main/client/test/src/mill/main/client/ClientTests.java index d8bf292e20b..7d810cdbd98 100644 --- a/main/client/test/src/mill/main/client/ClientTests.java +++ b/main/client/test/src/mill/main/client/ClientTests.java @@ -11,6 +11,10 @@ import java.util.*; public class ClientTests { + + @org.junit.Rule + public RetryRule retryRule = new RetryRule(3); + @Test public void readWriteInt() throws Exception{ int[] examples = { From 16035cec4973c1f188917372b360e2333d83f681 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 8 Sep 2024 15:45:06 +0800 Subject: [PATCH 14/14] . --- main/client/test/src/mill/main/client/ClientTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/client/test/src/mill/main/client/ClientTests.java b/main/client/test/src/mill/main/client/ClientTests.java index 7d810cdbd98..0466db7e2ec 100644 --- a/main/client/test/src/mill/main/client/ClientTests.java +++ b/main/client/test/src/mill/main/client/ClientTests.java @@ -61,7 +61,7 @@ public void checkStringRoundTrip(String example) throws Exception{ Util.writeString(o, example); ByteArrayInputStream i = new ByteArrayInputStream(o.toByteArray()); String s = Util.readString(i); - assertEquals(example, s, "String as bytes: ["+example.getBytes()+"] differs from expected: ["+s.getBytes()+"]"); + assertEquals(example, s); assertEquals(i.available(), 0); }