Skip to content

Upload Snapshots to Sentry using SnapshotPreview and SwiftSnapshotTesting libraries#780

Open
cameroncooke wants to merge 23 commits intomainfrom
cameroncooke/snapshots-ci
Open

Upload Snapshots to Sentry using SnapshotPreview and SwiftSnapshotTesting libraries#780
cameroncooke wants to merge 23 commits intomainfrom
cameroncooke/snapshots-ci

Conversation

@cameroncooke
Copy link
Copy Markdown

@cameroncooke cameroncooke commented Apr 1, 2026

This branch exists just to test the unreleased now merged changes to SnapshowPreviews on the here: EmergeTools/SnapshotPreviews#252

See workflow run which is using SnapshotPreviews with CI export functionality:
https://github.com/EmergeTools/hackernews/actions/runs/23857485626/job/69554642234?pr=780

Resulting Sentry Snapshots URL:
https://sentry.sentry.io/preprod/snapshots/177173/

Comment thread .github/scripts/ios/emerge-snapshots Outdated
@emerge-tools
Copy link
Copy Markdown

emerge-tools bot commented Apr 3, 2026

📸 Snapshot Test

2 modified, 70 unchanged

Name Added Removed Modified Renamed Unchanged Errored Approval
HackerNews
com.emergetools.hackernews.snapshots
0 0 2 0 70 0 ⏳ Needs approval

🛸 Powered by Emerge Tools

@sentry
Copy link
Copy Markdown

sentry bot commented Apr 3, 2026

📲 Install Builds

iOS

🔗 App Name App ID Version Configuration
HackerNews com.emergetools.hackernews 3.10 (1) Release
HackerNews com.emergetools.hackernews 3.10 (1) AdHoc

⚙️ launchpad-test-ios Build Distribution Settings

Comment thread .github/scripts/ios/emerge-snapshots Outdated
@cameroncooke cameroncooke force-pushed the cameroncooke/snapshots-ci branch 3 times, most recently from 7502a1a to beb2055 Compare April 7, 2026 09:59
Comment thread .github/workflows/ios_sentry_upload_snapshots.yml Outdated
Comment thread .github/workflows/ios_sentry_upload_snapshots.yml Outdated
Comment thread ios/HackerNews.xcodeproj/xcshareddata/xcschemes/HackerNews.xcscheme Outdated
cameroncooke and others added 5 commits April 7, 2026 12:11
…p crash

The test process working directory differs from the shell's, so a
relative path caused the SnapshotCIExportCoordinator to fail creating
the export directory, triggering a preconditionFailure (SIGTRAP) during
test bootstrapping.

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

Move TEST_RUNNER_SNAPSHOTS_EXPORT_DIR and XCODE_RUNNING_FOR_PREVIEWS into
the job-level env block so both iPhone and iPad test steps share the same
absolute export path. Add a dedicated boot step for the iPad simulator and
clarify the existing boot step name.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@cameroncooke cameroncooke force-pushed the cameroncooke/snapshots-ci branch from 537b41f to 4b99f38 Compare April 7, 2026 11:11
Run the snapshot upload workflow on PRs targeting main in addition to
pushes, scoped to ios and workflow file paths for both triggers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread .github/workflows/ios_sentry_upload_snapshots.yml Outdated
cameroncooke and others added 2 commits April 7, 2026 12:33
Add upload_sentry_preview_snapshots lane that uploads snapshot images
generated by SnapshotPreviews to Sentry, separate from the existing
swift-snapshot-testing upload lane.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread ios/HackerNews.xcodeproj/project.pbxproj Outdated
cameroncooke and others added 2 commits April 7, 2026 14:43
Add branch-scoped DerivedData caching and split the single xcodebuild
test into build-for-testing + test-without-building to improve CI
wall-clock time and give better visibility into build vs test duration.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move sim boot before cache steps so it boots during cache restore
- Pin DerivedData to a fixed path with -derivedDataPath so cache
  reliably hits across runs
- Add -skipPackageUpdates to avoid ~2min SPM resolution (Package.resolved
  is committed)
- Enable COMPILATION_CACHING, EAGER_LINKING, FUSE_BUILD_SCRIPT_PHASES
  for faster incremental and zero-change builds
