diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/AggregatingKotlinTestReportIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/AggregatingKotlinTestReportIT.kt index 0ad6f96d416..f31d051e903 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/AggregatingKotlinTestReportIT.kt +++ b/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", diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/util/stringUtil.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/util/stringUtil.kt index dc7f17b456b..6c6ca7a4460 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/util/stringUtil.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/util/stringUtil.kt @@ -1,9 +1,13 @@ package org.jetbrains.kotlin.gradle.util +import java.util.* + fun String.addBeforeSubstring(prefix: String, substring: String): String = replace(substring, prefix + substring) fun String.checkedReplace(original: String, replacement: String): String { check(contains(original)) { "Substring '$original' is not found in '$this'" } return replace(original, replacement) -} \ No newline at end of file +} + +fun String.capitalize() = replaceFirstChar { if (it.isLowerCase()) it.titlecase(Locale.getDefault()) else it.toString() } \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/KotlinTestReport.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/KotlinTestReport.kt index f9e7b5b3d25..0e88369c2b8 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/KotlinTestReport.kt +++ b/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() @@ -57,12 +58,9 @@ open class KotlinTestReport : TestReport() { @Transient val children = mutableListOf>() - @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 private val testReportService get() = testReportServiceProvider.get() private val hasFailedTests: Boolean get() = testReportService.hasFailedTests(path) - private val failedTestsListener = FailedTestListener(parentPaths, testReportServiceProvider) - private fun computeAllParentTasksPaths(): List { val allParents = mutableListOf() 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>, private val testReportServiceProvider: Provider ) : 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) } } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/KotlinTestsRegistry.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/KotlinTestsRegistry.kt index 5e81a0ae5b3..9a463cd2a41 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/KotlinTestsRegistry.kt +++ b/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 = 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) diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/TestReportService.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/TestReportService.kt index 5755704187a..21d45cd5a54 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/testing/internal/TestReportService.kt +++ b/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 +internal typealias TaskError = Pair -abstract class TestReportService : BuildService { - private val taskHasFailedTests = mutableMapOf() - private val taskSuppressedFailures = mutableMapOf>() - - fun testFailed(taskPath: String) { - taskHasFailedTests[taskPath] = true +/** + * A build service required for correct test failures detection in [KotlinTestReport] as it requires cross-task interaction. + */ +internal abstract class TestReportService : BuildService, AutoCloseable { + internal interface TestReportServiceParameters : BuildServiceParameters { + val testTasksStateFile: RegularFileProperty } + private val log = Logging.getLogger(this.javaClass) + private val previouslyFailedTestTasks = readPreviouslyFailedTasks() + private val reportHasFailedTests = ConcurrentHashMap() + private val testTaskSuppressedFailures = ConcurrentHashMap>() + + /** + * 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 { - 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 { + val failedTasksSet: MutableSet = ConcurrentHashMap.newKeySet() + try { + ObjectInputStream(FileInputStream(binaryStateFile)).use { + @Suppress("UNCHECKED_CAST") + failedTasksSet.addAll(it.readObject() as Set) + } + } 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 { - // Use class loader hashcode in case there are multiple class loaders in the same build - return gradle.sharedServices + fun registerIfAbsent(project: Project): Provider { + return project.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")) + } } } } \ No newline at end of file