238: Resolve the install tasks within their own project

CHANGELOG.md

@@ -20,6 +20,16 @@
 - 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`
+- 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 `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
 

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)

Plugins/Gradle/src/main/kotlin/dev/testify/tasks/main/ScreenshotTestTask.kt

@@ -24,13 +24,15 @@
  */
 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
 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
@@ -200,12 +202,98 @@ open class ScreenshotTestTask : TestifyDefaultTask() {
             getInstallDebugTask(project)?.let { installDebugTask ->
                 task.dependsOn(installDebugTask)
             }
+            guardAgainstMisconfiguredInstallTask(project, task)
         }
     }
 }
 
 internal fun getInstallDebugAndroidTestTask(project: Project): Task? =
-    project.tasks.findByPath(":${project.testifySettings.moduleName}:${project.testifySettings.installAndroidTestTask}")
+    project.findInstallTask(project.testifySettings.installAndroidTestTask)
 
 internal fun getInstallDebugTask(project: Project): Task? =
-    project.tasks.findByPath(":${project.testifySettings.moduleName}:${project.testifySettings.installTask}")
+    project.findInstallTask(project.testifySettings.installTask)
+
+/**
+ * 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 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.
+ *
+ * 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
+ * [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
+
+    // A configured value may be a full task path rather than a name in this project.
+    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.
+ */
+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
+        |
+        |A task in another project can be named by its full path, beginning with `:`.
+    """.trimMargin()
+}

Plugins/Gradle/src/test/kotlin/dev/testify/tasks/InstallTaskDependencyTest.kt

@@ -0,0 +1,202 @@
+/*
+ * 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 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
+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.
+ *
+ * `--dry-run` configures the build and prints the graph without running anything, so this needs no
+ * device.
+ */
+class InstallTaskDependencyTest {
+
+    @TempDir
+    lateinit var tempDir: File
+
+    private fun taskGraphFor(task: String, vararg extraArgs: String): String =
+        GradleRunner
+            .create()
+            .withProjectDir(File("../.."))
+            .withArguments(listOf("--dry-run", task) + extraArgs)
+            .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 the one setting has to be wrong.
+     */
+    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').$setting = '$value'
+                        }
+                    }
+                }
+                """.trimIndent()
+            )
+        }
+
+    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
+     * 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 = forceSetting(":FlixLibrary", "moduleName", "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 SKIPPED")
+    }
+
+    @Test
+    fun `screenshotRecord depends on the androidTest install task`() {
+        val graph = taskGraphFor(":FlixLibrary:screenshotRecord")
+
+        assertThat(graph).contains(":FlixLibrary:installDebugAndroidTest SKIPPED")
+    }
+
+    /**
+     * 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 SKIPPED")
+        assertThat(graph).contains(":LegacySample:installDebug SKIPPED")
+    }
+
+    /**
+     * 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 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 (")
+    }
+}