238: Resolve the install tasks within their own project

[DanielJette]

What does this change accomplish?

Fixes #238

Applying the plugin to a nested module silently produces a build that never installs the APKs. The
reporter's case: for feature-2:ui, moduleName falls back to project.name — ui — so the plugin
looked for :ui:installDebugAndroidTest, which does not exist. The result was discarded with
?.let, so the tests ran against whatever was already on the device.

This is not limited to composite builds. :feature:login is an ordinary nested layout and hits it
the same way. It is also the root cause behind several confused reports on #131.

How have you achieved it?

The lookup does not need moduleName at all. inferredInstallTask and
inferredAndroidTestInstallTask pick the task name out of project.tasks.names — the names come
from this project. Rebuilding ":$moduleName:$taskName" and resolving it with findByPath was an
indirection that could only ever fail; tasks.findByName(taskName) resolves in a top-level module, a
nested module and an included build alike.

So #238 is now fixed rather than reported. With the lookup independent of moduleName, the only way
left to have a name that does not resolve is an installTask / installAndroidTestTask that you set
yourself naming a task that does not exist. That is a genuine misconfiguration, it throws, and the
message is three lines. A configured value containing : is treated as a task path, so naming a task
in another project still works.

This does not touch the moduleName default. project.path carries a leading colon and the old
lookup interpolated another, which would have produced ::feature:login:installDebugAndroidTest —
the same class of bug as #195.

Scope of Impact and Testing instructions

A nested module that was silently skipping the install now installs. No new configuration-time
failure for anyone who has not explicitly configured a task name.

CHANGELOG entry rewritten to describe the fix rather than the diagnostic.

