Repository navigation
test(android): Add GradleTestKit tests for sentry.gradle.kts asset tasks - #6754
Merged
Merged
Conversation
…of source tree The Android Gradle plugin copied `sentry.options.json` into the version-controlled `src/main/assets` during builds via a task with no declared inputs/outputs. That broke Gradle's up-to-date checks and build caching for the asset-merge tasks and required a cleanup task to remove the file afterward, which could leave the file behind on a failed build. Replace it with a typed `generateSentryOptions` task that writes into `build/generated/sentry/options` with declared inputs/outputs (source file plus the SENTRY_ENVIRONMENT/RELEASE/DIST overrides), registered as a generated assets source via the AGP Variant API, with a classic sourceSets fallback for older AGP. Nothing is written into the source tree anymore, and asset merging is now correctly cached. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…out output The lint model/analysis tasks read the generated assets dir without a declared dependency on generateSentryOptions, which Gradle 9 fails as an implicit dependency error. Declare it explicitly. Also make the SENTRY_COPY_OPTIONS_FILE opt-out an @input instead of onlyIf, so disabling it re-runs the task and clears the output dir rather than leaving a stale generated file to be packaged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Older plugin versions copied the file into src/main/assets; a crashed build could leave it behind, where it now shadows or conflicts with the generated copy. Emit a configuration-time warning pointing the user to remove it. Never delete it automatically — the file may be intentional. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ce tree The Android Gradle plugin collected `modules.json` into the version-controlled `src/main/assets` directory on release builds via an `Exec` task with no declared inputs/outputs, then removed it afterward with a cleanup task. That broke up-to-date checks and build caching for asset merging, and a failed build could leave the file behind in the source tree. Replace it with a typed `CollectModulesTask` that writes into `build/generated/sentry/modules/<variant>`, registered as a generated assets source via the AGP Variant API (with a classic source-set fallback), so AGP merges and orders it into `merge<Variant>Assets` with correct up-to-date/caching behavior. Lint tasks depend on it explicitly (Gradle 9 rejects the previously implicit dependency), and the source-map cleanup now runs after it. Mirrors the `sentry.options.json` fix (#6751); closes the same anti-pattern for `modules.json` from #6750. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…arn on stale copy Addresses review feedback on the modules.json generated-assets change: - Source-set fallback (old AGP < 7.3) now registers each variant's generated modules dir into that variant's own source set instead of the shared "main". Adding per-variant dirs to "main" leaked modules.json into debug and caused a duplicate-asset merge conflict between multiple non-debug variants. Because each variant is now isolated, no dedup guard is required (unlike the shared-dir options fallback). The primary AGP 7.3+ variant-API path was already scoped and is unchanged. - Warn on a stale modules.json left in src/main/assets by older plugin versions (mirrors the sentry.options.json warning), since a leftover copy would clash with the generated one during asset merge on upgrade. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
Contributor
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3da007c+dirty | 424.56 ms | 448.54 ms | 23.98 ms |
| 38ab0b8+dirty | 426.03 ms | 446.65 ms | 20.62 ms |
| 1e3886c+dirty | 421.49 ms | 464.16 ms | 42.67 ms |
| 7a2d816+dirty | 470.94 ms | 515.62 ms | 44.68 ms |
| 3bacb46+dirty | 423.54 ms | 428.93 ms | 5.39 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3da007c+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| 38ab0b8+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| 1e3886c+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| 7a2d816+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| 3bacb46+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
Previous results on branch: al/gradle-asset-tasks-tests
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1a52662+dirty | 471.36 ms | 518.12 ms | 46.76 ms |
| 9945bc7+dirty | 487.80 ms | 540.44 ms | 52.64 ms |
| e04bc70+dirty | 431.27 ms | 455.69 ms | 24.42 ms |
| aa679e6+dirty | 426.69 ms | 455.90 ms | 29.20 ms |
| f20cf41+dirty | 426.83 ms | 446.08 ms | 19.25 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1a52662+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| 9945bc7+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| e04bc70+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| aa679e6+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| f20cf41+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
📲 Install BuildsAndroid
|
Contributor
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3da007c+dirty | 429.00 ms | 457.34 ms | 28.34 ms |
| 38ab0b8+dirty | 417.88 ms | 431.80 ms | 13.92 ms |
| 1e3886c+dirty | 434.40 ms | 470.29 ms | 35.89 ms |
| 7a2d816+dirty | 433.18 ms | 458.43 ms | 25.25 ms |
| 3bacb46+dirty | 428.24 ms | 459.82 ms | 31.58 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3da007c+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| 38ab0b8+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| 1e3886c+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| 7a2d816+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| 3bacb46+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
Previous results on branch: al/gradle-asset-tasks-tests
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1a52662+dirty | 425.61 ms | 440.60 ms | 14.99 ms |
| 9945bc7+dirty | 447.71 ms | 506.08 ms | 58.37 ms |
| e04bc70+dirty | 422.98 ms | 441.69 ms | 18.71 ms |
| aa679e6+dirty | 452.06 ms | 474.58 ms | 22.52 ms |
| f20cf41+dirty | 429.12 ms | 447.57 ms | 18.45 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1a52662+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| 9945bc7+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| e04bc70+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| aa679e6+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
| f20cf41+dirty | 50.56 MiB | 56.50 MiB | 5.93 MiB |
This was referenced Sep 18, 2026
antonis
marked this pull request as ready for review
September 18, 2026 13:04
…fix-gradle-modules-generated-assets
…fingerprint the collect-modules script Addresses two review findings on the modules.json generation change: - The lint-task dependency used a loose `it.name.contains(variantCapitalized)` substring match, so a longer build type whose name ends in a shorter one (e.g. `qaRelease` vs `release`) would have its lint tasks pull in the wrong variant's modules task. Match the variant as a full task-name segment via the known AGP lint verbs / `generate<Variant>Lint…` prefix instead, mirroring the exact match already used for the `merge<Variant>Assets` fallback wiring. - `collectModulesScript` was an `@Input` on the path string only, which does not fingerprint the script's content — editing it in place (e.g. an SDK upgrade at the same path) would not re-run the task. Make it `@Internal` and add `collectModulesScriptFiles` (`@InputFiles`, RELATIVE path sensitivity) to fingerprint content, tolerating a missing script as an empty input. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
antonis
force-pushed
the
al/gradle-asset-tasks-tests
branch
from
September 21, 2026 13:02
69a44c1 to
14b2a29
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 14b2a29. Configure here.
antonis
force-pushed
the
al/gradle-asset-tasks-tests
branch
from
September 21, 2026 13:51
14b2a29 to
6be74ae
Compare
…stic Match a variant's whole AGP lint family by the `Lint` camelCase word boundary (name starts with the lowercase `lint` verb, or embeds a capital-L `Lint` segment) scoped to that variant, instead of an allowlist of lint verbs plus a generator-prefix regex. This covers `updateLintBaseline*`, `*UnitTest` lint models, and any future AGP lint task without relying on those tasks transitively depending on an allowlisted `lintAnalyze*` task. The word boundary — rather than a case-insensitive `lint` substring — excludes unrelated `ktlint*` tasks (e.g. `ktlintReleaseCheck`), which must not be made to depend on the modules task and pull in the JS bundler. Cross-variant matches (e.g. `release` inside `qaRelease`/`releaseStaging`) are excluded precisely using the set of processed variant names, which is complete by the time lint tasks are realized. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
antonis
force-pushed
the
al/fix-gradle-modules-generated-assets
branch
from
September 21, 2026 14:03
9d22855 to
a2fcc04
Compare
antonis
force-pushed
the
al/gradle-asset-tasks-tests
branch
from
September 21, 2026 14:03
6be74ae to
c89d8ba
Compare
… races The CollectModulesTask was decoupled from the source-map upload chain so it runs via mergeAssets, which introduced two races with the upload flow: - The upload rewrites the source map in place (copy-debugid) in its doFirst, while the modules task reads that same file. With nothing ordering them, a parallel / configuration-cache build could read a half-written map and fail JSON parsing. Order the upload strictly after the modules task. - The "clean up extra sourcemap" task deletes the source map after uploading. Fingerprinting that (deleted) map as the task input invalidated the modules task on every rebuild and could package an empty modules.json when an up-to-date bundle task didn't regenerate the map. Fingerprint the stable bundle instead (it changes iff the JS/module list changes) and read the map at execution as an @internal input. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
antonis
force-pushed
the
al/gradle-asset-tasks-tests
branch
from
September 21, 2026 14:30
c89d8ba to
4367636
Compare
`resolveSentryReactNativeSDKPath` spawns a blocking `node --print require.resolve(...)` subprocess. It was hoisted into the plain body of `processVariant`, which the AGP `onVariants` callback runs at configuration time for every non-debug variant on every Gradle invocation (including debug builds and `./gradlew tasks`). Its result also feeds only the fallback branch, so it was wasted whenever `collectModulesScript` is configured. Move the resolution into the CollectModulesTask config block and into the `?:` fallback only, so the node subprocess runs lazily (when the task is realized) and conditionally (when no explicit script path is set) — restoring the pre-refactor behavior where it lived inside the Exec task's config block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover the generated-asset tasks introduced for issue #6750 (PRs #6751/#6753) with functional tests that apply the real sentry.gradle.kts script plugin to a minimal AGP fixture and drive the tasks through GradleTestKit: - SentryOptionsTaskTest: plain copy, SENTRY_RELEASE override, opt-out via SENTRY_COPY_OPTIONS_FILE=false, and missing-source fallback. - SentryModulesTaskTest: modules.json generated into build/ (never src/main/ assets), UP-TO-DATE on rebuild, skipCollectModules opt-out, and missing source map fallback. Runs on the existing RNSentryAndroidTester testDebugUnitTest job, so no new CI infrastructure is required. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pendency Two-build-type fixture (`release` + `qaRelease`) proving `lintQaRelease` does not depend on the `release` variant's modules task. Verified red-capable: it fails against the old `it.name.contains(variantCapitalized)` substring match and passes with the token-boundary scoping. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pass a SENTRY_*-scrubbed environment to GradleRunner in both asset-task test classes so the assertions depend only on the fixture and each test's explicit env, not on SENTRY_RELEASE/ENVIRONMENT/DIST/COPY_OPTIONS_FILE a developer may have exported locally. Also extend the variant-scoping regression to assert the non-`lint*`-prefixed `updateLintBaselineQaRelease` is likewise not wired to the release modules task, and document that `lintRelease` is a non-vacuity control (in this fixture lint depends on the modules task only through the explicit wiring). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Guards the lint-family matcher against over-binding: a `ktlint*` task (from the ktlint Gradle plugin) carries a lowercase `lint` mid-name and the variant name but is not an AGP lint task, so it must not depend on CollectModulesTask and pull in the JS bundler. Fails against a case-insensitive `lint` substring match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a GradleTestKit regression test asserting the modules task stays UP-TO-DATE — and its modules.json is preserved, not emptied — when the source map it reads is deleted (as the upload cleanup does) and the bundle is unchanged. The bundle stub now also writes the --bundle-output file (the task's stable fingerprint) and can declare it as an output so the bundle task goes up-to-date on rerun, reproducing the real bundle task that does not track the forced source map as an output. Red against a source-map-fingerprinted task (re-runs, finds no map, packages an empty assets dir); green once the fingerprint is the bundle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
antonis
force-pushed
the
al/gradle-asset-tasks-tests
branch
from
September 22, 2026 07:08
4367636 to
5217d5e
Compare
GenerateSentryOptionsTask.generate() parses the source as JSON before applying SENTRY_ENVIRONMENT/RELEASE/DIST overrides, and on a parse failure logs a warning and copies the source verbatim rather than failing the build. That catch branch was untested (Warden flagged the coverage gap on #6751). Add a case with an override set and a malformed source, asserting the task succeeds and the output matches the source byte-for-byte — verbatim equality proves the fallback ran, since a successful parse would re-serialize the JSON. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
alwx
requested changes
Sep 22, 2026
| File(projectDir, "build.gradle").writeText( | ||
| """ | ||
| plugins { | ||
| id 'com.android.application' version '8.3.2' |
Contributor
There was a problem hiding this comment.
does it actually have to be hardcoded here (and below as well)? If we bump AGP, the fixtures will keep testing 8.3.2
| ] | ||
| android { | ||
| namespace 'io.sentry.fixture' | ||
| compileSdk 34 |
Contributor
There was a problem hiding this comment.
compileSdk 34 in the fixture while the host project pins 33
|
|
||
| val bundleFile = File(projectDir, "build/generated/assets/react/release/index.android.bundle") | ||
| val sourcemapFile = File(projectDir, "build/generated/sourcemaps/react/release/index.android.bundle.map") | ||
|
|
Contributor
There was a problem hiding this comment.
both files (this and SentryOptionsTaskTest.kt) have identical shared code (settings.gradle, local.properties, manifest) that can be extracted to some common base or helper
Address review feedback on the GradleTestKit fixtures: - Stop hardcoding AGP 8.3.2 and compileSdk 34 in the fixtures. The host build now injects its own AGP version (com.android.Version) and compileSdk as system properties, and the fixtures read them, so bumping the host no longer leaves the fixtures silently testing a stale toolchain. This also fixes the fixture pinning compileSdk 34 while the host is on 33. - Extract the duplicated scaffolding (settings.gradle, local.properties, stub manifest, path escaping, SENTRY_* env scrubbing, system-property reads) from both test classes into a shared BaseSentryGradleTest base class. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
alwx
approved these changes
Sep 22, 2026
Base automatically changed from
al/fix-gradle-modules-generated-assets
to
main
September 23, 2026 07:27
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
3 participants
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.
📢 Type of change
📜 Description
Adds test for PRs #6751 and #6753
💡 Motivation and Context
Closes #6752
💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps