From 9472a30f803414879bea2c88e00ed510a98f6358 Mon Sep 17 00:00:00 2001 From: Daniel Jette Date: Fri, 2 Oct 2026 11:17:13 -0400 Subject: [PATCH 1/4] 238: Report an actionable error when the install task cannot be found --- CHANGELOG.md | 4 + .../testify/tasks/main/ScreenshotTestTask.kt | 58 ++++++- .../tasks/main/InstallTaskResolutionTest.kt | 145 ++++++++++++++++++ 3 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index ce5cb028..c94c55e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ - Fix `RootViewWithoutFocusException` being intermittently thrown by `Espresso.closeSoftKeyboard()` - **Breaking:** `dev.testify.internal.helpers.closeSoftKeyboard()` now requires an `Activity` parameter - Fix `ClassCastException` when passing `-PshardCount` and `-PshardIndex` to `screenshotTest` or `screenshotRecord` +- `screenshotTest` and `screenshotRecord` now fail with an actionable message when the install task + they depend on cannot be resolved, instead of dropping the dependency silently. The message names + the path searched, the `moduleName` used and whether it was inferred. Modules that legitimately + have no install task, such as an Android library or a `com.android.test` module, are unaffected ## 6.0.0 diff --git a/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt b/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt index 1a5bda8d..f3068027 100644 --- a/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt +++ b/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt @@ -31,6 +31,7 @@ import dev.testify.internal.Style.Failure import dev.testify.internal.TestOptionsBuilder import dev.testify.internal.fromEnv import dev.testify.internal.println +import dev.testify.GradleExtensionException import dev.testify.tasks.internal.TaskDependencyProvider import dev.testify.tasks.internal.TaskNameProvider import dev.testify.tasks.internal.TestifyDefaultTask @@ -205,7 +206,60 @@ open class ScreenshotTestTask : TestifyDefaultTask() { } internal fun getInstallDebugAndroidTestTask(project: Project): Task? = - project.tasks.findByPath(":${project.testifySettings.moduleName}:${project.testifySettings.installAndroidTestTask}") + project.findInstallTask( + taskName = project.testifySettings.installAndroidTestTask, + settingName = "installAndroidTestTask" + ) internal fun getInstallDebugTask(project: Project): Task? = - project.tasks.findByPath(":${project.testifySettings.moduleName}:${project.testifySettings.installTask}") + project.findInstallTask( + taskName = project.testifySettings.installTask, + settingName = "installTask" + ) + +/** + * Resolve an install task by its full Gradle path, built from the configured `moduleName`. + * + * Returns `null` when [taskName] is `null`, which is the legitimate case for a module that has no + * install task at all — an Android library, a `com.android.test` module, or a JVM-only module such + * as the Paparazzi sample. Those modules are expected to have nothing to depend on. + * + * When [taskName] is known but the path does not resolve, the configuration is wrong rather than + * absent, and the task would otherwise be dropped silently — leaving the tests to run against + * whatever happened to be installed on the device, or failing much later with Gradle's + * `A dependency must not be empty`. This is the case reported in + * [#238](https://github.com/ndtp/android-testify/issues/238): under a composite build the inferred + * `moduleName` of `project.name` is not the module's Gradle path, so the lookup cannot succeed. + */ +private fun Project.findInstallTask(taskName: String?, settingName: String): Task? { + if (taskName == null) return null + + val moduleName = this.testifySettings.moduleName + val path = ":$moduleName:$taskName" + + return this.tasks.findByPath(path) ?: throw GradleExtensionException( + """ + |Testify could not find the `$taskName` task for this project. + | + | Searched for : $path + | moduleName : $moduleName${if (this.testifySettings.moduleName == this.name) " (inferred from the project name)" else ""} + | $settingName : $taskName + | + |`moduleName` must be the module's Gradle path, without the leading colon. It is inferred + |from the project name, which is correct for a top-level module but not for a nested one or + |for a module inside an included build — there, `${this.name}` is only the last segment of + |the path. + | + |Set it explicitly in this module's build file: + | + | testify { + | moduleName = "" + | } + | + |Run `./gradlew $moduleName:testifySettings` to print the resolved configuration, and + |`./gradlew tasks --all` to find the task's real path. + | + |See https://testify.dev/docs/recipes/build-types-and-flavors#nested-modules + """.trimMargin() + ) +} diff --git a/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt new file mode 100644 index 00000000..ec186e46 --- /dev/null +++ b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt @@ -0,0 +1,145 @@ +/* + * The MIT License (MIT) + * + * Copyright (c) 2023-2026 ndtp + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package dev.testify.tasks.main + +import com.google.common.truth.Truth.assertThat +import dev.testify.TestifySettings +import dev.testify.test.BaseTest +import io.mockk.every +import io.mockk.impl.annotations.RelaxedMockK +import io.mockk.mockk +import org.gradle.api.Project +import org.gradle.api.Task +import org.gradle.api.plugins.ExtensionContainer +import org.gradle.api.tasks.TaskContainer +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows + +/** + * Covers the resolution of the install tasks that `screenshotTest` and `screenshotRecord` depend on. + * + * The distinction under test is between a task that is legitimately absent and one that is + * misconfigured. See [getInstallDebugAndroidTestTask]. + */ +class InstallTaskResolutionTest : BaseTest() { + + @RelaxedMockK + lateinit var project: Project + + @RelaxedMockK + lateinit var extensions: ExtensionContainer + + @RelaxedMockK + lateinit var tasks: TaskContainer + + private val installTask: Task = mockk(relaxed = true) + + private fun settings( + moduleName: String, + installAndroidTestTask: String?, + installTask: String? = "installDebug" + ): TestifySettings = mockk { + every { this@mockk.moduleName } returns moduleName + every { this@mockk.installAndroidTestTask } returns installAndroidTestTask + every { this@mockk.installTask } returns installTask + } + + private fun givenSettings(settings: TestifySettings) { + every { extensions.getByName(any()) } returns settings + } + + @BeforeEach + fun before() { + every { project.extensions } returns extensions + every { project.tasks } returns tasks + every { project.name } returns "ui" + } + + @Test + fun `WHEN the install task resolves THEN it is returned`() { + givenSettings(settings(moduleName = "app", installAndroidTestTask = "installDebugAndroidTest")) + every { tasks.findByPath(":app:installDebugAndroidTest") } returns installTask + + assertThat(getInstallDebugAndroidTestTask(project)).isSameInstanceAs(installTask) + } + + /** + * A library, `com.android.test` or JVM-only module has no install task to depend on. That is + * not a misconfiguration, so it must not fail the build. + */ + @Test + fun `WHEN no install task was inferred THEN null is returned`() { + givenSettings(settings(moduleName = "app", installAndroidTestTask = null)) + + assertThat(getInstallDebugAndroidTestTask(project)).isNull() + } + + /** + * The #238 case: under a composite build the inferred `moduleName` is the project name rather + * than the module's Gradle path, so the lookup can never succeed. Previously this was swallowed. + */ + @Test + fun `WHEN the install task was inferred but does not resolve THEN it fails with the searched path`() { + givenSettings(settings(moduleName = "ui", installAndroidTestTask = "installDebugAndroidTest")) + every { tasks.findByPath(any()) } returns null + + val error = assertThrows { getInstallDebugAndroidTestTask(project) } + + assertThat(error).hasMessageThat().contains(":ui:installDebugAndroidTest") + assertThat(error).hasMessageThat().contains("moduleName") + assertThat(error).hasMessageThat().contains("inferred from the project name") + assertThat(error).hasMessageThat().contains("testifySettings") + } + + @Test + fun `WHEN moduleName was configured explicitly THEN the message does not blame inference`() { + givenSettings(settings(moduleName = "feature-2:ui", installAndroidTestTask = "installDebugAndroidTest")) + every { tasks.findByPath(any()) } returns null + + val error = assertThrows { getInstallDebugAndroidTestTask(project) } + + assertThat(error).hasMessageThat().contains(":feature-2:ui:installDebugAndroidTest") + assertThat(error).hasMessageThat().doesNotContain("inferred from the project name") + } + + @Test + fun `WHEN the plain install task does not resolve THEN it names that setting`() { + givenSettings(settings(moduleName = "ui", installAndroidTestTask = null, installTask = "installDebug")) + every { tasks.findByPath(any()) } returns null + + val error = assertThrows { getInstallDebugTask(project) } + + assertThat(error).hasMessageThat().contains(":ui:installDebug") + assertThat(error).hasMessageThat().contains("installTask") + } + + @Test + fun `WHEN no plain install task was inferred THEN null is returned`() { + givenSettings(settings(moduleName = "app", installAndroidTestTask = null, installTask = null)) + + assertThat(getInstallDebugTask(project)).isNull() + } +} From d3648c7101f08fd4d37c877edb0df13039b370ed Mon Sep 17 00:00:00 2001 From: Daniel Jette Date: Fri, 2 Oct 2026 16:54:10 -0400 Subject: [PATCH 2/4] 238: Resolve the install tasks within their own project --- CHANGELOG.md | 12 ++- .../testify/tasks/main/ScreenshotTestTask.kt | 55 +++++------ .../tasks/InstallTaskDependencyTest.kt | 87 ++++++++++++++++ .../tasks/main/InstallTaskResolutionTest.kt | 98 ++++++++++--------- docs/docs/errors.md | 9 ++ .../recipes/23-build-types-and-flavors.md | 4 +- 6 files changed, 183 insertions(+), 82 deletions(-) create mode 100644 Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index c94c55e2..49ad1d36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,10 +20,14 @@ - Fix `RootViewWithoutFocusException` being intermittently thrown by `Espresso.closeSoftKeyboard()` - **Breaking:** `dev.testify.internal.helpers.closeSoftKeyboard()` now requires an `Activity` parameter - Fix `ClassCastException` when passing `-PshardCount` and `-PshardIndex` to `screenshotTest` or `screenshotRecord` -- `screenshotTest` and `screenshotRecord` now fail with an actionable message when the install task - they depend on cannot be resolved, instead of dropping the dependency silently. The message names - the path searched, the `moduleName` used and whether it was inferred. Modules that legitimately - have no install task, such as an Android library or a `com.android.test` module, are unaffected +- Fix `screenshotTest` and `screenshotRecord` silently skipping the APK install for a module whose + `moduleName` is not its full Gradle path — a nested module such as `:feature:login`, or any module + in an included build. The install tasks are now resolved within their own project rather than by + rebuilding a path from `moduleName`, so they are found whatever `moduleName` is set to + - `moduleName` still determines the Gradle commands printed in failure messages, so setting it + for a nested module is still worthwhile + - An `installTask` or `installAndroidTestTask` that you set yourself and that names a + non-existent task now fails with a message naming the setting, rather than being ignored ## 6.0.0 diff --git a/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt b/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt index f3068027..3d19ccf4 100644 --- a/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt +++ b/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt @@ -218,48 +218,39 @@ internal fun getInstallDebugTask(project: Project): Task? = ) /** - * Resolve an install task by its full Gradle path, built from the configured `moduleName`. + * Resolve an install task that `screenshotTest` and `screenshotRecord` should depend on. + * + * The task belongs to this project. [inferredInstallTask] and [inferredAndroidTestInstallTask] pick + * the name out of `project.tasks.names`, so a simple name is looked up with `findByName` — which + * resolves identically in a top-level module, a module nested inside another directory and a module + * inside an included build. + * + * This is deliberately independent of `moduleName`. Building `":${'$'}moduleName:${'$'}taskName"` and + * resolving it with `findByPath` meant the lookup depended on a setting that defaults to + * `project.name`, which is only the last segment of a nested module's path — so for `:feature:login` + * it searched `:login:installDebugAndroidTest`, found nothing, and silently dropped the dependency. + * That is [#238](https://github.com/ndtp/android-testify/issues/238), and resolving the task where it + * actually lives fixes it rather than reporting it. * * Returns `null` when [taskName] is `null`, which is the legitimate case for a module that has no - * install task at all — an Android library, a `com.android.test` module, or a JVM-only module such - * as the Paparazzi sample. Those modules are expected to have nothing to depend on. + * such task — an Android library has no `installDebug`, and a `com.android.test` module has no + * `installDebugAndroidTest` because its own APK carries the tests. * - * When [taskName] is known but the path does not resolve, the configuration is wrong rather than - * absent, and the task would otherwise be dropped silently — leaving the tests to run against - * whatever happened to be installed on the device, or failing much later with Gradle's - * `A dependency must not be empty`. This is the case reported in - * [#238](https://github.com/ndtp/android-testify/issues/238): under a composite build the inferred - * `moduleName` of `project.name` is not the module's Gradle path, so the lookup cannot succeed. + * The one remaining way to have a name that does not resolve is an explicitly configured + * [settingName] naming a task that does not exist, which is a misconfiguration worth failing on. */ private fun Project.findInstallTask(taskName: String?, settingName: String): Task? { if (taskName == null) return null - val moduleName = this.testifySettings.moduleName - val path = ":$moduleName:$taskName" + // A configured value may be a full task path rather than a name in this project. + val task = if (taskName.contains(':')) tasks.findByPath(taskName) else tasks.findByName(taskName) - return this.tasks.findByPath(path) ?: throw GradleExtensionException( + return task ?: throw GradleExtensionException( """ - |Testify could not find the `$taskName` task for this project. - | - | Searched for : $path - | moduleName : $moduleName${if (this.testifySettings.moduleName == this.name) " (inferred from the project name)" else ""} - | $settingName : $taskName - | - |`moduleName` must be the module's Gradle path, without the leading colon. It is inferred - |from the project name, which is correct for a top-level module but not for a nested one or - |for a module inside an included build — there, `${this.name}` is only the last segment of - |the path. - | - |Set it explicitly in this module's build file: - | - | testify { - | moduleName = "" - | } - | - |Run `./gradlew $moduleName:testifySettings` to print the resolved configuration, and - |`./gradlew tasks --all` to find the task's real path. + |Testify could not find the task `$taskName`, configured as `$settingName`. | - |See https://testify.dev/docs/recipes/build-types-and-flavors#nested-modules + |Set it to a task that exists in this project, or remove it and let Testify infer the task. + |`./gradlew $path:tasks --all` lists the tasks available here. """.trimMargin() ) } diff --git a/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt new file mode 100644 index 00000000..0c36a3ab --- /dev/null +++ b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt @@ -0,0 +1,87 @@ +/* + * The MIT License (MIT) + * + * Copyright (c) 2023-2026 ndtp + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package dev.testify.tasks + +import com.google.common.truth.Truth.assertThat +import org.gradle.testkit.runner.GradleRunner +import org.junit.jupiter.api.Test +import java.io.File + +/** + * Asserts against the real root project that the install tasks actually join the task graph. + * + * The unit tests in `InstallTaskResolutionTest` mock `TaskContainer`, so they pin the lookup but + * not the wiring. [#238](https://github.com/ndtp/android-testify/issues/238) was a dropped + * dependency, and a dropped dependency is only visible in a graph, which is what this checks. + * + * `--dry-run` configures the build and prints the graph without running anything, so this needs no + * device. + */ +class InstallTaskDependencyTest { + + private fun taskGraphFor(task: String): String = + GradleRunner + .create() + .withProjectDir(File("../..")) + .withArguments("--dry-run", task) + .build() + .output + + @Test + fun `screenshotTest depends on the androidTest install task`() { + val graph = taskGraphFor(":FlixLibrary:screenshotTest") + + assertThat(graph).contains(":FlixLibrary:installDebugAndroidTest") + } + + @Test + fun `screenshotRecord depends on the androidTest install task`() { + val graph = taskGraphFor(":FlixLibrary:screenshotRecord") + + assertThat(graph).contains(":FlixLibrary:installDebugAndroidTest") + } + + /** + * An application module installs both APKs. + */ + @Test + fun `screenshotTest on an application module depends on both install tasks`() { + val graph = taskGraphFor(":LegacySample:screenshotTest") + + assertThat(graph).contains(":LegacySample:installDebugAndroidTest") + assertThat(graph).contains(":LegacySample:installDebug") + } + + /** + * A library module has no application APK to install, so there is nothing to depend on and the + * build must still configure. + */ + @Test + fun `a library module has no plain install task in its graph`() { + val graph = taskGraphFor(":FlixLibrary:screenshotTest") + + assertThat(graph).doesNotContain(":FlixLibrary:installDebug\n") + } +} diff --git a/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt index ec186e46..73bd7b02 100644 --- a/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt +++ b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt @@ -30,6 +30,7 @@ import dev.testify.test.BaseTest import io.mockk.every import io.mockk.impl.annotations.RelaxedMockK import io.mockk.mockk +import io.mockk.verify import org.gradle.api.Project import org.gradle.api.Task import org.gradle.api.plugins.ExtensionContainer @@ -39,10 +40,13 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows /** - * Covers the resolution of the install tasks that `screenshotTest` and `screenshotRecord` depend on. + * Covers resolution of the install tasks that `screenshotTest` and `screenshotRecord` depend on. * - * The distinction under test is between a task that is legitimately absent and one that is - * misconfigured. See [getInstallDebugAndroidTestTask]. + * The behaviour under test is that the task is found where it lives — in this project — rather than + * by reconstructing a path from `moduleName`, which is what made the lookup fail for nested modules + * in [#238](https://github.com/ndtp/android-testify/issues/238). + * + * @see getInstallDebugAndroidTestTask */ class InstallTaskResolutionTest : BaseTest() { @@ -57,17 +61,16 @@ class InstallTaskResolutionTest : BaseTest() { private val installTask: Task = mockk(relaxed = true) - private fun settings( - moduleName: String, - installAndroidTestTask: String?, - installTask: String? = "installDebug" - ): TestifySettings = mockk { - every { this@mockk.moduleName } returns moduleName - every { this@mockk.installAndroidTestTask } returns installAndroidTestTask - every { this@mockk.installTask } returns installTask - } - - private fun givenSettings(settings: TestifySettings) { + private fun givenSettings( + installAndroidTestTask: String? = "installDebugAndroidTest", + installTask: String? = "installDebug", + moduleName: String = "app" + ) { + val settings: TestifySettings = mockk { + every { this@mockk.moduleName } returns moduleName + every { this@mockk.installAndroidTestTask } returns installAndroidTestTask + every { this@mockk.installTask } returns installTask + } every { extensions.getByName(any()) } returns settings } @@ -76,70 +79,75 @@ class InstallTaskResolutionTest : BaseTest() { every { project.extensions } returns extensions every { project.tasks } returns tasks every { project.name } returns "ui" + every { project.path } returns ":feature:ui" } @Test - fun `WHEN the install task resolves THEN it is returned`() { - givenSettings(settings(moduleName = "app", installAndroidTestTask = "installDebugAndroidTest")) - every { tasks.findByPath(":app:installDebugAndroidTest") } returns installTask + fun `WHEN the install task exists THEN it is resolved by name`() { + givenSettings() + every { tasks.findByName("installDebugAndroidTest") } returns installTask assertThat(getInstallDebugAndroidTestTask(project)).isSameInstanceAs(installTask) } /** - * A library, `com.android.test` or JVM-only module has no install task to depend on. That is - * not a misconfiguration, so it must not fail the build. + * The #238 case. `moduleName` defaults to `project.name`, which is only the last segment of a + * nested module's path, so a lookup built from it could never succeed. The task lives in this + * project either way, so the name is all that is needed. */ @Test - fun `WHEN no install task was inferred THEN null is returned`() { - givenSettings(settings(moduleName = "app", installAndroidTestTask = null)) + fun `WHEN moduleName does not match the project path THEN the task still resolves`() { + givenSettings(moduleName = "ui") + every { tasks.findByName("installDebugAndroidTest") } returns installTask - assertThat(getInstallDebugAndroidTestTask(project)).isNull() + assertThat(getInstallDebugAndroidTestTask(project)).isSameInstanceAs(installTask) + verify(exactly = 0) { tasks.findByPath(any()) } } /** - * The #238 case: under a composite build the inferred `moduleName` is the project name rather - * than the module's Gradle path, so the lookup can never succeed. Previously this was swallowed. + * A library module has no `installDebug`; a `com.android.test` module has no + * `installDebugAndroidTest`. Neither is a misconfiguration. */ @Test - fun `WHEN the install task was inferred but does not resolve THEN it fails with the searched path`() { - givenSettings(settings(moduleName = "ui", installAndroidTestTask = "installDebugAndroidTest")) - every { tasks.findByPath(any()) } returns null - - val error = assertThrows { getInstallDebugAndroidTestTask(project) } + fun `WHEN no install task was inferred THEN null is returned`() { + givenSettings(installAndroidTestTask = null, installTask = null) - assertThat(error).hasMessageThat().contains(":ui:installDebugAndroidTest") - assertThat(error).hasMessageThat().contains("moduleName") - assertThat(error).hasMessageThat().contains("inferred from the project name") - assertThat(error).hasMessageThat().contains("testifySettings") + assertThat(getInstallDebugAndroidTestTask(project)).isNull() + assertThat(getInstallDebugTask(project)).isNull() } @Test - fun `WHEN moduleName was configured explicitly THEN the message does not blame inference`() { - givenSettings(settings(moduleName = "feature-2:ui", installAndroidTestTask = "installDebugAndroidTest")) - every { tasks.findByPath(any()) } returns null + fun `WHEN a configured task name does not exist THEN it fails naming the setting`() { + givenSettings(installAndroidTestTask = "installNopeDebugAndroidTest") + every { tasks.findByName(any()) } returns null val error = assertThrows { getInstallDebugAndroidTestTask(project) } - assertThat(error).hasMessageThat().contains(":feature-2:ui:installDebugAndroidTest") - assertThat(error).hasMessageThat().doesNotContain("inferred from the project name") + assertThat(error).hasMessageThat().contains("installNopeDebugAndroidTest") + assertThat(error).hasMessageThat().contains("installAndroidTestTask") + assertThat(error).hasMessageThat().contains(":feature:ui:tasks --all") } @Test - fun `WHEN the plain install task does not resolve THEN it names that setting`() { - givenSettings(settings(moduleName = "ui", installAndroidTestTask = null, installTask = "installDebug")) - every { tasks.findByPath(any()) } returns null + fun `WHEN a configured plain install task does not exist THEN it names that setting`() { + givenSettings(installTask = "installNope") + every { tasks.findByName(any()) } returns null val error = assertThrows { getInstallDebugTask(project) } - assertThat(error).hasMessageThat().contains(":ui:installDebug") + assertThat(error).hasMessageThat().contains("installNope") assertThat(error).hasMessageThat().contains("installTask") } + /** + * A configured value may name a task in another project, which is a path rather than a name. + */ @Test - fun `WHEN no plain install task was inferred THEN null is returned`() { - givenSettings(settings(moduleName = "app", installAndroidTestTask = null, installTask = null)) + fun `WHEN a configured value is a task path THEN it is resolved as a path`() { + givenSettings(installAndroidTestTask = ":app:installDebugAndroidTest") + every { tasks.findByPath(":app:installDebugAndroidTest") } returns installTask - assertThat(getInstallDebugTask(project)).isNull() + assertThat(getInstallDebugAndroidTestTask(project)).isSameInstanceAs(installTask) + verify(exactly = 0) { tasks.findByName(any()) } } } diff --git a/docs/docs/errors.md b/docs/docs/errors.md index 8973360e..c9372b5a 100644 --- a/docs/docs/errors.md +++ b/docs/docs/errors.md @@ -234,3 +234,12 @@ You must define `applicationPackageId` in your `testify` gradle extension block ``` - Add the missing setting to the `testify` block. For library modules, see [Configuring Testify for Android Library Projects](recipes/21-library-projects.md). Every plugin setting is described in [Use the Gradle Plugin tasks](get-started/8-use-gradle-plugin.md). + +The same exception reports an `installTask` or `installAndroidTestTask` that names a task which doesn't exist: + +``` +Testify could not find the task `installNopeDebugAndroidTest`, configured as `installAndroidTestTask`. +``` + +- Check the name against `./gradlew :tasks --all`, or remove the setting and let Testify infer the task from the module's own install tasks. +- This only applies to a value you set yourself. If you have not configured either setting, Testify infers it, and a module with no install task — an Android library has no `installDebug`, a `com.android.test` module has no `installDebugAndroidTest` — simply has no dependency to add. diff --git a/docs/docs/recipes/23-build-types-and-flavors.md b/docs/docs/recipes/23-build-types-and-flavors.md index 2fb22f63..3a4f7af3 100644 --- a/docs/docs/recipes/23-build-types-and-flavors.md +++ b/docs/docs/recipes/23-build-types-and-flavors.md @@ -120,7 +120,9 @@ $ ./gradlew app:screenshotTest -Pverbose=true ## Nested modules -`screenshotTest` and `screenshotRecord` depend on the install tasks, so the APKs are installed before the tests run. The plugin looks for the install tasks at `::`, and `moduleName` defaults to the module's own name, without its parent directories. For a module nested inside another directory, such as `:feature:login`, that gives `:login:installGoogleMockDebugAndroidTest`, which doesn't exist, so the plugin skips installing without an error ([#238](https://github.com/ndtp/android-testify/issues/238)). +`moduleName` defaults to the module's own name, without its parent directories. For a module nested inside another directory, such as `:feature:login`, that gives `login` rather than `feature:login`. + +That no longer affects whether your APKs get installed — `screenshotTest` and `screenshotRecord` resolve the install tasks within their own project, so they are found whatever `moduleName` says. What it does affect is every Gradle command Testify prints. A failing test in `:feature:login` will tell you to run `./gradlew login:screenshotPull`, which is not a task that exists. Set `moduleName` to the module's full path, without the leading colon: From 14d5d0481f78dcaf0cc8c50470d66b6117243fa1 Mon Sep 17 00:00:00 2001 From: Daniel Jette Date: Fri, 2 Oct 2026 21:25:00 -0400 Subject: [PATCH 3/4] 238: Defer the misconfiguration failure to the task --- CHANGELOG.md | 4 +- .../testify/tasks/main/ScreenshotTestTask.kt | 71 +++++++++++++------ .../tasks/InstallTaskDependencyTest.kt | 62 +++++++++++++--- .../tasks/main/InstallTaskResolutionTest.kt | 68 +++++++++++++++--- docs/docs/errors.md | 11 +-- 5 files changed, 170 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49ad1d36..44981df2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,9 @@ - `moduleName` still determines the Gradle commands printed in failure messages, so setting it for a nested module is still worthwhile - An `installTask` or `installAndroidTestTask` that you set yourself and that names a - non-existent task now fails with a message naming the setting, rather than being ignored + non-existent task now fails `screenshotTest` and `screenshotRecord` with a message naming the + setting, rather than being ignored. The failure is reported when the task runs, so the rest of + the build still configures ## 6.0.0 diff --git a/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt b/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt index 3d19ccf4..f5f5a4f6 100644 --- a/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt +++ b/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt @@ -24,6 +24,7 @@ */ package dev.testify.tasks.main +import dev.testify.getTestifyExtension import dev.testify.internal.Adb import dev.testify.internal.AdbParam import dev.testify.internal.StreamData.ConsoleStream @@ -201,21 +202,18 @@ open class ScreenshotTestTask : TestifyDefaultTask() { getInstallDebugTask(project)?.let { installDebugTask -> task.dependsOn(installDebugTask) } + configuredInstallTaskProblem(project)?.let { problem -> + task.doFirst { throw GradleExtensionException(problem) } + } } } } internal fun getInstallDebugAndroidTestTask(project: Project): Task? = - project.findInstallTask( - taskName = project.testifySettings.installAndroidTestTask, - settingName = "installAndroidTestTask" - ) + project.findInstallTask(project.testifySettings.installAndroidTestTask) internal fun getInstallDebugTask(project: Project): Task? = - project.findInstallTask( - taskName = project.testifySettings.installTask, - settingName = "installTask" - ) + project.findInstallTask(project.testifySettings.installTask) /** * Resolve an install task that `screenshotTest` and `screenshotRecord` should depend on. @@ -232,25 +230,56 @@ internal fun getInstallDebugTask(project: Project): Task? = * That is [#238](https://github.com/ndtp/android-testify/issues/238), and resolving the task where it * actually lives fixes it rather than reporting it. * - * Returns `null` when [taskName] is `null`, which is the legitimate case for a module that has no - * such task — an Android library has no `installDebug`, and a `com.android.test` module has no + * Returns `null` when the name is `null`, which is the legitimate case for a module that has no such + * task — an Android library has no `installDebug`, and a `com.android.test` module has no * `installDebugAndroidTest` because its own APK carries the tests. * - * The one remaining way to have a name that does not resolve is an explicitly configured - * [settingName] naming a task that does not exist, which is a misconfiguration worth failing on. + * It also returns `null`, rather than throwing, when a name does not resolve. The remaining way for + * that to happen is an explicitly configured setting naming a task that does not exist, which + * [verifyConfiguredInstallTasks] reports when the task runs. This runs from `afterEvaluate`, so + * throwing here would fail every invocation of the build — `help`, `assemble`, `tasks --all` and IDE + * sync — including the commands the message would suggest to diagnose it. */ -private fun Project.findInstallTask(taskName: String?, settingName: String): Task? { +private fun Project.findInstallTask(taskName: String?): Task? { if (taskName == null) return null // A configured value may be a full task path rather than a name in this project. - val task = if (taskName.contains(':')) tasks.findByPath(taskName) else tasks.findByName(taskName) + return if (taskName.contains(':')) tasks.findByPath(taskName) else tasks.findByName(taskName) +} - return task ?: throw GradleExtensionException( - """ - |Testify could not find the task `$taskName`, configured as `$settingName`. +/** + * Describe a misconfigured `installTask` or `installAndroidTestTask`, or `null` when both are fine. + * + * Only a value set in the `testify` block is checked. An inferred name always resolves, because it + * was read from this project's own task names, and a module with no install task has no name to + * resolve. + * + * The message is built here, at configuration time, so the task action that reports it captures a + * `String` rather than the `Project` — capturing the project would make `screenshotTest` and + * `screenshotRecord` incompatible with the configuration cache. + */ +internal fun configuredInstallTaskProblem(project: Project): String? { + val extension = project.getTestifyExtension() + + val (settingName, taskName) = listOf( + "installTask" to extension.installTask, + "installAndroidTestTask" to extension.installAndroidTestTask + ).firstOrNull { (_, name) -> + name != null && project.findInstallTask(name) == null + } ?: return null + + val listTasks = + if (project.path == ":") "./gradlew tasks --all" else "./gradlew ${project.path}:tasks --all" + + return """ + |Testify could not find the task `$taskName`, configured as `$settingName` in the + |`testify` block of ${project.path}. + | + |Remove that setting and Testify will infer the task from this project's own install + |tasks. To pick one yourself, remove it first and then run: + | + | $listTasks | - |Set it to a task that exists in this project, or remove it and let Testify infer the task. - |`./gradlew $path:tasks --all` lists the tasks available here. - """.trimMargin() - ) + |A task in another project can be named by its full path, beginning with `:`. + """.trimMargin() } diff --git a/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt index 0c36a3ab..6459ebe0 100644 --- a/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt +++ b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt @@ -27,40 +27,82 @@ package dev.testify.tasks import com.google.common.truth.Truth.assertThat import org.gradle.testkit.runner.GradleRunner import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir import java.io.File /** * Asserts against the real root project that the install tasks actually join the task graph. * - * The unit tests in `InstallTaskResolutionTest` mock `TaskContainer`, so they pin the lookup but - * not the wiring. [#238](https://github.com/ndtp/android-testify/issues/238) was a dropped - * dependency, and a dropped dependency is only visible in a graph, which is what this checks. + * The unit tests in `InstallTaskResolutionTest` mock `TaskContainer`, so they pin the lookup but not + * the wiring. [#238](https://github.com/ndtp/android-testify/issues/238) was a dropped dependency, + * and a dropped dependency is only visible in a graph. * * `--dry-run` configures the build and prints the graph without running anything, so this needs no * device. */ class InstallTaskDependencyTest { - private fun taskGraphFor(task: String): String = + @TempDir + lateinit var tempDir: File + + private fun taskGraphFor(task: String, vararg extraArgs: String): String = GradleRunner .create() .withProjectDir(File("../..")) - .withArguments("--dry-run", task) + .withArguments(listOf("--dry-run", task) + extraArgs) .build() .output + /** + * An init script is used rather than a fixture project because the behaviour needs the real + * plugin applied to a real Android module; only `moduleName` has to be wrong. + */ + private fun forceModuleName(projectPath: String, moduleName: String): File = + File(tempDir, "wrong-module-name.gradle").apply { + writeText( + """ + gradle.beforeProject { project -> + if (project.path == '$projectPath') { + project.plugins.withId('dev.testify') { + project.extensions.getByName('testify').moduleName = '$moduleName' + } + } + } + """.trimIndent() + ) + } + + /** + * The #238 regression. `moduleName` defaults to `project.name`, which is only the last segment + * of a nested module's path, and the lookup used to be built from it — so the dependency was + * silently dropped. Forcing a `moduleName` that is not the project's path reproduces that + * without needing a nested fixture; this assertion fails against the implementation on `main`. + */ + @Test + fun `the install task is found when moduleName is not the project path`() { + val initScript = forceModuleName(":FlixLibrary", "features:FlixLibrary") + + val graph = taskGraphFor( + ":FlixLibrary:screenshotTest", + "--init-script", + initScript.absolutePath + ) + + assertThat(graph).contains(":FlixLibrary:installDebugAndroidTest SKIPPED") + } + @Test fun `screenshotTest depends on the androidTest install task`() { val graph = taskGraphFor(":FlixLibrary:screenshotTest") - assertThat(graph).contains(":FlixLibrary:installDebugAndroidTest") + assertThat(graph).contains(":FlixLibrary:installDebugAndroidTest SKIPPED") } @Test fun `screenshotRecord depends on the androidTest install task`() { val graph = taskGraphFor(":FlixLibrary:screenshotRecord") - assertThat(graph).contains(":FlixLibrary:installDebugAndroidTest") + assertThat(graph).contains(":FlixLibrary:installDebugAndroidTest SKIPPED") } /** @@ -70,8 +112,8 @@ class InstallTaskDependencyTest { fun `screenshotTest on an application module depends on both install tasks`() { val graph = taskGraphFor(":LegacySample:screenshotTest") - assertThat(graph).contains(":LegacySample:installDebugAndroidTest") - assertThat(graph).contains(":LegacySample:installDebug") + assertThat(graph).contains(":LegacySample:installDebugAndroidTest SKIPPED") + assertThat(graph).contains(":LegacySample:installDebug SKIPPED") } /** @@ -82,6 +124,6 @@ class InstallTaskDependencyTest { fun `a library module has no plain install task in its graph`() { val graph = taskGraphFor(":FlixLibrary:screenshotTest") - assertThat(graph).doesNotContain(":FlixLibrary:installDebug\n") + assertThat(graph).doesNotContain(":FlixLibrary:installDebug SKIPPED") } } diff --git a/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt index 73bd7b02..7ad55a9a 100644 --- a/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt +++ b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/main/InstallTaskResolutionTest.kt @@ -25,7 +25,9 @@ package dev.testify.tasks.main import com.google.common.truth.Truth.assertThat +import dev.testify.TestifyExtension import dev.testify.TestifySettings +import dev.testify.tasks.main.configuredInstallTaskProblem import dev.testify.test.BaseTest import io.mockk.every import io.mockk.impl.annotations.RelaxedMockK @@ -37,7 +39,6 @@ import org.gradle.api.plugins.ExtensionContainer import org.gradle.api.tasks.TaskContainer import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test -import org.junit.jupiter.api.assertThrows /** * Covers resolution of the install tasks that `screenshotTest` and `screenshotRecord` depend on. @@ -74,6 +75,17 @@ class InstallTaskResolutionTest : BaseTest() { every { extensions.getByName(any()) } returns settings } + private fun givenExtension( + installAndroidTestTask: String? = null, + installTask: String? = null + ) { + val extension = TestifyExtension().apply { + this.installAndroidTestTask = installAndroidTestTask + this.installTask = installTask + } + every { extensions.findByType(TestifyExtension::class.java) } returns extension + } + @BeforeEach fun before() { every { project.extensions } returns extensions @@ -116,27 +128,63 @@ class InstallTaskResolutionTest : BaseTest() { assertThat(getInstallDebugTask(project)).isNull() } + /** + * The lookup itself no longer throws: it runs from `afterEvaluate`, so failing there would fail + * every invocation of the build, including the commands the message suggests for diagnosing it. + */ + @Test + fun `WHEN a configured task name does not exist THEN the lookup returns null`() { + givenSettings(installAndroidTestTask = "installNopeDebugAndroidTest") + every { tasks.findByName(any()) } returns null + + assertThat(getInstallDebugAndroidTestTask(project)).isNull() + } + @Test - fun `WHEN a configured task name does not exist THEN it fails naming the setting`() { + fun `WHEN a configured task name does not exist THEN the problem names the setting`() { givenSettings(installAndroidTestTask = "installNopeDebugAndroidTest") + givenExtension(installAndroidTestTask = "installNopeDebugAndroidTest") every { tasks.findByName(any()) } returns null - val error = assertThrows { getInstallDebugAndroidTestTask(project) } + val problem = configuredInstallTaskProblem(project) - assertThat(error).hasMessageThat().contains("installNopeDebugAndroidTest") - assertThat(error).hasMessageThat().contains("installAndroidTestTask") - assertThat(error).hasMessageThat().contains(":feature:ui:tasks --all") + assertThat(problem).contains("installNopeDebugAndroidTest") + assertThat(problem).contains("installAndroidTestTask") + assertThat(problem).contains("./gradlew :feature:ui:tasks --all") } @Test - fun `WHEN a configured plain install task does not exist THEN it names that setting`() { + fun `WHEN a configured plain install task does not exist THEN the problem names that setting`() { givenSettings(installTask = "installNope") + givenExtension(installTask = "installNope") every { tasks.findByName(any()) } returns null - val error = assertThrows { getInstallDebugTask(project) } + val problem = configuredInstallTaskProblem(project) + + assertThat(problem).contains("installNope") + assertThat(problem).contains("installTask") + } + + /** + * An inferred name always resolves, because it came from this project's own task names. Only a + * value someone typed can be wrong, so nothing is reported when the extension is empty. + */ + @Test + fun `WHEN nothing was configured THEN no problem is reported`() { + givenSettings() + givenExtension() + every { tasks.findByName(any()) } returns null + + assertThat(configuredInstallTaskProblem(project)).isNull() + } + + @Test + fun `WHEN a configured task exists THEN no problem is reported`() { + givenSettings() + givenExtension(installAndroidTestTask = "installDebugAndroidTest") + every { tasks.findByName("installDebugAndroidTest") } returns installTask - assertThat(error).hasMessageThat().contains("installNope") - assertThat(error).hasMessageThat().contains("installTask") + assertThat(configuredInstallTaskProblem(project)).isNull() } /** diff --git a/docs/docs/errors.md b/docs/docs/errors.md index c9372b5a..27694b8a 100644 --- a/docs/docs/errors.md +++ b/docs/docs/errors.md @@ -235,11 +235,14 @@ You must define `applicationPackageId` in your `testify` gradle extension block - Add the missing setting to the `testify` block. For library modules, see [Configuring Testify for Android Library Projects](recipes/21-library-projects.md). Every plugin setting is described in [Use the Gradle Plugin tasks](get-started/8-use-gradle-plugin.md). -The same exception reports an `installTask` or `installAndroidTestTask` that names a task which doesn't exist: +The same exception reports an `installTask` or `installAndroidTestTask` that names a task which doesn't exist. `screenshotTest` and `screenshotRecord` fail when they run; the rest of the build still configures. ``` -Testify could not find the task `installNopeDebugAndroidTest`, configured as `installAndroidTestTask`. +Testify could not find the task `installNopeDebugAndroidTest`, configured as +`installAndroidTestTask` in the `testify` block of :app. ``` -- Check the name against `./gradlew :tasks --all`, or remove the setting and let Testify infer the task from the module's own install tasks. -- This only applies to a value you set yourself. If you have not configured either setting, Testify infers it, and a module with no install task — an Android library has no `installDebug`, a `com.android.test` module has no `installDebugAndroidTest` — simply has no dependency to add. +- Remove the setting and Testify will infer the task from the module's own install tasks. That is usually the answer — the inferred value is correct for an application, library or `com.android.test` module. +- To pick one yourself, remove the setting first, then run `./gradlew :tasks --all` to list what is available. Listing the tasks while the bad value is still set works too, since the failure is deferred to the task, but removing it first tells you what Testify would have chosen. +- A task in another project can be named by its full path, beginning with `:`. +- This only applies to a value you set yourself. With neither setting configured, Testify infers it, and a module with no install task — an Android library has no `installDebug`, a `com.android.test` module has no `installDebugAndroidTest` — simply has no dependency to add. From 88a680f4435d28382c045d0e5a49a0bed12acf35 Mon Sep 17 00:00:00 2001 From: Daniel Jette Date: Sat, 3 Oct 2026 13:21:58 -0400 Subject: [PATCH 4/4] 238: Guard the record chain before it does any work --- .../tasks/main/ScreenshotRecordTask.kt | 4 + .../testify/tasks/main/ScreenshotTestTask.kt | 34 +++++--- .../tasks/InstallTaskDependencyTest.kt | 83 +++++++++++++++++-- 3 files changed, 106 insertions(+), 15 deletions(-) diff --git a/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotRecordTask.kt b/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotRecordTask.kt index 9ef335d9..24763ba8 100644 --- a/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotRecordTask.kt +++ b/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotRecordTask.kt @@ -56,6 +56,10 @@ open class ScreenshotRecordTask : TestifyDefaultTask() { ScreenshotTestTask.setDependencies(taskNameProvider, project) + // setDependencies above guards the task it is given, which for this chain is the + // `screenshotRecord` placeholder - and that runs last. Guard the tasks that do the work. + guardAgainstMisconfiguredInstallTask(project, screenshotClearTask, recordInternalTask) + getInstallDebugAndroidTestTask(project)?.let { installDebugAndroidTestTask -> screenshotClearTask.mustRunAfter(installDebugAndroidTestTask) screenshotPullTask.mustRunAfter(installDebugAndroidTestTask) diff --git a/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt b/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt index f5f5a4f6..357581af 100644 --- a/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt +++ b/Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt @@ -202,9 +202,7 @@ open class ScreenshotTestTask : TestifyDefaultTask() { getInstallDebugTask(project)?.let { installDebugTask -> task.dependsOn(installDebugTask) } - configuredInstallTaskProblem(project)?.let { problem -> - task.doFirst { throw GradleExtensionException(problem) } - } + guardAgainstMisconfiguredInstallTask(project, task) } } } @@ -236,9 +234,9 @@ internal fun getInstallDebugTask(project: Project): Task? = * * It also returns `null`, rather than throwing, when a name does not resolve. The remaining way for * that to happen is an explicitly configured setting naming a task that does not exist, which - * [verifyConfiguredInstallTasks] reports when the task runs. This runs from `afterEvaluate`, so - * throwing here would fail every invocation of the build — `help`, `assemble`, `tasks --all` and IDE - * sync — including the commands the message would suggest to diagnose it. + * [guardAgainstMisconfiguredInstallTask] reports when the task runs. This runs from `afterEvaluate`, + * so throwing here would fail every invocation of the build — `help`, `assemble`, `tasks --all` + * and IDE sync — including the commands the message would suggest to diagnose it. */ private fun Project.findInstallTask(taskName: String?): Task? { if (taskName == null) return null @@ -247,16 +245,32 @@ private fun Project.findInstallTask(taskName: String?): Task? { return if (taskName.contains(':')) tasks.findByPath(taskName) else tasks.findByName(taskName) } +/** + * Fail each of [tasks] before it does any work if an install task is misconfigured. + * + * Attach this to every task that touches the device or the baseline directory, not only to the task + * the user names. `screenshotRecord` is a placeholder that depends on `screenshotClear`, + * `screenshotTestRecord` and `screenshotPull`, so guarding it alone reports the problem *after* the + * device has been cleared, the tests have run against whatever was installed, and the results have + * been pulled over the baselines — the worst possible moment. + * + * The message is computed once here, at configuration time, so the action captures a `String` rather + * than the `Project`; capturing the project would break the configuration cache. + */ +internal fun guardAgainstMisconfiguredInstallTask(project: Project, vararg tasks: Task) { + val problem = configuredInstallTaskProblem(project) ?: return + + tasks.forEach { task -> + task.doFirst { throw GradleExtensionException(problem) } + } +} + /** * Describe a misconfigured `installTask` or `installAndroidTestTask`, or `null` when both are fine. * * Only a value set in the `testify` block is checked. An inferred name always resolves, because it * was read from this project's own task names, and a module with no install task has no name to * resolve. - * - * The message is built here, at configuration time, so the task action that reports it captures a - * `String` rather than the `Project` — capturing the project would make `screenshotTest` and - * `screenshotRecord` incompatible with the configuration cache. */ internal fun configuredInstallTaskProblem(project: Project): String? { val extension = project.getTestifyExtension() diff --git a/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt index 6459ebe0..242b1ff9 100644 --- a/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt +++ b/Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt @@ -25,6 +25,8 @@ package dev.testify.tasks import com.google.common.truth.Truth.assertThat +import com.google.common.truth.TruthJUnit.assume +import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.GradleRunner import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir @@ -53,18 +55,25 @@ class InstallTaskDependencyTest { .build() .output + private fun buildFailureFor(task: String, vararg extraArgs: String): BuildResult = + GradleRunner + .create() + .withProjectDir(File("../..")) + .withArguments(listOf(task) + extraArgs) + .buildAndFail() + /** * An init script is used rather than a fixture project because the behaviour needs the real - * plugin applied to a real Android module; only `moduleName` has to be wrong. + * plugin applied to a real Android module; only the one setting has to be wrong. */ - private fun forceModuleName(projectPath: String, moduleName: String): File = - File(tempDir, "wrong-module-name.gradle").apply { + private fun forceSetting(projectPath: String, setting: String, value: String): File = + File(tempDir, "forced-$setting.gradle").apply { writeText( """ gradle.beforeProject { project -> if (project.path == '$projectPath') { project.plugins.withId('dev.testify') { - project.extensions.getByName('testify').moduleName = '$moduleName' + project.extensions.getByName('testify').$setting = '$value' } } } @@ -72,6 +81,18 @@ class InstallTaskDependencyTest { ) } + private fun assumeDevice() { + assume() + .that( + GradleRunner + .create() + .withProjectDir(File("../..")) + .withArguments(":LegacySample:testifyDevices") + .build() + .output + ).contains("Connected devices = 1") + } + /** * The #238 regression. `moduleName` defaults to `project.name`, which is only the last segment * of a nested module's path, and the lookup used to be built from it — so the dependency was @@ -80,7 +101,7 @@ class InstallTaskDependencyTest { */ @Test fun `the install task is found when moduleName is not the project path`() { - val initScript = forceModuleName(":FlixLibrary", "features:FlixLibrary") + val initScript = forceSetting(":FlixLibrary", "moduleName", "features:FlixLibrary") val graph = taskGraphFor( ":FlixLibrary:screenshotTest", @@ -126,4 +147,56 @@ class InstallTaskDependencyTest { assertThat(graph).doesNotContain(":FlixLibrary:installDebug SKIPPED") } + + /** + * A misconfigured install task has to be reported before anything happens, not after. + * + * `screenshotRecord` is a placeholder that depends on `screenshotClear`, `screenshotTestRecord` + * and `screenshotPull`, so a guard on it alone fires only once the device has been cleared, the + * tests have run against whatever was installed, and the results have been pulled over the + * baselines. + */ + @Test + fun `screenshotRecord fails before it clears the device`() { + assumeDevice() + val initScript = forceSetting( + ":FlixLibrary", + "installAndroidTestTask", + "installNopeDebugAndroidTest" + ) + + val result = buildFailureFor( + ":FlixLibrary:screenshotRecord", + "--init-script", + initScript.absolutePath + ) + + assertThat(result.output).contains("installNopeDebugAndroidTest") + assertThat(result.output).contains(":FlixLibrary:screenshotClear FAILED") + assertThat(result.output).doesNotContain(":FlixLibrary:screenshotTestRecord") + assertThat(result.output).doesNotContain(":FlixLibrary:screenshotPull") + } + + /** + * The internal record task is an entry point of its own, so it is guarded too. + */ + @Test + fun `screenshotTestRecord fails before it runs the tests`() { + assumeDevice() + val initScript = forceSetting( + ":FlixLibrary", + "installAndroidTestTask", + "installNopeDebugAndroidTest" + ) + + val result = buildFailureFor( + ":FlixLibrary:screenshotTestRecord", + "--init-script", + initScript.absolutePath + ) + + assertThat(result.output).contains("installNopeDebugAndroidTest") + assertThat(result.output).contains(":FlixLibrary:screenshotTestRecord FAILED") + assertThat(result.output).doesNotContain("OK (") + } }