Open
Conversation
Add FeaturedPluginIntegrationTest that uses Gradle TestKit with a minimal Android application fixture to verify: - generateProguardRules produces correct -assumevalues rules - assembleRelease auto-wires the generated proguard file via Variant API Tests are skipped when ANDROID_HOME is not set. The testPluginClasspath configuration injects AGP into the TestKit subprocess classpath. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Review Summary by QodoAdd E2E integration test for Gradle plugin with Android fixture
WalkthroughsDescription• Add end-to-end integration test for Gradle plugin using TestKit • Test verifies ProGuard rules generation with correct -assumevalues blocks • Test verifies assembleRelease auto-wires generated ProGuard file via AGP Variant API • Add minimal Android application fixture for integration testing • Configure testPluginClasspath to inject AGP into TestKit subprocess Diagramflowchart LR
A["Test Configuration"] -->|injects AGP| B["testPluginClasspath"]
B -->|appended to| C["pluginUnderTestMetadata"]
C -->|loaded in| D["GradleRunner TestKit"]
D -->|executes| E["Android Fixture Project"]
E -->|verifies| F["ProGuard Rules Generation"]
E -->|verifies| G["assembleRelease Integration"]
File Changes1. featured-gradle-plugin/build.gradle.kts
|
Code Review by Qodo
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
There was a problem hiding this comment.
Pull request overview
Adds a Gradle TestKit-based end-to-end integration test for :featured-gradle-plugin using a minimal Android app fixture, aiming to validate ProGuard rule generation and (intended) release-variant wiring.
Changes:
- Introduce
FeaturedPluginIntegrationTestthat runsgenerateProguardRulesandassembleReleaseagainst a fixture Android app. - Add an Android fixture project under
src/test/fixtures/android-project/. - Extend
pluginUnderTestMetadataclasspath via a newtestPluginClasspathconfiguration (to inject AGP into the TestKit subprocess).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/FeaturedPluginIntegrationTest.kt | New TestKit E2E test verifying ProGuard output and (intended) wiring into assembleRelease. |
| featured-gradle-plugin/src/test/fixtures/android-project/build.gradle.kts | Minimal Android app applying AGP + Featured plugin and declaring a boolean local flag. |
| featured-gradle-plugin/src/test/fixtures/android-project/settings.gradle.kts | Fixture repositories/pluginManagement setup for TestKit build. |
| featured-gradle-plugin/src/test/fixtures/android-project/src/main/AndroidManifest.xml | Minimal manifest for the fixture app. |
| featured-gradle-plugin/build.gradle.kts | Adds testPluginClasspath and appends it to pluginUnderTestMetadata for TestKit. |
Comment on lines
+73
to
+83
| fun `assembleRelease wires proguard rules and completes successfully`() { | ||
| val result = | ||
| gradleRunner(projectDir) | ||
| .withArguments("assembleRelease", "--stacktrace") | ||
| .build() | ||
|
|
||
| // generateProguardRules must have run as part of the release build. | ||
| val proguardOutcome = result.task(":generateProguardRules")?.outcome | ||
| assertTrue( | ||
| proguardOutcome == TaskOutcome.SUCCESS || proguardOutcome == TaskOutcome.UP_TO_DATE, | ||
| "Expected :generateProguardRules to participate in assembleRelease, got $proguardOutcome\n${result.output}", |
There was a problem hiding this comment.
assembleRelease test currently expects :generateProguardRules to run as part of the Android release build, but FeaturedPlugin only registers the generateProguardRules task and does not wire it into any Android variant/task graph (no Variant API wiring in FeaturedPlugin). As-is, assembleRelease in the fixture won't invoke generateProguardRules, so this test should fail unless the wiring code is included in this PR branch. Either include the wiring change (from the dependent PR) in this branch or change the fixture/build to explicitly add the generated .pro file to proguardFiles(...) and adjust the assertion accordingly.
Suggested change
| fun `assembleRelease wires proguard rules and completes successfully`() { | |
| val result = | |
| gradleRunner(projectDir) | |
| .withArguments("assembleRelease", "--stacktrace") | |
| .build() | |
| // generateProguardRules must have run as part of the release build. | |
| val proguardOutcome = result.task(":generateProguardRules")?.outcome | |
| assertTrue( | |
| proguardOutcome == TaskOutcome.SUCCESS || proguardOutcome == TaskOutcome.UP_TO_DATE, | |
| "Expected :generateProguardRules to participate in assembleRelease, got $proguardOutcome\n${result.output}", | |
| fun `generateProguardRules and assembleRelease complete successfully`() { | |
| val result = | |
| gradleRunner(projectDir) | |
| .withArguments("generateProguardRules", "assembleRelease", "--stacktrace") | |
| .build() | |
| // Explicitly invoke generateProguardRules before assembleRelease for the current plugin behavior. | |
| val proguardOutcome = result.task(":generateProguardRules")?.outcome | |
| assertTrue( | |
| proguardOutcome == TaskOutcome.SUCCESS || proguardOutcome == TaskOutcome.UP_TO_DATE, | |
| "Expected :generateProguardRules to succeed before assembleRelease, got $proguardOutcome\n${result.output}", |
Comment on lines
+156
to
+169
| /** | ||
| * Resolves the fixture directory. The plugin module's project directory is either | ||
| * injected as the `user.dir` system property by Gradle's test task, or derived | ||
| * relative to this class file's location. | ||
| */ | ||
| private fun fixtureDir(): File { | ||
| // Gradle's test task sets user.dir to the module project directory. | ||
| val moduleDir = File(System.getProperty("user.dir")) | ||
| val candidate = moduleDir.resolve("src/test/fixtures/android-project") | ||
| require(candidate.isDirectory) { | ||
| "Fixture directory not found at ${candidate.absolutePath}. " + | ||
| "Expected it relative to module project dir: ${moduleDir.absolutePath}" | ||
| } | ||
| return candidate |
There was a problem hiding this comment.
The KDoc for fixtureDir() says the module directory is either provided via user.dir or derived relative to this class file's location, but the implementation only uses System.getProperty("user.dir") and has no fallback. Either implement the described fallback (e.g., resolve via a classpath resource) or update the KDoc to match the actual behavior to avoid confusion and brittle test setup.
Comment on lines
+175
to
+179
| * AGP is declared as `compileOnly` in this module — the applying build provides it at runtime. | ||
| * In the TestKit subprocess, AGP is loaded by the build's own classloader when `com.android.application` | ||
| * is applied from the fixture's `plugins {}` block (resolved from Google Maven). The Featured plugin | ||
| * code that references [com.android.build.api.variant.AndroidComponentsExtension] is loaded in the | ||
| * same classloader context, so no extra classpath injection is needed. |
There was a problem hiding this comment.
The gradleRunner() KDoc claims “no extra classpath injection is needed”, but this PR adds testPluginClasspath to inject AGP into pluginUnderTestMetadata. This comment is now misleading (and the classloader explanation is likely incorrect in TestKit). Please update the KDoc to reflect the actual mechanism being used so future changes don’t accidentally remove required classpath setup.
Suggested change
| * AGP is declared as `compileOnly` in this module — the applying build provides it at runtime. | |
| * In the TestKit subprocess, AGP is loaded by the build's own classloader when `com.android.application` | |
| * is applied from the fixture's `plugins {}` block (resolved from Google Maven). The Featured plugin | |
| * code that references [com.android.build.api.variant.AndroidComponentsExtension] is loaded in the | |
| * same classloader context, so no extra classpath injection is needed. | |
| * The runner uses [GradleRunner.withPluginClasspath] so TestKit loads the plugin-under-test from | |
| * the generated `pluginUnderTestMetadata` classpath. | |
| * | |
| * This test setup also relies on additional classpath wiring for that metadata so AGP is available | |
| * to the plugin under test, even though AGP is declared as `compileOnly` in this module. Keep that | |
| * wiring in place when changing the test configuration; removing it can break access to | |
| * [com.android.build.api.variant.AndroidComponentsExtension] during the TestKit build. |
| @@ -0,0 +1,29 @@ | |||
| plugins { | |||
| id("com.android.application") version "9.1.0" | |||
There was a problem hiding this comment.
The fixture applies com.android.application with an explicit version. With TestKit, this typically resolves/loads AGP via plugin resolution rather than from the injected withPluginClasspath() classpath, while this PR also injects com.android.tools.build:gradle into the plugin-under-test classpath. That combination can create classloader/type-identity issues (e.g., AndroidComponentsExtension loaded twice) and make Variant API lookups/casts fail. Prefer applying com.android.application without a version here so it uses the injected classpath, keeping AGP types consistent.
Suggested change
| id("com.android.application") version "9.1.0" | |
| id("com.android.application") |
| dependencies { | ||
| // Inject AGP into the TestKit subprocess via pluginUnderTestMetadata so that the Featured | ||
| // plugin can access AndroidComponentsExtension when wireProguardToVariants() is called. | ||
| testPluginClasspath("com.android.tools.build:gradle:9.1.0") |
There was a problem hiding this comment.
AGP version is hard-coded as 9.1.0 in the new testPluginClasspath dependency. Since the repo already tracks the AGP version in the version catalog (libs.versions.agp), this should be sourced from there to avoid accidental drift (tests may break if the plugin’s compileOnly AGP version changes but this stays pinned).
Suggested change
| testPluginClasspath("com.android.tools.build:gradle:9.1.0") | |
| testPluginClasspath("com.android.tools.build:gradle:${libs.versions.agp.get()}") |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| dependencies { | ||
| // Inject AGP into the TestKit subprocess via pluginUnderTestMetadata so that the Featured | ||
| // plugin can access AndroidComponentsExtension when wireProguardToVariants() is called. | ||
| testPluginClasspath("com.android.tools.build:gradle:9.1.0") |
There was a problem hiding this comment.
1. Hardcoded agp in testpluginclasspath 📘 Rule violation ⚙ Maintainability
The build script adds an AGP dependency with a literal version string (9.1.0) instead of sourcing the version from the Gradle version catalog. This can lead to version drift and violates the requirement to declare dependency versions centrally.
Agent Prompt
## Issue description
A dependency version is hardcoded as `9.1.0` in `featured-gradle-plugin/build.gradle.kts`, violating the requirement to declare dependency versions in the Gradle version catalog.
## Issue Context
`gradle/libs.versions.toml` already defines `agp = "9.1.0"`, but the build script still hardcodes the version in the dependency coordinate.
## Fix Focus Areas
- featured-gradle-plugin/build.gradle.kts[74-74]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| @@ -0,0 +1,29 @@ | |||
| plugins { | |||
| id("com.android.application") version "9.1.0" | |||
There was a problem hiding this comment.
2. Fixture uses plugin version literal 📘 Rule violation ⚙ Maintainability
The fixture project applies com.android.application with a hardcoded version (9.1.0) in `plugins
{}`. This violates the requirement to avoid literal version strings in Gradle build scripts.
Agent Prompt
## Issue description
The TestKit fixture `build.gradle.kts` applies `com.android.application` with a hardcoded plugin version string (`9.1.0`).
## Issue Context
This fixture is used for TestKit E2E tests and should not hardcode versions in the build script. If AGP is injected via the TestKit classpath, the fixture can typically apply the plugin without specifying a version.
## Fix Focus Areas
- featured-gradle-plugin/src/test/fixtures/android-project/build.gradle.kts[1-4]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+79
to
+84
| // generateProguardRules must have run as part of the release build. | ||
| val proguardOutcome = result.task(":generateProguardRules")?.outcome | ||
| assertTrue( | ||
| proguardOutcome == TaskOutcome.SUCCESS || proguardOutcome == TaskOutcome.UP_TO_DATE, | ||
| "Expected :generateProguardRules to participate in assembleRelease, got $proguardOutcome\n${result.output}", | ||
| ) |
There was a problem hiding this comment.
3. Assemblerelease not wired 🐞 Bug ≡ Correctness
FeaturedPluginIntegrationTest asserts :generateProguardRules runs during assembleRelease, but
the Featured plugin currently only registers the task and does not wire it into AGP variants, and
the fixture does not include the generated .pro file in proguardFiles. This will cause the test
to fail because result.task(":generateProguardRules") will be null or not executed during
assembleRelease.
Agent Prompt
## Issue description
The new E2E test expects `assembleRelease` to trigger `generateProguardRules`, but the plugin/fixture currently do not wire the generated ProGuard file into AGP’s release variant. This makes the test fail because `:generateProguardRules` will not be in the task graph for `assembleRelease`.
## Issue Context
- The plugin registers `generateProguardRules` and writes `build/featured/proguard-featured.pro`.
- The fixture’s `release` build type does not reference this file.
- There is no Android Components/Variant API integration in the plugin to add this file automatically.
## Fix Focus Areas
- featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedPlugin.kt[37-58]
- featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedPlugin.kt[99-111]
- featured-gradle-plugin/src/test/fixtures/android-project/build.gradle.kts[15-22]
- featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/FeaturedPluginIntegrationTest.kt[72-97]
## What to change
1. Implement actual AGP integration in the Featured plugin so that for release variants it:
- adds `build/featured/proguard-featured.pro` to the variant’s ProGuard/R8 configuration, and
- ensures the relevant R8/minify task depends on `generateProguardRules` (or otherwise consumes its output via the Variant API).
2. Keep the fixture minimal (it should not manually add the file if the plugin is supposed to auto-wire it).
3. If auto-wiring is not intended yet, adjust the test and fixture accordingly (but then remove the “auto-wires” claim and the `assembleRelease` participation assertion).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1
to
+4
| plugins { | ||
| id("com.android.application") version "9.1.0" | ||
| id("dev.androidbroadcast.featured") | ||
| } |
There was a problem hiding this comment.
4. Agp double-resolution risk 🐞 Bug ☼ Reliability
AGP is appended to the TestKit plugin classpath via pluginUnderTestMetadata, but the fixture still applies com.android.application with an explicit version. This can cause Gradle plugin resolution to fail because the plugin is already on the classpath while also being requested with a version.
Agent Prompt
## Issue description
AGP is injected into the TestKit subprocess classpath via `pluginUnderTestMetadata`, but the fixture still requests the Android plugin with an explicit version. This can break Gradle’s plugin resolution (plugin already on classpath + versioned request).
## Issue Context
The goal of `testPluginClasspath` is to make AGP available in the TestKit subprocess without relying on external plugin resolution.
## Fix Focus Areas
- featured-gradle-plugin/build.gradle.kts[57-75]
- featured-gradle-plugin/src/test/fixtures/android-project/build.gradle.kts[1-4]
- featured-gradle-plugin/src/test/fixtures/android-project/settings.gradle.kts[1-15]
## What to change
- Prefer one mechanism:
1) **Injected-classpath approach (recommended here):**
- Change fixture to `plugins { id("com.android.application"); id("dev.androidbroadcast.featured") }` (no version for AGP).
- Optionally simplify/remove `pluginManagement.repositories` if it’s no longer needed.
OR
2) **Repository resolution approach:**
- Remove AGP from `testPluginClasspath` injection and keep the fixture’s versioned plugin request.
Ensure the chosen approach is consistent with the comments in the fixture/settings files.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FeaturedPluginIntegrationTestusing Gradle TestKit with a minimal Android app fixturegenerateProguardRulesproduces correct-assumevaluesrules for boolean local flagsassembleReleaseauto-wires the generated ProGuard file via the Variant APItestPluginClasspathconfiguration to inject AGP into the TestKit subprocess classpathsrc/test/fixtures/android-project/with minimal Android setupTests are skipped when
ANDROID_HOME/ANDROID_SDK_ROOTis not set.Depends on #155
Test plan
./gradlew :featured-gradle-plugin:testwithANDROID_HOMEset🤖 Generated with Claude Code