Docs. recipes/23-build-types-and-flavors.md said the plugin "skips installing without an error
(#238)"; it now explains that moduleName no longer affects installation but still determines the
Gradle commands printed in failure messages, which is the real reason to set it on a nested module.
errors.md's GradleExtensionException entry covers the new message.

Tests. InstallTaskResolutionTest rewritten — 6 cases, including that the lookup never calls
findByPath for a simple name, which is the #238 guard. All 6 fail against main's implementation.

InstallTaskDependencyTest added alongside ConfigurationCacheTest, because mocked
TaskContainer tests pin the lookup but not the wiring, and a dropped dependency is only visible in
a graph. It runs --dry-run against the real root project — no device needed — and asserts
:FlixLibrary:screenshotTest and :screenshotRecord both carry installDebugAndroidTest,
:LegacySample:screenshotTest carries both install tasks, and a library module has no plain
installDebug to depend on.

Verified locally:

./gradlew Plugin:ktlintCheck Plugin:test Plugin:assemble   # BUILD SUCCESSFUL
cd docs && npm run build                                   # clean

The reviewer's repro, which is the point of the change:

# with moduleName "features:FlixLibrary" set on :FlixLibrary
$ ./gradlew help
BUILD SUCCESSFUL                                  # was: build fails to configure
$ ./gradlew FlixLibrary:screenshotTest --dry-run
:FlixLibrary:installDebugAndroidTest SKIPPED      # on main: absent from the graph

[AndroidTestifyBot]

Changes requested

The message is well written, but this turns a recoverable misconfiguration into a build that will not configure at all, and it does so instead of fixing a lookup that has a one-line fix.

1. The failure is thrown for every Gradle invocation, not just the screenshot tasks

setDependencies runs from afterEvaluate, so the throw happens at configuration time. I set moduleName "features:FlixLibrary" on :FlixLibrary and ran ./gradlew help:

	result
main	BUILD SUCCESSFUL
this PR	A problem occurred configuring project ':FlixLibrary'. > Testify could not find the installDebugAndroidTest task

That means assemble, unit tests and IDE sync all fail. The inference is wrong for every nested module (:feature:login is an ordinary layout, not just composite builds), so on upgrade each of those projects stops syncing until moduleName is set. The description says it "will surface as a new failure"; it should say the whole build stops configuring.

2. The message's own remediation cannot be run

Run ./gradlew features:FlixLibrary:testifySettings to print the resolved configuration, and ./gradlew tasks --all to find the task's real path.

Both commands configure the project, so both die with this same error. The advice is unusable in exactly the situation it is printed.

3. The lookup doesn't need moduleName at all

inferredInstallTask / inferredAndroidTestInstallTask read this.tasks.names — the names come from this project. Building ":$moduleName:$name" and calling findByPath to find them again is an indirection that can only fail. project.tasks.findByName(taskName) resolves in a top-level module, a nested module and an included build alike, with no dependence on moduleName.

With that change #238 is actually fixed rather than reported, and the only remaining way to get null for a non-null name is a user who explicitly set installTask / installAndroidTestTask to something that doesn't exist. Throwing there is right, and the message can be much shorter. The "deliberately not included" paragraph argues against changing the moduleName default; this doesn't touch that default.

4. Docs now contradict the code

docs/docs/recipes/23-build-types-and-flavors.md ("Nested modules") still says "the plugin skips installing without an error (#238)", and the new message links to that very section. docs/docs/errors.md has no entry for the new error.

Smaller points

• The KDoc and CHANGELOG say a com.android.test module has no install task. On #333's FlixTest, testifySettings prints installTask = installDebug. Only installAndroidTestTask is null there.
• "inferred from the project name" is detected with moduleName == project.name, which is also true when someone set it explicitly to that value.
• The tests mock Project, TaskContainer and TestifySettings, so they pin the string, not the behaviour. Nothing covers a real nested module; a TestKit case alongside ConfigurationCacheTest would.

Verified

Plugin:ktlintCheck Plugin:test Plugin:assemble pass locally (6/6 new tests, ConfigurationCacheTest green). Merged with #333, FlixTest:testifySettings still configures. CHANGELOG hunk conflicts with the other five open code PRs.

[DanielJette]

You were right on all four, and point 3 was the one that mattered — the PR has been reshaped around it. Retitled, since it is now a fix rather than a diagnostic.

3. The lookup doesn't need moduleName — agreed, and this is now the whole change

inferredInstallTask reads project.tasks.names, so the name already came from this project. Rebuilding a path and resolving it with findByPath could only ever fail. The lookup is now tasks.findByName(taskName), with a findByPath branch kept only for a configured value containing : so naming a task in another project still works.

Your repro, before and after:

# moduleName "features:FlixLibrary" on :FlixLibrary
$ ./gradlew help
BUILD SUCCESSFUL

$ ./gradlew FlixLibrary:screenshotTest --dry-run
:FlixLibrary:installDebugAndroidTest SKIPPED

And on main with the same setting, for contrast: zero FlixLibrary:install entries in the graph. So the dependency was being dropped exactly as #238 describes, and it now survives.

1 and 2 — both dissolved by the above

Agreed that throwing from afterEvaluate breaks assemble, unit tests and IDE sync, and that "will surface as a new failure" badly understated it. And you are right that the remediation was unusable — testifySettings and tasks --all both configure the project, so both would have died on the error that printed them. That was a real hole in the design and I should have caught it by running the advice.

Neither applies now: with the lookup independent of moduleName, nothing throws for an inferred name. The only remaining throw is a setting you typed yourself naming a task that does not exist, which no amount of inference can rescue, and the message is three lines pointing at <module>:tasks --all — a command that works, because that project configures fine.

4. Docs

• recipes/23-build-types-and-flavors.md no longer says the plugin "skips installing without an error". It now explains that moduleName does not affect installation but does determine the Gradle commands printed in failure messages — so a failing test in :feature:login tells you to run ./gradlew login:screenshotPull, which is not a task. That is the real reason to set it, and it is a better reason than the one the page gave.
• errors.md covers the new message under the existing GradleExtensionException entry, including that it only applies to a value you set yourself.

Smaller points

• com.android.test and install tasks — correct, and I had seen installTask = installDebug in 245: Add support for com.android.test modules #333's own output without joining it up. The KDoc and CHANGELOG now say an Android library has no installDebug and a com.android.test module has no installDebugAndroidTest, rather than claiming a test module has neither.
• "inferred from the project name" — gone. It was ambiguous as you say, and the shorter message has no use for it.
• Tests pin the string, not the behaviour — fair, and fixed two ways. InstallTaskResolutionTest is rewritten to 6 cases, one of which asserts findByPath is never called for a simple name; all 6 fail against main's implementation. And InstallTaskDependencyTest is new alongside ConfigurationCacheTest: --dry-run against the real root project, no device, asserting the install tasks are actually in the graph for a library module, an application module, and both screenshotTest and screenshotRecord. A dropped dependency is only visible in a graph, which is exactly the gap you identified.

[AndroidTestifyBot]

Changes requested

The core fix is right and verified. One part of the old problem survives in the remaining error path, and the regression test doesn't cover the regression.

Verified: #238 is fixed

With moduleName "features:FlixLibrary" on :FlixLibrary:

$ ./gradlew help                                  BUILD SUCCESSFUL
$ ./gradlew FlixLibrary:screenshotTest --dry-run  :FlixLibrary:installDebugAndroidTest SKIPPED
$ ./gradlew FlixLibrary:screenshotTest            > Task :FlixLibrary:installDebugAndroidTest ... OK (3 tests)

findByName, with findByPath kept for a value containing :, is the right shape. The docs and CHANGELOG now describe what the code does. Plugin:ktlintCheck Plugin:test Plugin:assemble pass locally.

1. The remaining error's advice still can't be run

The comment says the new message points at "<module>:tasks --all — a command that works, because that project configures fine." It doesn't. The throw is still in afterEvaluate, so with installAndroidTestTask "installNopeDebugAndroidTest" on :FlixLibrary:

$ ./gradlew help
A problem occurred configuring project ':FlixLibrary'.   BUILD FAILED
$ ./gradlew :FlixLibrary:tasks --all
A problem occurred configuring project ':FlixLibrary'.   BUILD FAILED

The scope is now much smaller (only a value someone typed), and failing on that is reasonable. But the remedy the message gives dies on the same error, and so does every other command in the build, IDE sync included. Two ways out:

• Defer the failure to execution: register the dependency lazily, or fail in a doFirst on screenshotTest / screenshotRecord. Configuration then succeeds and tasks --all works.
• Or keep it at configuration and change the advice to something that can run: remove the setting first, then list the tasks. errors.md says the same thing and needs the same correction.

2. InstallTaskDependencyTest would pass on main

All four cases use :FlixLibrary and :LegacySample. Both are top-level, moduleName == project.name, and the old findByPath(":$moduleName:…") resolves for both. So the class described as the thing that makes a dropped dependency visible doesn't exercise the case that dropped it. It needs one module whose moduleName isn't its path: a TestKit fixture with a nested :feature:ui, or a -P hook that sets moduleName on an existing module. The negative doesNotContain(":FlixLibrary:installDebug\n") also depends on Gradle's line formatting; anchoring on the SKIPPED suffix would be sturdier.

Minor

./gradlew $path:tasks --all prints ./gradlew ::tasks --all if the plugin is ever applied to the root project.

[DanielJette]

Both points taken, and the first one found a configuration-cache regression on the way.

1. The failure is now deferred to the task

You were right that my previous comment was wrong about this: the throw was still in afterEvaluate, so the remedy died on the error it was diagnosing. Took your first option — the lookup now returns null rather than throwing, and the misconfiguration is reported from a doFirst on screenshotTest / screenshotRecord.

# installAndroidTestTask "installNopeDebugAndroidTest" on :FlixLibrary
$ ./gradlew help                        BUILD SUCCESSFUL
$ ./gradlew :FlixLibrary:tasks --all     BUILD SUCCESSFUL
$ ./gradlew FlixLibrary:screenshotTest   BUILD FAILED
  > Testify could not find the task `installNopeDebugAndroidTest`, configured as
    `installAndroidTestTask` in the `testify` block of :FlixLibrary.

The first attempt broke the configuration cache. Putting verifyConfiguredInstallTasks(project) in the doFirst captured the Project in the task action, and ConfigurationCacheTest failed on [10] screenshotRecord and [11] screenshotTest. Good test. The message is now built at configuration time and the action captures only the resulting String:

$ ./gradlew --configuration-cache FlixLibrary:screenshotTest --dry-run
Configuration cache entry stored
$ ./gradlew --configuration-cache FlixLibrary:screenshotTest --dry-run
Configuration cache entry reused

errors.md is corrected the same way: it now says the failure is deferred, that removing the setting is usually the answer, and that if you do want to pick one yourself you should remove it first — because that also tells you what Testify would have chosen.

2. The test would have passed on main — fixed

Correct, and the "makes a dropped dependency visible" claim was hollow: :FlixLibrary and :LegacySample are both top-level, so findByPath(":$moduleName:…") resolved for both.

Rather than build a nested AGP fixture, the new case forces a moduleName that is not the project's path, via a generated init script:

gradle.beforeProject { project ->
    if (project.path == ':FlixLibrary') {
        project.plugins.withId('dev.testify') {
            project.extensions.getByName('testify').moduleName = 'features:FlixLibrary'
        }
    }
}

beforeProject registers the withId hook before the build script runs, so the value is set at apply time — ahead of the plugin's own afterEvaluate. That reproduces the inference that dropped the dependency, against the real plugin on a real Android module, with nothing to maintain but the script.

It fails against main's implementation and passes here; the other four pass on both, which is correct since they cover wiring that already worked.

Also switched the assertions to anchor on the SKIPPED suffix, including the negative one — your point about depending on Gradle's line formatting was right.

Minor

Handled: the message uses ./gradlew tasks --all when project.path is :, so the root project no longer renders ::tasks --all.

Verified

./gradlew Plugin:ktlintCheck Plugin:test Plugin:assemble   # BUILD SUCCESSFUL
cd docs && npm run build                                   # clean

ConfigurationCacheTest green across all 15 cases.

[AndroidTestifyBot]

Changes requested — one placement fix

screenshotTest is right now. screenshotRecord still runs the whole recording before reporting the misconfiguration.

Verified

With installAndroidTestTask "installNopeDebugAndroidTest" on :FlixLibrary:

$ ./gradlew help                        BUILD SUCCESSFUL
$ ./gradlew :FlixLibrary:tasks --all     BUILD SUCCESSFUL
$ ./gradlew FlixLibrary:screenshotTest   > Task :FlixLibrary:screenshotTest FAILED
  > Testify could not find the task `installNopeDebugAndroidTest`, configured as `installAndroidTestTask` ...

• The configuration-cache fix (capturing the String, not the Project) is right.
• The init-script case finally exercises a moduleName that isn't the project path.
• Plugin:ktlintCheck Plugin:test Plugin:assemble pass locally, including ConfigurationCacheTest.
• The docs build is clean.

The screenshotRecord guard fires last

ScreenshotRecordTask.setDependencies passes the record task into ScreenshotTestTask.setDependencies, so the doFirst lands on screenshotRecord. That's the no-op placeholder, and it only runs after everything it depends on:

$ ./gradlew FlixLibrary:screenshotRecord
> Task :FlixLibrary:screenshotClear
> Task :FlixLibrary:screenshotTestRecord
OK (3 tests)
> Task :FlixLibrary:screenshotPull
> Task :FlixLibrary:screenshotRecord FAILED
  > Testify could not find the task `installNopeDebugAndroidTest` ...

The device was cleared, the tests ran in record mode against whatever APK was already installed (the install dependency is exactly what's missing), and screenshotPull copied the results into src/androidTest/assets. Only then did the build fail. A user can be left with baselines recorded from a stale build, plus a failure message that doesn't mention it.

Attach the check to the first task that does real work instead: a doFirst on screenshotClear and on the internal screenshotTestRecord, or have screenshotRecord's path guard InternalScreenshotTestRecordTask rather than the placeholder. A dry-run assertion in InstallTaskDependencyTest that screenshotClear doesn't run when the setting is bad would pin it.

[DanielJette]

Agreed, and this was the dangerous one. screenshotRecord is a placeholder that depends on the work, so guarding it meant reporting the problem after the device was cleared, the tests had run against a stale APK, and screenshotPull had copied the results over the baselines.

The guard moved to the tasks that do the work

Extracted guardAgainstMisconfiguredInstallTask(project, vararg tasks) and attached it in ScreenshotRecordTask.setDependencies to screenshotClearTask and recordInternalTask. ScreenshotTestTask.setDependencies still guards the task it is given, which is correct for screenshotTest — that task does the work itself.

$ ./gradlew FlixLibrary:screenshotRecord
> Task :FlixLibrary:deviceLocale
> Task :FlixLibrary:deviceTimeZone
> Task :FlixLibrary:disableSoftKeyboard
> Task :FlixLibrary:disableStylusInput
> Task :FlixLibrary:hidePasswords
> Task :FlixLibrary:screenshotClear FAILED
> Testify could not find the task `installNopeDebugAndroidTest`, configured as
  `installAndroidTestTask` in the `testify` block of :FlixLibrary.

screenshotTestRecord and screenshotPull never run, and git status on the baselines is empty afterwards. The five tasks that do run first are the device-setting ones, which are idempotent and which screenshotTest would run anyway — I left those alone rather than widening the guard to shared utility tasks that can be invoked on their own.

I also guarded screenshotTestRecord directly, since it is an entry point in its own right:

$ ./gradlew FlixLibrary:screenshotTestRecord
> Task :FlixLibrary:screenshotTestRecord FAILED
> Testify could not find the task `installNopeDebugAndroidTest` ...

The test

A dry run doesn't execute task actions, so --dry-run can't see this — the guard never fires and every task is still listed. So the two new cases in InstallTaskDependencyTest use buildAndFail() and assert on which task failed and what did not run:

assertThat(result.output).contains(":FlixLibrary:screenshotClear FAILED")
assertThat(result.output).doesNotContain(":FlixLibrary:screenshotTestRecord")
assertThat(result.output).doesNotContain(":FlixLibrary:screenshotPull")

They need a device, since the device-setting tasks run first, so they are gated with the same assumeDevice() pattern ConfigurationCacheTest uses. Both fail against the previous placement and pass now. forceModuleName is generalised to forceSetting(path, setting, value) so the init script serves both this and the moduleName case.

Verified

./gradlew Plugin:ktlintCheck Plugin:test Plugin:assemble   # BUILD SUCCESSFUL, 7/7 in this class

Correctly configured, the chain is unchanged — screenshotClear → screenshotTestRecord → screenshotPull → screenshotRecord, FlixLibrary:screenshotTest gives OK (3 tests), and baselines come back byte-identical. Configuration cache still stores and reuses for screenshotRecord.

One tidy-up in passing: my previous commit left configuredInstallTaskProblem's KDoc attached to the wrong declaration. Fixed.

[AndroidTestifyBot]

Approved

The record chain is now guarded where the work happens. I verified it on an API 37 emulator.

With installAndroidTestTask "installNopeDebugAndroidTest" on :FlixLibrary:

$ ./gradlew FlixLibrary:screenshotRecord
> Task :FlixLibrary:screenshotClear FAILED
> Testify could not find the task `installNopeDebugAndroidTest`, configured as `installAndroidTestTask` ...
BUILD FAILED in 1s
$ git status --short Samples/Flix/FlixLibrary/src      # empty
$ ./gradlew help                                        BUILD SUCCESSFUL

screenshotTestRecord and screenshotPull never run, and no baselines are touched.

Correctly configured, the chain is unchanged:

installDebugAndroidTest → screenshotClear → screenshotTestRecord (OK (3 tests)) → screenshotPull → screenshotRecord

git status is clean afterwards. With --configuration-cache, the entry is stored on the first run and reused on the second, so capturing only the String holds up.

Plugin:ktlintCheck Plugin:test Plugin:assemble pass locally. All 7 InstallTaskDependencyTest cases pass, including the two new buildAndFail cases. Asserting on which task failed and what didn't run is the right test for this; a dry run couldn't have seen it. CI is green.

Non-blocking:

• screenshotClear run on its own now also fails on this misconfiguration, although it doesn't need an install task. That is the right trade for guarding the chain; just noting it's a visible change for anyone who runs it standalone.
• assumeDevice() matches Connected devices = 1, so with two devices attached the two new cases skip silently rather than run. Matching "not 0" would be more forgiving. Also worth confirming the Bitrise Plugin step has a device, or these two never run in CI.
• This and #333 both edit ScreenshotTestTask.setDependencies; whichever lands second needs a careful rebase.