Skip to content

feat(snapshots): Add CI image export via SNAPSHOTS_EXPORT_DIR env var (EME-931)#252

Merged
cameroncooke merged 11 commits intomainfrom
cameroncooke/snapshot-ci
Apr 8, 2026
Merged

feat(snapshots): Add CI image export via SNAPSHOTS_EXPORT_DIR env var (EME-931)#252
cameroncooke merged 11 commits intomainfrom
cameroncooke/snapshot-ci

Conversation

@cameroncooke
Copy link
Copy Markdown
Contributor

@cameroncooke cameroncooke commented Apr 1, 2026

This branch adds filesystem-based snapshot export for CI flows and makes exported snapshot naming and metadata deterministic across the preview scenarios we cover.

Summary

  • Adds an env-var-gated CI export path via SNAPSHOTS_EXPORT_DIR
  • Writes snapshot PNGs plus flattened JSON sidecars through SnapshotCIExportCoordinator
  • Uses shared canonical rules for exported group and display_name metadata
  • Sanitizes snapshot baseFileName at construction time so attachments and exports use the same stable names
  • Skips oversized snapshot exports above the 40M pixel limit before writing PNGs that the upload CLI would reject

Naming and metadata behavior

Filename and sidecar naming now cover the main preview cases explicitly:

  • Unique named previews keep their display name in the filename
  • Duplicate preview display names within the same canonical group fall back to a disambiguator
  • Preview macros fall back to line-<n> when needed
  • Unnamed PreviewProvider variants fall back to the preview index
  • Exported group resolves as fileId -> typeDisplayName -> typeName
  • Exported display_name resolves as explicit preview display name -> At line #N -> preview index

Artifacts:

Images and sidecar files from HackerNews local run:
HackerNewsSnapshots.zip

Latest workflow run https://github.com/EmergeTools/hackernews/actions/runs/23940370307/job/69825189739?pr=780
Sentry Snapshots Viewer: https://sentry.sentry.io/preprod/snapshots/180875/

Example

HackerNews_FeedScreen.swift_161_Has_posts.png:

HackerNews_FeedScreen swift_161_Has_posts

HackerNews_FeedScreen.swift_161_Has_posts.json:

{
  "baseFileName" : "HackerNews_FeedScreen.swift_161_Has_posts",
  "display_name" : "Has posts",
  "fileId" : "HackerNews\/FeedScreen.swift",
  "group" : "HackerNews\/FeedScreen.swift",
  "image_file_name" : "HackerNews_FeedScreen.swift_161_Has_posts",
  "line" : 161,
  "orientation" : "portrait",
  "previewDisplayName" : "Has posts",
  "previewId" : "0",
  "previewIndex" : 0,
  "simulatorDeviceName" : "iPhone 17 Pro Max",
  "simulatorModelIdentifier" : "iPhone18,2",
  "testName" : "-[HackerNewsSnapshotTest portrait-Feed Screen-0-19]",
  "typeDisplayName" : "Feed Screen",
  "typeName" : "HackerNews.$s10HackerNews0021FeedScreenswift_IfFDefMX160_0_33_2B34B1DE919BF62EDDE40FEC34EF23D3Ll7PreviewfMf2_15PreviewRegistryfMu_"
}

Same snapshot in Sentry UI:

Screenshot 2026-04-07 at 11 45 52

Testing

Run tests with TEST_RUNNER_SNAPSHOTS_EXPORT_DIR set:

 TEST_RUNNER_SNAPSHOTS_EXPORT_DIR=/tmp/snapshots \
  xcodebuild test \
    -scheme DemoAppTests \
    -destination 'platform=iOS Simulator,name=iPhone 17 Pro Max'

View files

ls -la /tmp/snapshots

Upload to Sentry

sentry build snapshots \
    --app-id "snapshots-test" \
    --head-sha "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" \
    --base-sha "0000000000000000000000000000000000000000" \
    --head-ref "main" \
    --base-ref "main" \
    --head-repo-name "getsentry/SnapshotPreviewsTest" \
    --base-repo-name "getsentry/SnapshotPreviewsTest" \
    --vcs-provider "github" \
    --force-git-metadata \
    /tmp/snapshots

Linear

EME-931

cameroncooke and others added 9 commits April 2, 2026 08:17
Underscores are valid filename characters and common in Swift type names.
Previously they were treated as unsafe and collapsed with adjacent
non-kept characters. Now they are preserved as-is.

Update test to verify unsafe char collapsing separately from underscore
preservation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The _shared static var was read/written without any isolation while
hasDrained used NSLock protection. Both production callers
(discoverPreviews, testPreview) are already @mainactor, so this makes
the contract explicit and prevents future data races.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The init already creates the directory or calls preconditionFailure, so
the directory is guaranteed to exist. The guard also had the side-effect
of skipping the drain while operations could still be in-flight.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace manual semaphore+barrier pattern with the built-in
OperationQueue API that does the same thing. Blocks the calling
thread until all enqueued operations complete.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the color scheme reset into a defer block so the override is
always cleared even if the render loop exits early due to an unexpected
error. Previously the reset only ran after normal loop completion.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The grouping key in discoverPreviews and testPreview used
fileID ?? typeName, skipping the typeDisplayName fallback that
canonicalGroup uses. This could cause filename collisions when a
PreviewProvider has no fileId but a typeDisplayName differing from
typeName.

Extract canonicalGroup to accept raw parameters and make it internal
so both SnapshotTest and the coordinator share the same fallback chain:
fileId -> typeDisplayName -> typeName.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move filename sanitization to the point where baseFileName is created
in testPreview, so SnapshotContext.baseFileName and the sidecar
image_file_name field are always consistent. Previously baseFileName
stored the raw unsanitized name while image_file_name was sanitized,
creating a confusing mismatch for sidecar consumers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the branch-added color scheme override API from SnapshotTest.
Render each preview once using its natural appearance again so snapshot
names and exports no longer fan out by forced light and dark passes.

Co-Authored-By: OpenAI Codex <noreply@openai.com>
Reject snapshot renders above the 40 million pixel upload limit before
writing PNGs to disk.

This prevents extremely tall expanding snapshots from producing files
that the upload CLI will reject.

Co-Authored-By: OpenAI Codex <noreply@openai.com>
@cameroncooke cameroncooke requested a review from noahsmartin April 3, 2026 09:07
@cameroncooke cameroncooke changed the title feat(snapshots): Add CI image export via SNAPSHOTS_EXPORT_DIR env var feat(snapshots): Add CI image export via SNAPSHOTS_EXPORT_DIR env var (EME-931) Apr 7, 2026
#endif
Self.renderingStrategy = strategy
}
let strategy = Self.renderingStrategy!
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we avoid force unwrap? Pretty sure the pattern from before can e used here too in a way that the compiler would be able to tell it should always be non-null

)
let colorSchemeValue = result.colorScheme?.stringValue

let context = SnapshotContext(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we move this into the if statement below? Doesn't seem to be used if CI exports aren't enabled

/// - Returns: An array of `DiscoveredPreview` objects representing the found previews.
@MainActor
override class func discoverPreviews() -> [DiscoveredPreview] {
_ = SnapshotCIExportCoordinator.sharedIfEnabled()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems like a code smell, why assign to _, is there a side effect of calling sharedIfEnabled that needs to happen here?

Can we make SnapshotCIExportCoordinator not a singleton and instead just have it be a static property of the SnapshotTest class?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes we can do that and I agree.

- Remove force unwrap of renderingStrategy by restoring if-let pattern
- Scope SnapshotContext creation to CI export path only
- Replace singleton with static factory + SnapshotTest-owned property

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

@noahsmartin noahsmartin left a comment

Choose a reason for hiding this comment

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

New changes LGTM thanks!

@cameroncooke cameroncooke merged commit 9cdc81a into main Apr 8, 2026
7 checks passed
@cameroncooke cameroncooke deleted the cameroncooke/snapshot-ci branch April 8, 2026 15:43
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