Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix output image consistency issue #366

Merged
merged 5 commits into from
May 24, 2024
Merged

Fix output image consistency issue #366

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a4b20de
Check integrationt tests when removing isskiptest in Plugin
takahirom May 18, 2024
75a1380
Try to use TestListener instead of doLast of task
takahirom May 18, 2024
d984b7f
Remove isTestSkipped completely
takahirom May 18, 2024
d84be71
Add comments
takahirom May 18, 2024
5392f08
Remove some comments
takahirom May 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 44 additions & 138 deletions ...ld/roborazzi-gradle-plugin/src/main/java/io/github/takahirom/roborazzi/RoborazziPlugin.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,27 @@ 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
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
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

Expand Down Expand Up @@ -129,21 +120,8 @@ abstract class RoborazziPlugin : Plugin<Project> {
fun configureRoborazziTasks(
variantSlug: String,
testTaskName: String,
testTaskSkipEventsServiceProvider: Provider<TestTaskSkipEventsServiceProvider>,
testTaskClass: KClass<out AbstractTestTask> = 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(
Expand Down Expand Up @@ -229,6 +207,8 @@ abstract class RoborazziPlugin : Plugin<Project> {
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<Task> {
Expand All @@ -244,32 +224,13 @@ abstract class RoborazziPlugin : Plugin<Project> {
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<CaptureResult> = resultDirFileTree.get().mapNotNull {
if (it.name.endsWith(".json")) {
Expand Down Expand Up @@ -298,7 +259,7 @@ abstract class RoborazziPlugin : Plugin<Project> {
}
})
testTaskProvider
.configureEach { test ->
.configureEach { test: AbstractTestTask ->
val resultsDir = resultDirFileProperty.get().asFile
if (restoreOutputDirRoborazziTaskProvider.isPresent) {
test.inputs.dir(restoreOutputDirRoborazziTaskProvider.map {
Expand Down Expand Up @@ -381,19 +342,37 @@ abstract class RoborazziPlugin : Plugin<Project> {
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)
}

Expand All @@ -403,14 +382,6 @@ abstract class RoborazziPlugin : Plugin<Project> {
verifyAndRecordTaskProvider.configure { it.dependsOn(testTaskProvider) }
}

val testTaskSkipEventsServiceProvider: Provider<TestTaskSkipEventsServiceProvider> =
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
Expand All @@ -421,7 +392,6 @@ abstract class RoborazziPlugin : Plugin<Project> {
configureRoborazziTasks(
variantSlug = variantSlug,
testTaskName = "test$testVariantSlug",
testTaskSkipEventsServiceProvider = testTaskSkipEventsServiceProvider
)
}
}
Expand All @@ -439,7 +409,6 @@ abstract class RoborazziPlugin : Plugin<Project> {
configureRoborazziTasks(
variantSlug = "Jvm",
testTaskName = "test",
testTaskSkipEventsServiceProvider = testTaskSkipEventsServiceProvider
)
}
project.pluginManager.withPlugin("org.jetbrains.kotlin.multiplatform") {
Expand All @@ -455,7 +424,6 @@ abstract class RoborazziPlugin : Plugin<Project> {
configureRoborazziTasks(
variantSlug = target.name.capitalizeUS(),
testTaskName = testRun.executionTask.name,
testTaskSkipEventsServiceProvider = testTaskSkipEventsServiceProvider
)
}
}
Expand All @@ -465,7 +433,6 @@ abstract class RoborazziPlugin : Plugin<Project> {
configureRoborazziTasks(
variantSlug = target.name.capitalizeUS(),
testTaskName = (testRun as ExecutionTaskHolder<*>).executionTask.name,
testTaskSkipEventsServiceProvider = testTaskSkipEventsServiceProvider,
testTaskClass = KotlinNativeTest::class
)
}
Expand Down Expand Up @@ -509,66 +476,5 @@ abstract class RoborazziPlugin : Plugin<Project> {
}
}

/**
* 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<BuildServiceParameters.None?>,
OperationCompletionListener {
val skippedTestTaskMap = mutableMapOf<String, Boolean>()
var countDownLatch = CountDownLatch(1)
private set
private val logger = Logging.getLogger(this::class.java)
private val expectingTestNames = mutableListOf<String>()
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)
Loading