Fix configuration cache compatibility#18
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the Android Snaptesting Gradle plugin to be compatible with Gradle Configuration Cache by avoiding execution-time Task.project access and by precomputing project/variant-derived values during configuration.
Changes:
- Refactors
DeviceFileManagerto accept pre-resolvedapplicationId,adbExecutablePath, andProviderFactoryinstead of deriving them fromtestTask.project. - Reworks plugin wiring so snapshot cleanup and report generation are attached directly to
DeviceProviderInstrumentTestTaskactions, and removes the task-completion listener. - Keeps empty
androidSnaptestingBefore*/androidSnaptestingAfter*tasks as CI dependency anchors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| include-build/gradle-plugin/src/main/java/com/telefonica/androidsnaptesting/DeviceFileManager.kt | Removes project/AGP lookups from DeviceFileManager and uses injected/captured values at runtime. |
| include-build/gradle-plugin/src/main/java/com/telefonica/androidsnaptesting/AndroidSnaptestingPlugin.kt | Rewires task actions to avoid config-cache-incompatible listeners and execution-time project access, and keeps CI anchor tasks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Suppress("DEPRECATION") | ||
| val testedVariant = extension.testVariants | ||
| .firstOrNull { it.name == deviceProviderTask.variantName } | ||
| ?: throw RuntimeException("TestVariant not found for ${deviceProviderTask.variantName}") | ||
| val applicationIdProvider = providerFactory.provider { testedVariant.applicationId } | ||
| val adbExecutablePath = extension.adbExecutable.absolutePath |
There was a problem hiding this comment.
applicationIdProvider is evaluated during task execution via applicationIdProvider.get() in doFirst/doLast, and its supplier reads testedVariant.applicationId. This reintroduces execution-time access to an AGP variant object (and potentially project-backed state), which is typically incompatible with Gradle configuration cache and undermines the PR goal. Prefer resolving applicationId to a plain String during configuration (inside afterEvaluate) and capture that value in the task actions, or otherwise ensure the provider does not touch AGP variant/project state at execution time.
There was a problem hiding this comment.
should not be a real issue, because gradle is reporting that the configuration cache is being used in the local tests
| deviceProviderTask.doLast { | ||
| (it as DeviceProviderInstrumentTestTask) | ||
| .afterExecution( | ||
| applicationId = applicationIdProvider.get(), | ||
| adbExecutablePath = adbExecutablePath, | ||
| providerFactory = providerFactory, | ||
| isRecordMode = isRecordMode, | ||
| goldenSnapshotsSourcePath = goldenSnapshotsSourcePath, | ||
| ) | ||
| } |
There was a problem hiding this comment.
afterExecution() is now only invoked from a doLast action on DeviceProviderInstrumentTestTask. If the instrumented test task fails (e.g., test failures / snapshot mismatches), Gradle typically aborts remaining task actions, so this doLast may never run and reports/snapshot pulls won’t be generated. Previously the completion listener ensured the post-processing ran even when the task failed. Consider running the post-processing from a finalizer task that always executes (e.g., a non-empty androidSnaptestingAfter* task wired via finalizedBy) or another config-cache-safe mechanism that guarantees execution on failure.
There was a problem hiding this comment.
reviewing this...
There was a problem hiding this comment.
should be fixed at aeb1d53, according to tests done in android-messenger and SmartWifi, where reports are successfully generated now
no copilot tokens left, btw 😅
🎟️ Jira ticket
ANDROID-17457
🥅 What's the goal?
Fix configuration cache compatibility
🚧 How do we do it?
📘 Documentation changes?
🧪 How can I test this?