diff --git a/include-build/roborazzi-gradle-plugin/src/main/java/io/github/takahirom/roborazzi/RoborazziPlugin.kt b/include-build/roborazzi-gradle-plugin/src/main/java/io/github/takahirom/roborazzi/RoborazziPlugin.kt index c7b175cc..73fe9335 100644 --- a/include-build/roborazzi-gradle-plugin/src/main/java/io/github/takahirom/roborazzi/RoborazziPlugin.kt +++ b/include-build/roborazzi-gradle-plugin/src/main/java/io/github/takahirom/roborazzi/RoborazziPlugin.kt @@ -13,12 +13,8 @@ import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.api.Task import org.gradle.api.file.DirectoryProperty -import org.gradle.api.logging.Logging import org.gradle.api.model.ObjectFactory import org.gradle.api.provider.Property -import org.gradle.api.provider.Provider -import org.gradle.api.services.BuildService -import org.gradle.api.services.BuildServiceParameters import org.gradle.api.tasks.InputDirectory import org.gradle.api.tasks.Optional import org.gradle.api.tasks.OutputDirectory @@ -26,14 +22,11 @@ import org.gradle.api.tasks.TaskAction import org.gradle.api.tasks.options.Option import org.gradle.api.tasks.testing.AbstractTestTask import org.gradle.api.tasks.testing.Test +import org.gradle.api.tasks.testing.TestDescriptor +import org.gradle.api.tasks.testing.TestListener +import org.gradle.api.tasks.testing.TestResult import org.gradle.build.event.BuildEventsListenerRegistry import org.gradle.language.base.plugins.LifecycleBasePlugin.VERIFICATION_GROUP -import org.gradle.tooling.events.FinishEvent -import org.gradle.tooling.events.OperationCompletionListener -import org.gradle.tooling.events.task.TaskFailureResult -import org.gradle.tooling.events.task.TaskFinishEvent -import org.gradle.tooling.events.task.TaskSkippedResult -import org.gradle.tooling.events.task.TaskSuccessResult import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension import org.jetbrains.kotlin.gradle.plugin.ExecutionTaskHolder import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinNativeTargetWithTests @@ -41,8 +34,6 @@ import org.jetbrains.kotlin.gradle.targets.jvm.KotlinJvmTarget import org.jetbrains.kotlin.gradle.targets.native.KotlinNativeBinaryTestRun import org.jetbrains.kotlin.gradle.targets.native.tasks.KotlinNativeTest import java.util.Locale -import java.util.concurrent.CountDownLatch -import java.util.concurrent.TimeUnit import javax.inject.Inject import kotlin.reflect.KClass @@ -129,21 +120,8 @@ abstract class RoborazziPlugin : Plugin { fun configureRoborazziTasks( variantSlug: String, testTaskName: String, - testTaskSkipEventsServiceProvider: Provider, testTaskClass: KClass = Test::class ) { - try { - testTaskSkipEventsServiceProvider.get().addExpectingTestTaskName(testTaskName) - } catch (e: ClassCastException) { - throw IllegalStateException( - """You should use `id("io.github.takahirom.roborazzi") version "[version]" apply false` in the root project - |to ensure the build cache property functions correctly. - |This is a temporary workaround, - |and we are awaiting a permanent fix from the Gradle core. - |https://github.com/takahirom/roborazzi/issues/266""".trimMargin(), - e - ) - } val testTaskOutputDirForEachVariant: DirectoryProperty = project.objects.directoryProperty() val intermediateDirForEachVariant = testTaskOutputDirForEachVariant.convention( @@ -229,6 +207,8 @@ abstract class RoborazziPlugin : Plugin { val reportFileProperty = project.layout.buildDirectory.file(RoborazziReportConst.reportFilePathFromBuildDir) + // The difference between finalizedTask and afterSuite is that + // finalizedTask is called even if the test is skipped. val finalizeTestRoborazziTask = project.tasks.register( /* name = */ "finalizeTestRoborazzi$variantSlug", /* configurationAction = */ object : Action { @@ -244,32 +224,13 @@ abstract class RoborazziPlugin : Plugin { project.path + ":" } finalizeTestTask.doLast { - val taskSkipEventsService = testTaskSkipEventsServiceProvider.get() - val buildFinishCountDownLatch = taskSkipEventsService.countDownLatch - val currentIsTestSkipped = - taskSkipEventsService.skippedTestTaskMap[taskPath + testTaskName] - val isTestSkipped = if (currentIsTestSkipped == null) { - // BuildService could cause race condition - // https://github.com/gradle/gradle/issues/24887 - finalizeTestTask.infoln("Roborazzi: roborazziTestFinalizer.doLast test task result doesn't exits. currentIsTestSkipped:$currentIsTestSkipped currentTimeMillis:${System.currentTimeMillis()}") - val waitSuccess = buildFinishCountDownLatch.await(200, TimeUnit.MILLISECONDS) - val new = - taskSkipEventsService.skippedTestTaskMap[taskPath + testTaskName] - finalizeTestTask.infoln("Roborazzi: roborazziTestFinalizer.doLast wait end currentIsTestSkipped:$currentIsTestSkipped new:$new waitSuccess:$waitSuccess currentTimeMillis:${System.currentTimeMillis()}") - new ?: false - } else { - currentIsTestSkipped - } - finalizeTestTask.infoln("Roborazzi: roborazziTestFinalizer.doLast isTestSkipped:$isTestSkipped") - if (isTestSkipped) { - // If the test is skipped, we need to use cached files - finalizeTestTask.infoln("Roborazzi: finalizeTestRoborazziTask isTestSkipped:$isTestSkipped Copy files from ${intermediateDir.get()} to ${outputDir.get()}") - intermediateDir.get().asFile.mkdirs() - intermediateDir.get().asFile.copyRecursively( - target = outputDir.get().asFile, - overwrite = true - ) - } + val startCopy = System.currentTimeMillis() + intermediateDir.get().asFile.mkdirs() + intermediateDir.get().asFile.copyRecursively( + target = outputDir.get().asFile, + overwrite = true + ) + finalizeTestTask.infoln("Roborazzi: finalizeTestRoborazziTask Copy files from ${intermediateDir.get()} to ${outputDir.get()} end ${System.currentTimeMillis() - startCopy}ms") val results: List = resultDirFileTree.get().mapNotNull { if (it.name.endsWith(".json")) { @@ -298,7 +259,7 @@ abstract class RoborazziPlugin : Plugin { } }) testTaskProvider - .configureEach { test -> + .configureEach { test: AbstractTestTask -> val resultsDir = resultDirFileProperty.get().asFile if (restoreOutputDirRoborazziTaskProvider.isPresent) { test.inputs.dir(restoreOutputDirRoborazziTaskProvider.map { @@ -381,19 +342,37 @@ abstract class RoborazziPlugin : Plugin { resultsDir.deleteRecursively() resultsDir.mkdirs() } - test.doLast { - // Copy all files from outputDir to intermediateDir - // so that we can use Gradle's output caching - it.infoln("Roborazzi: test.doLast Copy files from ${outputDir.get()} to ${intermediateDir.get()}") - // outputDir.get().asFileTree.forEach { - // println("Copy file ${finalizeTask.absolutePath} to ${intermediateDir.get()}") - // } - outputDir.get().asFile.mkdirs() - outputDir.get().asFile.copyRecursively( - target = intermediateDir.get().asFile, - overwrite = true - ) - } + test.addTestListener(object : TestListener { + override fun beforeSuite(suite: TestDescriptor?) { + // Do nothing + } + + // The reason why we use afterSuite() instead of task.doLast is that + // task.doLast is not called if the test is failed. + // And why we use afterSuite() instead of finalizedBy is that + // we want to change the tasks' output in the task execution phase. + override fun afterSuite(suite: TestDescriptor?, result: TestResult?) { + // Copy all files from outputDir to intermediateDir + // so that we can use Gradle's output caching + test.infoln("Roborazzi: test.doLast Copy files from ${outputDir.get()} to ${intermediateDir.get()}") + // outputDir.get().asFileTree.forEach { + // println("Copy file ${finalizeTask.absolutePath} to ${intermediateDir.get()}") + // } + outputDir.get().asFile.mkdirs() + outputDir.get().asFile.copyRecursively( + target = intermediateDir.get().asFile, + overwrite = true + ) + } + + override fun beforeTest(testDescriptor: TestDescriptor?) { + // Do nothing + } + + override fun afterTest(testDescriptor: TestDescriptor?, result: TestResult?) { + // Do nothing + } + }) test.finalizedBy(finalizeTestRoborazziTask) } @@ -403,14 +382,6 @@ abstract class RoborazziPlugin : Plugin { verifyAndRecordTaskProvider.configure { it.dependsOn(testTaskProvider) } } - val testTaskSkipEventsServiceProvider: Provider = - project.gradle.sharedServices.registerIfAbsent( - "roborazziTestTaskEvents", TestTaskSkipEventsServiceProvider::class.java - ) { spec -> - // do nothing - } - getEventsListenerRegistry().onTaskCompletion(testTaskSkipEventsServiceProvider) - fun AndroidComponentsExtension<*, *, *>.configureComponents() { onVariants { variant -> val unitTest = variant.unitTest ?: return@onVariants @@ -421,7 +392,6 @@ abstract class RoborazziPlugin : Plugin { configureRoborazziTasks( variantSlug = variantSlug, testTaskName = "test$testVariantSlug", - testTaskSkipEventsServiceProvider = testTaskSkipEventsServiceProvider ) } } @@ -439,7 +409,6 @@ abstract class RoborazziPlugin : Plugin { configureRoborazziTasks( variantSlug = "Jvm", testTaskName = "test", - testTaskSkipEventsServiceProvider = testTaskSkipEventsServiceProvider ) } project.pluginManager.withPlugin("org.jetbrains.kotlin.multiplatform") { @@ -455,7 +424,6 @@ abstract class RoborazziPlugin : Plugin { configureRoborazziTasks( variantSlug = target.name.capitalizeUS(), testTaskName = testRun.executionTask.name, - testTaskSkipEventsServiceProvider = testTaskSkipEventsServiceProvider ) } } @@ -465,7 +433,6 @@ abstract class RoborazziPlugin : Plugin { configureRoborazziTasks( variantSlug = target.name.capitalizeUS(), testTaskName = (testRun as ExecutionTaskHolder<*>).executionTask.name, - testTaskSkipEventsServiceProvider = testTaskSkipEventsServiceProvider, testTaskClass = KotlinNativeTest::class ) } @@ -509,66 +476,5 @@ abstract class RoborazziPlugin : Plugin { } } -/** - * We can't get whether the test is skipped or not from the test task itself - * because of the configuration cache - */ -abstract class TestTaskSkipEventsServiceProvider : BuildService, - OperationCompletionListener { - val skippedTestTaskMap = mutableMapOf() - var countDownLatch = CountDownLatch(1) - private set - private val logger = Logging.getLogger(this::class.java) - private val expectingTestNames = mutableListOf() - fun addExpectingTestTaskName(testName: String) { - expectingTestNames.add(testName) - } - - override fun onFinish(finishEvent: FinishEvent) { - val displayName = finishEvent.displayName -// println( -// "Roborazzi: onFinish " + -// "expectingTestNames:$expectingTestNames" + -// "displayName:$displayName " + -// "finishEvent:$finishEvent " + -// "finishEvent.descriptor:${finishEvent.descriptor}" + -// "finishEvent.descriptor.name:${finishEvent.descriptor.name}" -// ) - if (finishEvent !is TaskFinishEvent) { - return - } - if (!expectingTestNames.any { - displayName.contains(it, ignoreCase = true) - }) { - return - } - val result = finishEvent.result - if (when (result) { - is TaskSuccessResult -> { - if (result.isFromCache) { - true - } else if (result.isUpToDate) { - true - } else { - false - } - } - - is TaskFailureResult -> false - is TaskSkippedResult -> true - else -> false - } - ) { - logger.info("Roborazzi: Skip test ${finishEvent.descriptor.name} ${System.currentTimeMillis()}") - skippedTestTaskMap[finishEvent.descriptor.name] = true - } else { - logger.info("Roborazzi: Don't skip test ${finishEvent.descriptor.name} ${System.currentTimeMillis()}") - skippedTestTaskMap[finishEvent.descriptor.name] = false - } - countDownLatch.countDown() - countDownLatch = CountDownLatch(1) - } -} - fun Task.infoln(format: String) = logger.info(format)