From 1d08116a2e08f56ebe5238dd57b7c8621708999d Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Thu, 30 Jul 2020 14:43:22 -0700 Subject: [PATCH] Stop instrumenting scripted The commit 388ed641fbf89227f3cb19e3dd0f18d1be6ee9aa added an autoplugin that was compiled for every scripted test. Compiling autoplugins introduces a fair bit of overhead because it can easily take 3-4 seconds to compile with a cold compiler and even a warn compiler takes a second or so. Removing the instrumentation caused 3 tests to fail: 1. genereated-root-no-publish relied on setUpScripted modifing the project name. Explicitly setting the name in the project build.sbt fixed it. 2. cp-order I'm not sure why this change broke that test, but changing the coursier classpath ordering setting does not automatically trigger a reload on the next update. I have a more involved change that makes changing coursier settings invalidate the update cache but I'm markign the test as pending for now. It could be fixed by adding a call to `update` after `set csrConfiguration ~= (_.withClasspathOrder(false))` but I think it's better that the test actually reflect the expected behavior until I push the fix. 3. auto-plugins there was a hack that seemed added to address https://github.com/sbt/sbt/issues/3164. I cannot tell from either the issue or the linked PR what was going on and since removing the lines that were explicitly commented as being temporary fixed it, I figured it was ok to remove them. This reverts commit e01f5f5ef11240fcc1c63d750ef32a664a5aefb7. --- .../cp-order/{test => pending} | 0 .../sbt-test/project/auto-plugins/build.sbt | 4 +- .../generated-root-no-publish/build.sbt | 1 + .../sbt/scriptedtest/ScriptedTests.scala | 44 +------------------ 4 files changed, 4 insertions(+), 45 deletions(-) rename sbt/src/sbt-test/dependency-management/cp-order/{test => pending} (100%) diff --git a/sbt/src/sbt-test/dependency-management/cp-order/test b/sbt/src/sbt-test/dependency-management/cp-order/pending similarity index 100% rename from sbt/src/sbt-test/dependency-management/cp-order/test rename to sbt/src/sbt-test/dependency-management/cp-order/pending diff --git a/sbt/src/sbt-test/project/auto-plugins/build.sbt b/sbt/src/sbt-test/project/auto-plugins/build.sbt index 242d2b51e..a4471b3c0 100644 --- a/sbt/src/sbt-test/project/auto-plugins/build.sbt +++ b/sbt/src/sbt-test/project/auto-plugins/build.sbt @@ -52,9 +52,9 @@ check := { same(ddel, None, "del in projD") // val buildValue = (demo in ThisBuild).value - same(buildValue, "build 1", "demo in ThisBuild") // this is temporary, should be 0 until # is fixed + same(buildValue, "build 0", "demo in ThisBuild") val globalValue = (demo in Global).value - same(globalValue, "global 1", "demo in Global") // this is temporary, should be 0 until # is fixed + same(globalValue, "global 0", "demo in Global") val projValue = (demo in projC).?.value same(projValue, Some("project projC Q R"), "demo in projC") val qValue = (del in projC in Quux).?.value diff --git a/sbt/src/sbt-test/project/generated-root-no-publish/build.sbt b/sbt/src/sbt-test/project/generated-root-no-publish/build.sbt index 8c8845224..4526e32d4 100644 --- a/sbt/src/sbt-test/project/generated-root-no-publish/build.sbt +++ b/sbt/src/sbt-test/project/generated-root-no-publish/build.sbt @@ -7,4 +7,5 @@ val commonSettings = Seq( lazy val app = (project in file("app")). settings(commonSettings: _*) +name := "generated-root-no-publish" commonSettings diff --git a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala index f0721bdf9..f2984e40d 100644 --- a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala +++ b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala @@ -20,7 +20,6 @@ import RemoteSbtCreatorProp._ import scala.collection.parallel.ForkJoinTaskSupport import scala.collection.{ GenSeq, mutable } -import scala.util.Try import scala.util.control.NonFatal final class ScriptedTests( @@ -173,40 +172,6 @@ final class ScriptedTests( } else _ => false - /** Defines an auto plugin that is injected to sbt between every scripted session. - * - * It sets the name of the local root project for those tests run in batch mode. - * - * This is necessary because the current design to run tests in batch mode forces - * scripted tests to share one common sbt dir instead of each one having its own. - * - * Sbt extracts the local root project name from the directory name. So those - * scripted tests that don't set the name for the root and whose test files check - * information based on the name will fail. - * - * The reason why we set the name here and not via `set` is because some tests - * dump the session to check that their settings have been correctly applied. - * - * @param testName The test name used to extract the root project name. - * @return A string-based implementation to run between every reload. - */ - private def createAutoPlugin(testName: String) = - s""" - |import sbt._, Keys._ - |object InstrumentScripted extends AutoPlugin { - | override def trigger = allRequirements - | override def globalSettings: Seq[Setting[_]] = - | Seq(commands += setUpScripted) ++ super.globalSettings - | - | def setUpScripted = Command.command("setUpScripted") { (state0: State) => - | val nameScriptedSetting = name.in(LocalRootProject).:=( - | if (name.value.startsWith("sbt_")) "$testName" else name.value) - | val state1 = Project.extract(state0).appendWithoutSession(nameScriptedSetting, state0) - | "initialize" :: state1 - | } - |} - """.stripMargin - /** Defines the batch execution of scripted tests. * * Scripted tests are run one after the other one recycling the handlers, under @@ -249,20 +214,13 @@ final class ScriptedTests( val runTest = () => { // Reload and initialize (to reload contents of .sbtrc files) - val pluginImplementation = createAutoPlugin(name) - val pluginFile = tempTestDir / "project" / "InstrumentScripted.scala" - IO.write(pluginFile, pluginImplementation) def sbtHandlerError = sys error "Missing sbt handler. Scripted is misconfigured." val sbtHandler = handlers.getOrElse('>', sbtHandlerError) - val commandsToRun = ";reload;setUpScripted" - val statement = Statement(commandsToRun, Nil, successExpected = true, line = -1) + val statement = Statement("reload;initialize", Nil, successExpected = true, line = -1) // Run reload inside the hook to reuse error handling for pending tests val wrapHook = (file: File) => { preHook(file) - while (!Try(IO.read(pluginFile)).toOption.contains(pluginImplementation)) { - IO.write(pluginFile, pluginImplementation) - } try runner.processStatement(sbtHandler, statement, states) catch { case t: Throwable =>