- Strip redundant build settings from test-without-building step
- Simplify SPM cache path (DD cache now covers SourcePackages)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread .github/workflows/ios_sentry_upload_snapshots.yml
cameroncooke and others added 4 commits April 8, 2026 09:42
DerivedData cache already contains SourcePackages, and
-skipPackageUpdates prevents xcodebuild from fetching. The SPM cache
was downloading 759MB for no benefit — removing it saves ~4 minutes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The split into build-for-testing + test-without-building was paying
~2min xcodebuild startup tax twice. For 23 snapshot tests that run in
10s, the visibility tradeoff isn't worth ~1min of added overhead.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread .github/workflows/ios_sentry_upload_snapshots.yml Outdated
cameroncooke and others added 2 commits April 8, 2026 11:52
-skipPackageUpdates prevents xcodebuild from fetching new commits when
Package.resolved changes, causing failures. Since the DD cache is keyed
on branch name (not Package.resolved), we need to allow SPM to fetch
deltas. -skipPackagePluginValidation still skips plugin trust prompts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
DerivedData cache download time was too variable (1.5-5 min) on GHA
network, often outweighing build savings. Restore the original SPM
cache which is smaller and more predictable. Keep COMPILATION_CACHING
and other build settings which work independently of DD caching.
Also removed -derivedDataPath to use default location (matches SPM
cache paths).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cameroncooke and others added 2 commits April 8, 2026 14:51
xcodebuild was matching both arm64 and x86_64 for the same simulator,
triggering a warning and potentially slowing destination resolution.
Pinning arch=arm64 eliminates the ambiguity on Apple Silicon runners.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add a new GitHub Actions workflow that runs SwiftSnapshotTest and
uploads the generated images to Sentry. Recording is controlled via
the SNAPSHOT_TESTING_RECORD env var instead of hardcoding record: .all
in the test file, so local runs default to reference comparison while
CI always records fresh snapshots.

- New workflow: ios_sentry_upload_swift_snapshots.yml
- Remove invokeTest() override from SwiftSnapshotTest.swift
- Use continue-on-error on test step since record mode fails assertions

Refs EME-1030
Co-Authored-By: Claude <noreply@anthropic.com>
@sentry
Copy link
Copy Markdown

sentry bot commented Apr 15, 2026

Sentry Snapshot Testing

Name Added Removed Modified Renamed Unchanged Status
com.emergetools.hackernews
com.emergetools.hackernews
0 0 0 0 18 ✅ Unchanged

⚙️ hackernews-ios Snapshot Settings

Comment thread ios/HackerNews.xcodeproj/project.pbxproj Outdated
super.invokeTest()
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

intentional?

Copy link
Copy Markdown
Author

@cameroncooke cameroncooke Apr 16, 2026

Choose a reason for hiding this comment

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

@NicoHinderling Yes, because Swift Snapshot Testing does diffing locally, we would only want to enable recording on CI workflows. We use an environmental variable in the GitHub workflow example TEST_RUNNER_SNAPSHOT_TESTING_RECORD which causes the library to record. This allows the user to run tests and catch regressions locally before pushing via the Pointfree library's own diff algo and on CI it will just be used to export images. It's also worth noting enabling recording causes XCTest failures which we ignore on CI but will run as real regression tests locally.

# COMPILATION_CACHING=YES \
# EAGER_LINKING=YES \
# FUSE_BUILD_SCRIPT_PHASES=YES \
# | xcpretty
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think lets actually uncomment and keep this in here! I think it's a great example :)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@NicoHinderling There is actually one issue at the moment the filenames don't include the simulator device info or runtime so iPad images would overwrite the iOS ones. What do we think the best strategy is for users on this?

  • Should we use nested folders does that name space on Sentry?
  • Should we just rename the files adding a prefix or suffix post generation?
  • Should the library automatically name images based on device/runtime?

Some things that are not clear to me is how we disambiguate on the Sentry end and what the expected exp. is for multiple platform/device snapshots.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getsentry/sentry-cli#3269

i actually just merged a PR related to this yesterday to add proper subdirectory support within the snapshots images-to-upload folder...

so I think two things:

  1. In our example, we should upload the ipad and iphone images as nested subdirectories, that way the image_file_name value will be unique for all images
  2. the deviceType is exactly the kind of metadata that the snapshotPreviews library should expose that we can then pass into the JSON sidecar metadata thanks to your PR :)

As a reminder, the setup currently allows you to include whatever extra fields you want in the metadata upload, so could you make sure for this example we include declaredDevice/simulatorDeviceName in this upload?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Cool so we already upload simulatorDeviceName:

{
  "baseFileName" : "Comment_View_Indentation_Preview_Indentation_0",
  "display_name" : "Indentation 0",
  "group" : "Comment View Indentation Preview",
  "image_file_name" : "Comment_View_Indentation_Preview_Indentation_0",
  "orientation" : "portrait",
  "previewDisplayName" : "Indentation 0",
  "previewId" : "0",
  "previewIndex" : 0,
  "simulatorDeviceName" : "iPad Air 11-inch (M4)",
  "simulatorModelIdentifier" : "iPad16,9",
  "testName" : "-[HackerNewsSnapshotTest portrait-Comment View Indentation Preview-0-5]",
  "typeDisplayName" : "Comment View Indentation Preview",
  "typeName" : "HackerNews.CommentViewIndentation_Preview"
}

