Skip to content

feat(snapshot): Merge snapshot extension into main sentry DSL#1136

Merged
runningcode merged 16 commits intomainfrom
no/consolidate-snapshots-extension
Apr 2, 2026
Merged

feat(snapshot): Merge snapshot extension into main sentry DSL#1136
runningcode merged 16 commits intomainfrom
no/consolidate-snapshots-extension

Conversation

@runningcode
Copy link
Copy Markdown
Contributor

@runningcode runningcode commented Apr 1, 2026

Summary

  • Moves snapshot configuration from the standalone sentrySnapshot plugin into the main sentry { snapshots { } } extension
  • Auto-detects Paparazzi plugin for test generation wiring (no enabled flag needed)
  • Wires sentryUploadSnapshots task to Paparazzi output directory and ordering
  • Uses consistent plural "snapshots" naming throughout
  • Upload tasks remain registered for all variants (library-agnostic), test generation is Paparazzi-specific

DSL before

// Separate plugin required
plugins { id("io.sentry.android.snapshot") }

sentrySnapshot {
    includePrivatePreviews = true
    packageTrees = listOf("com.example.app")
    theme = "MyTheme"
}

DSL after

sentry {
    snapshots {
        includePrivatePreviews = true
        packageTrees = listOf("com.example.app")
        theme = "MyTheme"
    }
}

#skip-changelog

🤖 Generated with Claude Code

Move snapshot configuration from the standalone `sentrySnapshot` plugin
into the main `sentry { snapshots { } }` extension. This consolidates
the DSL surface and auto-detects Paparazzi for test generation.

- Add `SnapshotsExtension` nested under `SentryPluginExtension`
- Move Paparazzi detection and test generation wiring into `SentryPlugin`
- Wire `sentryUploadSnapshots` task to Paparazzi output directory
- Rename generate task to `sentryGenerateSnapshotsTests` for consistency
- Remove standalone `SentrySnapshotPlugin` and `SentrySnapshotExtension`
- Use plural "snapshots" naming consistently

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2026

Semver Impact of This PR

🟡 Minor (new features)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


This PR will not appear in the changelog.


🤖 This preview updates automatically when you update the PR.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2026

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against cf3b2bb

Use dependsOn to ensure Paparazzi record task runs before upload,
and derive snapshotsPath from the test task's output directory
instead of a hardcoded path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@romtsn romtsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I guess I'd still prefer to have an enabled flag -- in case something goes off with the snapshots' tasks, people that don't use snapshots could simply disable the feature. Because now we're configuring those tasks unconditionally, right (even though they don't execute without the paparazzi plugin applied)?

Gate test generation and dependency addition on snapshots.enabled
so that applying Paparazzi alongside the Sentry plugin doesn't
affect normal Paparazzi task execution. Defaults to false.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@runningcode
Copy link
Copy Markdown
Contributor Author

Actually, that's a really good point. I built it so that all the tasks are only registered and not dependencies of other tasks unless you have the paparazzi plugin applied.
The issue is that some people might already be using paparazzi so having this auto-apply is a bad idea. Thank you! i've updated it

@runningcode runningcode marked this pull request as ready for review April 1, 2026 13:33
runningcode and others added 2 commits April 1, 2026 15:43
Remove the sentrySnapshotPlugin marker distribution and publication
task references that were left behind when the plugin was removed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The upload task and its Paparazzi wiring were registered
unconditionally for all variants. This caused projects using both
the Sentry plugin and Paparazzi to get unwanted task dependencies
even with snapshots disabled.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
runningcode and others added 2 commits April 1, 2026 16:21
testImplementation is a global configuration, not per-variant.
Move the ComposablePreviewScanner dependency addition to
afterEvaluate (gated on enabled) so it runs once instead of
once per variant.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sentry-release-bot bot and others added 4 commits April 1, 2026 16:04
The testDebugUnitTest task is not yet registered by AGP when
onVariants runs. Wrap the Paparazzi wiring in afterEvaluate so
the test task exists when we look it up.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tsConfig

Move test generation, dependency addition, and upload task wiring
into configureSnapshotsTasks so all snapshot logic lives in one
place. Use the AGP hostTests/unitTest API for generated source
wiring, and afterEvaluate for the test task output lookup.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines +495 to +497
"testImplementation",
"io.github.sergio-sastre.ComposablePreviewScanner:android:0.8.1",
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The code unsafely calls .first() to find a "snapshots" directory, which will crash the build if the directory is not found.
Severity: MEDIUM

Suggested Fix

Replace the call to .first() with a safer alternative like .firstOrNull(). If the result is null (meaning the directory was not found), you should handle it gracefully, for example by logging a warning or skipping the task's operation, to prevent the build from crashing.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
plugin-build/src/main/kotlin/io/sentry/android/gradle/AndroidComponentsConfig.kt#L495-L497

Potential issue: The code at line 526 calls `.first()` on a collection of files to find
one named "snapshots". If the Paparazzi test task runs but does not produce a
"snapshots" output directory for any reason, this call will throw a
`NoSuchElementException` during Gradle's execution phase, causing the build to fail.
This can happen if the Paparazzi configuration is altered or if it behaves unexpectedly
for a specific variant.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Not all variants have unit test tasks (e.g. users can disable them),
so skip wiring the snapshot upload task if the test task doesn't exist.
Uses tasks.names to avoid eagerly realizing tasks.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adding the testImplementation dependency inside a nested afterEvaluate
risks the configuration already being resolved and immutable. The
withPlugin callback already fires during configuration, so the wrapper
is unnecessary.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

runningcode and others added 2 commits April 2, 2026 11:35
The testImplementation dependency for ComposablePreviewScanner was
being added N times (once per variant) since configureSnapshotsTasks
runs inside onVariants. Move it to a single withPlugin block before
onVariants to add it only once.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines +523 to +524
project.afterEvaluate {
// Not all variants have unit test tasks (e.g. users can disable them),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The SentryUploadSnapshotsTask is registered unconditionally but its required snapshotsPath property is not set if the Paparazzi plugin is missing or if unit tests are disabled for a variant, causing a build failure.
Severity: HIGH

Suggested Fix

Move the registration of SentryUploadSnapshotsTask inside the project.pluginManager.withPlugin("app.cash.paparazzi") block. This ensures the task is only created when its required snapshotsPath property can be correctly configured, preventing build failures when Paparazzi is not in use.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
plugin-build/src/main/kotlin/io/sentry/android/gradle/AndroidComponentsConfig.kt#L523-L524

Potential issue: The `SentryUploadSnapshotsTask` is registered if
`extension.snapshots.enabled` is true. However, its required `snapshotsPath` property is
only configured under specific conditions. This leads to build failures in two
scenarios: 1) When the Paparazzi plugin is not applied, the configuration block is
skipped, leaving `snapshotsPath` unassigned. 2) When the Paparazzi plugin is applied but
unit tests are disabled for a specific build variant (e.g., `release`), an early return
in an `afterEvaluate` block prevents `snapshotsPath` from being set for that variant's
task. In both cases, Gradle's task validation fails because a required property is
missing.

@runningcode runningcode merged commit a02999b into main Apr 2, 2026
21 checks passed
@runningcode runningcode deleted the no/consolidate-snapshots-extension branch April 2, 2026 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants