[Gradle, MPP] Rerun test tasks whose failures were suppressed

#KT-54506 Fixed

libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/AggregatingKotlinTestReportIT.kt

@@ -6,8 +6,9 @@
 package org.jetbrains.kotlin.gradle.mpp
 
 import org.gradle.util.GradleVersion
+import org.jetbrains.kotlin.gradle.native.MPPNativeTargets
 import org.jetbrains.kotlin.gradle.testbase.*
-import org.junit.jupiter.api.Disabled
+import org.jetbrains.kotlin.gradle.util.capitalize
 import org.junit.jupiter.api.DisplayName
 
 @MppGradlePluginTests
@@ -15,7 +16,6 @@ import org.junit.jupiter.api.DisplayName
 class AggregatingKotlinTestReportIT : KGPBaseTest() {
     @DisplayName("KT-54506: `allTests` is not false positively up-to-date after failure")
     @GradleTest
-    @Disabled
     fun testFailedTestsAreNotUpToDate(gradleVersion: GradleVersion) {
         project(
             "new-mpp-lib-with-tests",
@@ -27,6 +27,9 @@ class AggregatingKotlinTestReportIT : KGPBaseTest() {
                 it.replace("expectedFun()", "assertEquals(0, 1)")
             }
 
+            val nativeTarget = MPPNativeTargets.current
+            val capitalizedNativeTarget = nativeTarget.capitalize()
+
             buildAndFail(":allTests") {
                 // expect only an aggregate test task fail
                 assertTasksExecuted(
@@ -36,6 +39,9 @@ class AggregatingKotlinTestReportIT : KGPBaseTest() {
                     ":compileKotlinJvmWithoutJava",
                     ":compileTestKotlinJvmWithoutJava",
                     ":jvmWithoutJavaTest",
+                    ":compileKotlin$capitalizedNativeTarget",
+                    ":compileTestKotlin$capitalizedNativeTarget",
+                    ":${nativeTarget}Test"
                 )
                 assertTasksFailed(":allTests")
             }
@@ -47,16 +53,37 @@ class AggregatingKotlinTestReportIT : KGPBaseTest() {
                     ":compileTestKotlinJvmWithoutJava",
                     ":compileKotlinJs",
                     ":compileTestKotlinJs",
+                    ":compileKotlin$capitalizedNativeTarget",
+                    ":compileTestKotlin$capitalizedNativeTarget",
                 )
                 assertTasksExecuted(
                     ":jvmWithoutJavaTest",
                     ":jsTest",
+                    ":${nativeTarget}Test",
+                )
+                assertTasksFailed(":allTests")
+            }
+
+            buildAndFail(":allTests") {
+                // still expect only an aggregate test task fail
+                assertTasksUpToDate(
+                    ":compileKotlinJvmWithoutJava",
+                    ":compileTestKotlinJvmWithoutJava",
+                    ":compileKotlinJs",
+                    ":compileTestKotlinJs",
+                    ":compileKotlin$capitalizedNativeTarget",
+                    ":compileTestKotlin$capitalizedNativeTarget",
+                )
+                assertTasksExecuted(
+                    ":jvmWithoutJavaTest",
+                    ":jsTest",
+                    ":${nativeTarget}Test",
                 )
                 assertTasksFailed(":allTests")
             }
 
             buildAndFail(":jvmWithoutJavaTest") {
-                // expect a single test task still fails after an aggregate test task fail
+                // expect that a single test task invocation also still fails
                 assertTasksUpToDate(
                     ":compileKotlinJvmWithoutJava",
                     ":compileTestKotlinJvmWithoutJava",

libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/util/stringUtil.kt

@@ -1,5 +1,7 @@
 package org.jetbrains.kotlin.gradle.util
 
+import java.util.*
+
 fun String.addBeforeSubstring(prefix: String, substring: String): String =
     replace(substring, prefix + substring)
 
@@ -7,3 +9,5 @@ fun String.checkedReplace(original: String, replacement: String): String {
     check(contains(original)) { "Substring '$original' is not found in '$this'" }
     return replace(original, replacement)
 }
+
+fun String.capitalize() = replaceFirstChar { if (it.isLowerCase()) it.titlecase(Locale.getDefault()) else it.toString() }

libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/KotlinTestReport.kt

@@ -7,6 +7,7 @@ package org.jetbrains.kotlin.gradle.testing.internal
 
 import org.gradle.api.GradleException
 import org.gradle.api.execution.TaskExecutionGraph
+import org.gradle.api.provider.Property
 import org.gradle.api.provider.Provider
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.Internal
@@ -42,7 +43,7 @@ import java.net.URI
  * In this case, only topmost aggregate test task will override reporting,
  * event if child tasks will be executed.
  */
-open class KotlinTestReport : TestReport() {
+abstract class KotlinTestReport : TestReport() {
     @Transient
     @Internal
     val testTasks = mutableListOf<AbstractTestTask>()
@@ -57,12 +58,9 @@ open class KotlinTestReport : TestReport() {
     @Transient
     val children = mutableListOf<TaskProvider<KotlinTestReport>>()
 
-    @Transient
-    private val projectProperties = PropertiesProvider(project)
-
     @get:Input
     val overrideReporting: Boolean by lazy {
-        !projectProperties.individualTaskReports
+        !PropertiesProvider(project).individualTaskReports
     }
 
     @Input
@@ -71,15 +69,14 @@ open class KotlinTestReport : TestReport() {
     @Input
     var ignoreFailures: Boolean = false
 
-    private val testReportServiceProvider = TestReportService.registerIfAbsent(project.gradle)
+    @get:Internal
+    internal abstract val testReportServiceProvider: Property<TestReportService>
     private val testReportService
         get() = testReportServiceProvider.get()
 
     private val hasFailedTests: Boolean
         get() = testReportService.hasFailedTests(path)
 
-    private val failedTestsListener = FailedTestListener(parentPaths, testReportServiceProvider)
-
     private fun computeAllParentTasksPaths(): List<String> {
         val allParents = mutableListOf<String>()
         var cur: KotlinTestReport? = this
@@ -114,7 +111,7 @@ open class KotlinTestReport : TestReport() {
     fun registerTestTask(task: AbstractTestTask) {
         testTasks.add(task)
 
-        task.addTestListener(failedTestsListener)
+        task.addTestListener(FailedTestListener(task.path, parentPaths, testReportServiceProvider))
         if (task is KotlinTest) {
             val listener = SuppressedTestRunningFailureListener(parentPaths, task.path, testReportServiceProvider)
             task.addRunListener(listener)
@@ -257,6 +254,7 @@ open class KotlinTestReport : TestReport() {
     }
 
     private class FailedTestListener(
+        private val testTaskPath: String,
         private val allListenedTaskParentsPaths: Provider<List<String>>,
         private val testReportServiceProvider: Provider<TestReportService>
     ) : TestListener {
@@ -275,7 +273,7 @@ open class KotlinTestReport : TestReport() {
         private fun reportFailure(result: TestResult) {
             if (result.failedTestCount > 0) {
                 allListenedTaskParentsPaths.get().forEach {
-                    testReportServiceProvider.get().testFailed(it)
+                    testReportServiceProvider.get().testFailed(it, testTaskPath)
                 }
             }
         }

libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/KotlinTestsRegistry.kt

@@ -12,6 +12,7 @@ import org.gradle.api.plugins.JavaBasePlugin
 import org.gradle.api.tasks.TaskProvider
 import org.gradle.api.tasks.testing.AbstractTestTask
 import org.gradle.language.base.plugins.LifecycleBasePlugin
+import org.jetbrains.kotlin.gradle.logging.kotlinInfo
 import org.jetbrains.kotlin.gradle.tasks.dependsOn
 import org.jetbrains.kotlin.gradle.tasks.locateOrRegisterTask
 import org.jetbrains.kotlin.gradle.tasks.registerTask
@@ -37,10 +38,21 @@ class KotlinTestsRegistry(val project: Project, val allTestsTaskName: String = "
     ) {
         project.tasks.named(LifecycleBasePlugin.CHECK_TASK_NAME).dependsOn(taskHolder)
         project.cleanAllTestTask.configure { it.dependsOn(cleanTaskName(taskHolder.name)) }
+        val testReportService = TestReportService.registerIfAbsent(project)
         aggregate.configure {
             it.dependsOn(taskHolder.name)
             it.registerTestTask(taskHolder.get())
         }
+        taskHolder.configure { task ->
+            task.usesService(testReportService)
+            task.outputs.upToDateWhen {
+                val hasFailedPreviously = testReportService.get().hasTestTaskFailedPreviously(it.path)
+                if (hasFailedPreviously) {
+                    task.logger.kotlinInfo("Marking $it as not up-to-date because the task has failed previously")
+                }
+                !hasFailedPreviously
+            }
+        }
 
         taskHolder.configure { task ->
             ijListenTestTask(task)
@@ -67,7 +79,7 @@ class KotlinTestsRegistry(val project: Project, val allTestsTaskName: String = "
         if (existed != null) return existed
 
         val reportName = name
-
+        val testReportService = TestReportService.registerIfAbsent(project)
         val aggregate: TaskProvider<KotlinTestReport> = project.registerTask(name) { aggregate ->
             aggregate.description = description
             aggregate.group = JavaBasePlugin.VERIFICATION_GROUP
@@ -79,6 +91,8 @@ class KotlinTestsRegistry(val project: Project, val allTestsTaskName: String = "
             if (isIdeaActive) {
                 aggregate.extensions.extraProperties.set("idea.internal.test", true)
             }
+            aggregate.usesService(testReportService)
+            aggregate.testReportServiceProvider.value(testReportService).finalizeValueOnRead()
 
             project.gradle.taskGraph.whenReady { graph ->
                 aggregate.maybeOverrideReporting(graph)

libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/TestReportService.kt

@@ -5,41 +5,109 @@
 
 package org.jetbrains.kotlin.gradle.testing.internal
 
-import org.gradle.api.invocation.Gradle
+import org.gradle.api.Project
+import org.gradle.api.file.RegularFileProperty
+import org.gradle.api.logging.Logging
 import org.gradle.api.provider.Provider
 import org.gradle.api.services.BuildService
 import org.gradle.api.services.BuildServiceParameters
+import java.io.*
+import java.util.concurrent.ConcurrentHashMap
 
-typealias TaskError = Pair<String, Error>
+internal typealias TaskError = Pair<String, Error>
 
-abstract class TestReportService : BuildService<BuildServiceParameters.None> {
+/**
-    private val taskHasFailedTests = mutableMapOf<String, Boolean>()
+ * A build service required for correct test failures detection in [KotlinTestReport] as it requires cross-task interaction.
-    private val taskSuppressedFailures = mutableMapOf<String, MutableList<TaskError>>()
+ */
-
+internal abstract class TestReportService : BuildService<TestReportService.TestReportServiceParameters>, AutoCloseable {
-    fun testFailed(taskPath: String) {
+    internal interface TestReportServiceParameters : BuildServiceParameters {
-        taskHasFailedTests[taskPath] = true
+        val testTasksStateFile: RegularFileProperty
     }
 
+    private val log = Logging.getLogger(this.javaClass)
+    private val previouslyFailedTestTasks = readPreviouslyFailedTasks()
+    private val reportHasFailedTests = ConcurrentHashMap<String, Boolean>()
+    private val testTaskSuppressedFailures = ConcurrentHashMap<String, MutableList<TaskError>>()
+
+    /**
+     * Marks [KotlinTestReport] with [reportTaskPath] as a report containing failed tests during the build.
+     * [testTaskPath] is a path of the actual test task with failed tests.
+     */
+    fun testFailed(reportTaskPath: String, testTaskPath: String) {
+        reportHasFailedTests[reportTaskPath] = true
+        previouslyFailedTestTasks.add(testTaskPath)
+    }
+
+    /**
+     * Checks whether [KotlinTestReport] defined by [path] contains any children test tasks that failed during the build
+     */
     fun hasFailedTests(path: String): Boolean {
-        return taskHasFailedTests[path] ?: false
+        return reportHasFailedTests[path] ?: false
     }
 
+    /**
+     * Reports a test task execution failure (not test failure).
+     * @param failedTaskPath is a path of the failed test task
+     * @param parentTaskPath is a path of a [KotlinTestReport] that the task reports to
+     */
     fun reportFailure(failedTaskPath: String, parentTaskPath: String, failure: Error) {
-        taskSuppressedFailures.computeIfAbsent(parentTaskPath) { mutableListOf() }.add(failedTaskPath to failure)
+        testTaskSuppressedFailures.computeIfAbsent(parentTaskPath) { mutableListOf() }.add(failedTaskPath to failure)
     }
 
+    /**
+     * Returns all the test task execution failures (not test failures) related to the [KotlinTestReport] defined by [taskPath]
+     */
     fun getAggregatedTaskFailures(taskPath: String): List<TaskError> {
-        return taskSuppressedFailures[taskPath] ?: emptyList()
+        return testTaskSuppressedFailures[taskPath] ?: emptyList()
+    }
+
+    override fun close() {
+        writePreviouslyFailedTasks()
+    }
+
+    /**
+     * Checks whether the test task defined by [path] had failed previously (doesn't matter if it's caused by failed test or any runtime problem).
+     * This function is not idempotent as it resets the task's failed state.
+     */
+    fun hasTestTaskFailedPreviously(path: String) = previouslyFailedTestTasks.remove(path)
+
+    private val binaryStateFile: File
+        get() = parameters.testTasksStateFile.get().asFile
+
+    private fun readPreviouslyFailedTasks(): MutableSet<String> {
+        val failedTasksSet: MutableSet<String> = ConcurrentHashMap.newKeySet()
+        try {
+            ObjectInputStream(FileInputStream(binaryStateFile)).use {
+                @Suppress("UNCHECKED_CAST")
+                failedTasksSet.addAll(it.readObject() as Set<String>)
+            }
+        } catch (e: Exception) {
+            log.error("Cannot read test tasks state from $binaryStateFile", e)
+        }
+        return failedTasksSet
+    }
+
+    private fun writePreviouslyFailedTasks() {
+        previouslyFailedTestTasks += testTaskSuppressedFailures.values.flatMap { taskErrors -> taskErrors.map { (taskPath, _) -> taskPath } }
+        binaryStateFile.parentFile.mkdirs()
+        try {
+            ObjectOutputStream(FileOutputStream(binaryStateFile)).use {
+                it.writeObject(previouslyFailedTestTasks.toSet())
+            }
+        } catch (e: Exception) {
+            log.error("Cannot store test tasks state into $binaryStateFile", e)
+        }
     }
 
     companion object {
-        fun registerIfAbsent(gradle: Gradle): Provider<TestReportService> {
+        fun registerIfAbsent(project: Project): Provider<TestReportService> {
-            // Use class loader hashcode in case there are multiple class loaders in the same build
+            return project.gradle.sharedServices
-            return gradle.sharedServices
                 .registerIfAbsent(
-                    "${TestReportService::class.java.canonicalName}_${TestReportService::class.java.classLoader.hashCode()}",
+                    "${TestReportService::class.java.canonicalName}_${project.path}",
                     TestReportService::class.java
-                ) {}
+                ) { spec ->
+                    spec.parameters.testTasksStateFile.set(project.buildDir.resolve("test-results/kotlin-test-tasks-state.bin"))
+                }
         }
     }
 }