[2.x] Add test result cache opt-out (#9675)

* [2.x] Add test result cache opt-out

* Update main/src/main/scala/sbt/internal/SysProp.scala

Co-authored-by: eugene yokota <[email protected]>

* Use Boolean values for test cache opt-out

---------

Co-authored-by: eugene yokota <[email protected]>

main-actions/src/main/scala/sbt/Tests.scala

@@ -48,14 +48,23 @@ object Tests {
   private[sbt] sealed trait AdhocOption
   private[sbt] object AdhocOption:
     case class Summary(summary: TestSummary) extends AdhocOption
+    case class CacheTestResult(enabled: Boolean) extends AdhocOption
 
     private[sbt] def parser: Parser[AdhocOption] =
       import DefaultParsers.*
       val value = token("--test_summary=") ~> token(NotSpace.examples("none", "failure", "success"))
-      value.flatMap: v =>
+      val summary = value.flatMap: v =>
         Tests.parseTestSummary(v) match
           case Some(ts) => Parser.success(Summary(ts))
           case None     => Parser.failure(s"Invalid test_summary value: $v")
+      val cacheValue =
+        token("--cache_test_result=") ~> token(NotSpace.examples("true", "false"))
+      val cache = cacheValue.flatMap: v =>
+        v.toLowerCase(Locale.ENGLISH) match
+          case "true"  => Parser.success(CacheTestResult(true))
+          case "false" => Parser.success(CacheTestResult(false))
+          case _       => Parser.failure(s"Invalid cache_test_result value: $v")
+      summary | cache
   end AdhocOption
 
   private[sbt] def parseTestSummary(value: String): Option[TestSummary] =

main/src/main/scala/sbt/Defaults.scala

@@ -1484,30 +1484,39 @@ object Defaults extends BuildCommon with DefExtra {
   private lazy val inputTests0: Initialize[InputTask[TestResult]] = {
     val parser =
       loadForParser(definedTestNames)((s, i) => testOnlyParserWithOption(s, i getOrElse Nil))
-    ParserGen(parser).flatMapTask { (selected, frameworkOptions, adhocOptions) =>
+    ParserGen(parser).flatMapTask { parsed =>
+      val (selected, frameworkOptions, adhocOptions) = parsed
       val s = streams.value
       val filter = testFilter.value
       val config = testExecution.value
+      val cacheTestResult = adhocOptions
+        .collectFirst { case Tests.AdhocOption.CacheTestResult(enabled) =>
+          enabled
+        }
+        .getOrElse(SysProp.cacheTestResult)
       val st = state.value
       given display: Show[ScopedKey[?]] = Project.showContextKey(st)
       val modifiedOpts =
         Tests.ExplicitlyRequestedNames(selected) +:
           Tests.Filters(
-            filter(
-              selected ++ (if frameworkOptions.nonEmpty then Seq("--") ++ frameworkOptions else Nil)
+            IncrementalTest.withCacheTestResult(cacheTestResult)(
+              filter(
+                selected ++ (if frameworkOptions.nonEmpty then Seq("--") ++ frameworkOptions
+                             else Nil)
+              )
             )
           ) +:
           Tests.Argument(frameworkOptions*) +: config.options
-      if frameworkOptions.nonEmpty then
-        modifiedOpts.foreach: opt =>
-          opt match
-            case Tests.Listeners(listeners) =>
-              listeners.toList.foreach: l =>
-                l match
-                  case r: TestStatusReporter =>
-                    r.setArguments(frameworkOptions)
-                  case _ => ()
-            case _ => ()
+      modifiedOpts.foreach: opt =>
+        opt match
+          case Tests.Listeners(listeners) =>
+            listeners.toList.foreach: l =>
+              l match
+                case r: TestStatusReporter =>
+                  r.setCacheTestResult(cacheTestResult)
+                  if frameworkOptions.nonEmpty then r.setArguments(frameworkOptions)
+                case _ => ()
+          case _ => ()
       val newConfig = config.copy(options = modifiedOpts)
       val output = allTestGroupsTask(
         s,
@@ -1536,6 +1545,7 @@ object Defaults extends BuildCommon with DefExtra {
             out.events.keySet,
             selected,
             frameworkOptions,
+            cacheTestResult,
           )
           TestSummary.append(taskName, out, cached, adhocOptions.toVector)
           try

main/src/main/scala/sbt/internal/IncrementalTest.scala

@@ -23,9 +23,15 @@ import sbt.util.CacheImplicits
 import sbt.util.CacheImplicits.given
 import scala.collection.concurrent
 import scala.collection.mutable
+import scala.util.DynamicVariable
 import xsbti.{ FileConverter, HashedVirtualFileRef, VirtualFileRef }
 
 object IncrementalTest:
+  private val cacheTestResultOverride = DynamicVariable[Option[Boolean]](None)
+
+  private[sbt] def withCacheTestResult[A](enabled: Boolean)(f: => A): A =
+    cacheTestResultOverride.withValue(Some(enabled))(f)
+
   def filterTask: Initialize[Task[Seq[String] => Seq[String => Boolean]]] =
     Def.task {
       val cp = (Keys.test / fullClasspath).value
@@ -34,11 +40,16 @@ object IncrementalTest:
       def hasCachedSuccess(ts: Digest, options: Seq[String]): Boolean =
         val input = cacheInput(ts, options)
         ActionCache.exists(input._1, input._2, input._3, config)
-      def hasSucceeded(className: String, options: Seq[String]): Boolean =
-        digests.get(className) match
+      def hasSucceeded(
+          className: String,
+          options: Seq[String],
+          cacheTestResult: Boolean
+      ): Boolean =
+        cacheTestResult && (digests.get(className) match
           case None     => false
-          case Some(ts) => hasCachedSuccess(ts, options)
+          case Some(ts) => hasCachedSuccess(ts, options))
       args =>
+        val cacheTestResult = cacheTestResultOverride.value.getOrElse(SysProp.cacheTestResult)
         val (pattern, options) =
           args.indexOf("--") match
             case idx if idx >= 0 =>
@@ -46,7 +57,7 @@ object IncrementalTest:
               (s1, s2.drop(1))
             case _ => (args, Nil)
         for filter <- selectedFilter(pattern)
-        yield (test: String) => filter(test) && !hasSucceeded(test, options)
+        yield (test: String) => filter(test) && !hasSucceeded(test, options, cacheTestResult)
     }
 
   // cache the test digests against the fullClasspath.
@@ -110,11 +121,13 @@ object IncrementalTest:
       executed: Set[String],
       selected: Seq[String],
       frameworkOptions: Seq[String],
+      cacheTestResult: Boolean,
   ): Vector[String] =
     val filters = selectedFilter(selected)
     digests.iterator
       .collect {
-        case (name, ts) if !executed.contains(name) && filters.exists(_(name)) && {
+        case (name, ts)
+            if cacheTestResult && !executed.contains(name) && filters.exists(_(name)) && {
               val input = cacheInput(ts, frameworkOptions)
               ActionCache.exists(input._1, input._2, input._3, config)
             } =>
@@ -132,9 +145,12 @@ private[sbt] case class TestStatusReporter(
   // int value to represent success
   private final val successfulTest = 0
   private var _args: Seq[String] = Nil
+  private var _cacheTestResult: Boolean = SysProp.cacheTestResult
   def getArgs: Seq[String] = _args
   def setArguments(args: Seq[String]): Unit =
     _args = args
+  def setCacheTestResult(enabled: Boolean): Unit =
+    _cacheTestResult = enabled
   def doInit(): Unit = ()
   def startGroup(name: String): Unit = ()
   def testEvent(event: TestEvent): Unit = ()
@@ -145,7 +161,7 @@ private[sbt] case class TestStatusReporter(
    * using its unique digest, so we can skip the test later.
    */
   def endGroup(name: String, result: TestResult): Unit =
-    if result == TestResult.Passed then
+    if result == TestResult.Passed && _cacheTestResult then
       digests.get(name) match
         case Some(ts) =>
           // treat each test suite as a successful action that returns 0

main/src/main/scala/sbt/internal/SysProp.scala

@@ -124,6 +124,9 @@ object SysProp:
   def testSummary: TestSummary =
     strOpt("sbt.test_summary").flatMap(Tests.parseTestSummary).getOrElse(TestSummary.default)
 
+  def cacheTestResult: Boolean =
+    getOrTrue("sbt.cache_test_result")
+
   /**
    * Indicates whether formatting has been disabled in environment variables.
    * 1. -Dsbt.log.noformat=true means no formatting.

sbt-app/src/sbt-test/tests/cache-test-result/build.sbt

@@ -0,0 +1,26 @@
+scalaVersion := "2.12.21"
+
+libraryDependencies += "org.scalatest" %% "scalatest" % "3.0.5" % Test
+
+Test / testQuick / testFilter ~= { filter => args => filter(args) }
+
+commands ++= Seq(
+  Command.command("disableTestResultCache") { state =>
+    System.setProperty("sbt.cache_test_result", "false")
+    state
+  },
+  Command.command("enableTestResultCache") { state =>
+    System.clearProperty("sbt.cache_test_result")
+    state
+  },
+  Command.command("replaceTestFilter") { state =>
+    Project.extract(state).appendWithSession(
+      Seq(
+        Test / testQuick / testFilter := Def.uncached(
+          (_: Seq[String]) => Seq((_: String) => true)
+        )
+      ),
+      state
+    )
+  }
+)

sbt-app/src/sbt-test/tests/cache-test-result/changes/CacheTest.scala

@@ -0,0 +1,10 @@
+import java.io.File
+
+import org.scalatest.FunSuite
+
+class CacheTest extends FunSuite {
+  test("record changed execution") {
+    val count = Iterator.from(1).find(i => !new File(s"run-$i").exists).get
+    assert(new File(s"run-$count").createNewFile())
+  }
+}

sbt-app/src/sbt-test/tests/cache-test-result/changes/CacheTestAgain.scala

@@ -0,0 +1,10 @@
+import java.io.File
+
+import org.scalatest.FunSuite
+
+class CacheTest extends FunSuite {
+  test("record another changed execution") {
+    val count = Iterator.from(1).find(i => !new File(s"run-$i").exists).get
+    assert(new File(s"run-$count").createNewFile())
+  }
+}

sbt-app/src/sbt-test/tests/cache-test-result/src/test/scala/CacheTest.scala

@@ -0,0 +1,10 @@
+import java.io.File
+
+import org.scalatest.FunSuite
+
+class CacheTest extends FunSuite {
+  test("record execution") {
+    val count = Iterator.from(1).find(i => !new File(s"run-$i").exists).get
+    assert(new File(s"run-$count").createNewFile())
+  }
+}

sbt-app/src/sbt-test/tests/cache-test-result/test

@@ -0,0 +1,37 @@
+> test
+> test
+$ exists run-1
+$ absent run-2
+
+> test --cache_test_result=false
+> test --cache_test_result=false
+$ exists run-2
+$ exists run-3
+
+$ copy-file changes/CacheTest.scala src/test/scala/CacheTest.scala
+> disableTestResultCache
+> test --cache_test_result=true
+> test --cache_test_result=true
+> enableTestResultCache
+> test
+$ exists run-4
+$ absent run-5
+
+$ copy-file changes/CacheTestAgain.scala src/test/scala/CacheTest.scala
+> disableTestResultCache
+> test
+> test
+> enableTestResultCache
+$ exists run-5
+$ exists run-6
+
+> test
+> test
+$ exists run-7
+$ absent run-8
+
+> replaceTestFilter
+> test
+> test
+$ exists run-8
+$ exists run-9

server-test/src/test/scala/testpkg/ClientTest.scala

@@ -176,7 +176,8 @@ class ClientTest extends AbstractServerTest with BeforeAndAfterEach {
     )
     assert(complete("testOnly") == testOnlyExpected)
 
-    val testOnlyOptionsExpected = Vector("--", "--test_summary=", ";", "test.pkg.FooSpec")
+    val testOnlyOptionsExpected =
+      Vector("--", "--cache_test_result=", "--test_summary=", ";", "test.pkg.FooSpec")
     assert(complete("testOnly ") == testOnlyOptionsExpected)
   }
   test("quote with semi") {