Copy link
Copy Markdown
Author

@cameroncooke cameroncooke Apr 17, 2026

Choose a reason for hiding this comment

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

So just tested with nested folders with duplicate file names and it works well. Only thing I'm not sure about is how the user disambiguates snapshot variants in the Sentry UI.

Ideas:

  1. We just leave it up to users to add a disambiguator i.e. suffix to the display_name field, though for Snapshots we still have to have an opinionated format as we generate the value dynamically.
  2. We support nested groups maybe we can parse / in the group name and show some kind of nested ui
  3. We add/use a field from json metadata that we then use in the UI to disambiguate in addition to display_name and group.

Be good to get your thoughts on this @NicoHinderling

Upstream SnapshotPreviews CI-export changes have merged to main,
so the temporary cameroncooke/snapshot-ci branch pin is no longer
needed. Reverts both project.pbxproj and Package.resolved to match
main.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread .github/workflows/ios_sentry_upload_swift_snapshots.yml
@NicoHinderling NicoHinderling changed the title [do not merge] Use SnapshotPreviews from Cameroncooke/snapshots ci branch Use SnapshotPreviews from Cameroncooke/snapshots ci branch Apr 16, 2026
Split SnapshotPreviews output into iphone/ and ipad/ subdirectories under a
shared base dir, re-enable the previously commented-out iPad snapshot test
run on iPad Air 11-inch (M4), and upload the combined tree to Sentry in one
call.

Rename the Fastfile lanes so each tool has a clear name: the
SnapshotPreviews lane becomes `upload_sentry_snapshots` and accepts a
`path` option, while the swift-snapshot-testing lane becomes
`upload_sentry_snapshots_swift_snapshot_testing`. Update both workflows
to call the renamed lanes.

Also strip trailing whitespace in `export_thinned_build`.
@cameroncooke cameroncooke changed the title Use SnapshotPreviews from Cameroncooke/snapshots ci branch Upload Snapshots to Sentry using SnapshotPreview and SwiftSnapshotTesting libraries Apr 17, 2026
Comment thread .github/workflows/ios_sentry_upload_snapshots.yml Outdated
Add `arch=arm64` to the iPad Air simulator destination in the Sentry snapshot upload workflow to skip destination disambiguation, matching the existing pattern used for the iPhone destination.
Drop the fastlane upload_sentry_snapshots step from the Emerge snapshot
workflow so snapshots are only uploaded to Emerge.
Build the snapshot test bundle once with build-for-testing targeting both
iPhone and iPad device families, then run test-without-building for each
simulator. This avoids rebuilding the test bundle per destination and
keeps iPhone and iPad runs sharing the same compiled artifacts.

Route DerivedData to a workspace-local path so the SPM cache key and
subsequent test runs point at a stable location.
Comment on lines +93 to +103
env:
TEST_RUNNER_SNAPSHOTS_EXPORT_DIR: "${{ env.SNAPSHOT_UPLOAD_BASE_DIR }}/ipad"
run: |
set -o pipefail && xcodebuild test-without-building \
-scheme HackerNews \
-sdk iphonesimulator \
-destination 'platform=iOS Simulator,name=iPad Air 11-inch (M4),arch=arm64' \
-only-testing:HackerNewsTests/HackerNewsSnapshotTest \
-resultBundlePath ../SnapshotResults-ipad.xcresult \
-derivedDataPath "${DERIVED_DATA_PATH}" \
| xcpretty
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 iPad snapshot generation step lacks continue-on-error: true, making it less resilient to transient failures compared to similar steps in other workflows.
Severity: LOW

Suggested Fix

Add continue-on-error: true to the "Generate snapshot images (iPad)" step. This will align its behavior with other snapshot testing jobs and prevent the entire workflow from failing due to non-critical, transient test issues.

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: .github/workflows/ios_sentry_upload_snapshots.yml#L92-L103

Potential issue: The "Generate snapshot images (iPad)" step in the
`ios_sentry_upload_snapshots.yml` workflow does not set `continue-on-error: true`. This
is inconsistent with similar snapshot testing steps in other workflows like
`ios_sentry_upload_swift_snapshots.yml` and `ios_emerge_upload_snapshots.yml`, which do
use this flag. Without it, the entire workflow could fail due to transient issues with
the iPad simulator or the test itself, reducing the overall reliability of the CI
pipeline. While a race condition with simulator boot-up is mitigated by a preceding
step, this lack of error handling introduces unnecessary brittleness.

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.

2 participants