Skip to content

Disable record on regular snapshot tests to prevent from passing after retry#6303

Merged
vegaro merged 4 commits intomainfrom
cesar/remove-rcui-snapshot-retries
Feb 23, 2026
Merged

Disable record on regular snapshot tests to prevent from passing after retry#6303
vegaro merged 4 commits intomainfrom
cesar/remove-rcui-snapshot-retries

Conversation

@vegaro
Copy link
Copy Markdown
Member

@vegaro vegaro commented Feb 19, 2026

Summary

Fixes RCUI snapshot recording mode so normal CI runs do not auto-record missing JSON snapshots.

SnapshotTesting README documents automatic recording when reference is missing. See the section that says first run auto-records snapshots and also “More hands-off”.

"More hands-off. New snapshots are recorded whether isRecording mode is true or not."

In the source code https://github.com/pointfreeco/swift-snapshot-testing/blob/main/Sources/SnapshotTesting/AssertSnapshot.swift

  • __record defaults to .missing (unless SNAPSHOT_TESTING_RECORD is set)
  • isRecording is deprecated and maps to .all/.missing. This is what we were doing, so it was always recording
  • record == .never is what we want

Updates

Snapshot mode now uses CIRCLECI_TESTS_GENERATE_REVENUECAT_UI_SNAPSHOTS and CIRCLECI_TESTS_GENERATE_SNAPSHOTS like this:

  • true/1 => record mode all
  • otherwise => record mode never

This keeps retries from masking snapshot failures by writing new baselines during regular test runs. Tests were succeeding even if snapshots didn't match because first run would record, second run would succeed.

I needed to update our snapshot testing to fix this.

@vegaro vegaro requested a review from a team as a code owner February 19, 2026 17:17
@vegaro vegaro marked this pull request as draft February 19, 2026 17:17
@vegaro vegaro changed the title Remove RevenueCatUI test retries in CI test plan [TESTING] Remove RevenueCatUI test retries in CI test plan Feb 19, 2026
@vegaro vegaro changed the title [TESTING] Remove RevenueCatUI test retries in CI test plan Fail snapshot validation tests fast while keeping RevenueCatUI retries Feb 19, 2026
@vegaro
Copy link
Copy Markdown
Member Author

vegaro commented Feb 19, 2026

@RCGitBot please test

1 similar comment
@vegaro
Copy link
Copy Markdown
Member Author

vegaro commented Feb 19, 2026

@RCGitBot please test

**xcodebuild_arguments,
result_bundle_path: 'fastlane/test_output/revenuecatui-paywall-validation.xcresult',
report_path: 'fastlane/test_output/revenuecatui/paywall_validation_tests.xml',
xcargs: "-testPlan CI-RevenueCatUI-NoRetries -only-testing:RevenueCatUITests/PaywallDataValidationTests"
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.

Do we need to specify -only-testing:RevenueCatUITests/PaywallDataValidationTests? I mean, couldn't we achieve this by having PaywallDataValidationTests be the only tests in the CI-RevenueCatUI-NoRetries test plan?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes, that way we can add more tests there easily

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.

Ohh right right. I see what you mean. Makes total sense 👌

@vegaro
Copy link
Copy Markdown
Member Author

vegaro commented Feb 20, 2026

@RCGitBot please test

2 similar comments
@vegaro
Copy link
Copy Markdown
Member Author

vegaro commented Feb 20, 2026

@RCGitBot please test

@vegaro
Copy link
Copy Markdown
Member Author

vegaro commented Feb 20, 2026

@RCGitBot please test

@vegaro vegaro force-pushed the cesar/remove-rcui-snapshot-retries branch from 8588f44 to fd3e371 Compare February 20, 2026 10:00
@vegaro vegaro force-pushed the cesar/remove-rcui-snapshot-retries branch from fd3e371 to b5f06ae Compare February 20, 2026 10:09
@vegaro vegaro changed the title Fail snapshot validation tests fast while keeping RevenueCatUI retries Disable record on regular snapshot tests to prevent from passing after retry Feb 20, 2026
@vegaro
Copy link
Copy Markdown
Member Author

vegaro commented Feb 20, 2026

@RCGitBot please test

@vegaro vegaro marked this pull request as ready for review February 20, 2026 11:05
@vegaro vegaro requested a review from a team as a code owner February 20, 2026 11:05
@vegaro vegaro requested a review from ajpallares February 20, 2026 11:54
Copy link
Copy Markdown
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

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

I believe this makes sense!

Copy link
Copy Markdown
Member

@ajpallares ajpallares left a comment

Choose a reason for hiding this comment

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

Great improvement! 🤩

Package.swift Outdated
@@ -50,7 +50,7 @@ var dependencies: [Package.Dependency] = [
// SST requires iOS 13 starting from version 1.13.0
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.

Maybe we can remove this comment?

.package(
url: "https://github.com/pointfreeco/swift-snapshot-testing",
revision: "26ed3a2b4a2df47917ca9b790a57f91285b923fb"
exact: "1.18.9"
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.

Nice 🤩

Copy link
Copy Markdown
Member

@ajpallares ajpallares left a comment

Choose a reason for hiding this comment

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

I wonder if it would be possible to use v1.17.7 of the SnapshotTesting package only for Package@swift-5.8.swift, which also supports Swift 5.7 (so it would work with Xcode 15) and also contains the withSnapshotTesting APIs (introduced in v1.17.0)

@vegaro
Copy link
Copy Markdown
Member Author

vegaro commented Feb 20, 2026

good catch @ajpallares , will try

@vegaro
Copy link
Copy Markdown
Member Author

vegaro commented Feb 20, 2026

@RCGitBot please test

@vegaro
Copy link
Copy Markdown
Member Author

vegaro commented Feb 20, 2026

@RCGitBot please test

@emerge-tools
Copy link
Copy Markdown

emerge-tools bot commented Feb 20, 2026

📸 Snapshot Test

242 unchanged

Name Added Removed Modified Renamed Unchanged Errored Approval
RevenueCat
com.revenuecat.PaywallsTester
0 0 0 0 242 0 N/A

🛸 Powered by Emerge Tools

@vegaro
Copy link
Copy Markdown
Member Author

vegaro commented Feb 20, 2026

@RCGitBot please test

@vegaro
Copy link
Copy Markdown
Member Author

vegaro commented Feb 23, 2026

@ajpallares loooks like it works 😄

Copy link
Copy Markdown
Member

@ajpallares ajpallares left a comment

Choose a reason for hiding this comment

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

Let's go! 🚀

@vegaro vegaro merged commit ae65e3f into main Feb 23, 2026
42 checks passed
@vegaro vegaro deleted the cesar/remove-rcui-snapshot-retries branch February 23, 2026 09:39
Comment on lines +14 to +15
// Uses SST 1.17.7, the last version with swift-tools-version 5.7, which includes the
// `withSnapshotTesting` API needed to prevent auto-recording in CI.
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.

🫶

polmiro pushed a commit that referenced this pull request Feb 23, 2026
…r retry (#6303)

* update snapshot testing

* use version 1.17.7
polmiro pushed a commit that referenced this pull request Feb 23, 2026
…r retry (#6303)

* update snapshot testing

* use version 1.17.7
ajpallares added a commit that referenced this pull request Feb 23, 2026
PR #6303 updated Package.swift to use exact: "1.18.9" but the
Tuist/Package.swift was left on the old revision pin.

Co-authored-by: Cursor <cursoragent@cursor.com>
ajpallares added a commit that referenced this pull request Feb 23, 2026
…#6335)

PR #6303 updated Package.swift to use exact: "1.18.9" but the
Tuist/Package.swift was left on the old revision pin.

Co-authored-by: Cursor <cursoragent@cursor.com>
ajpallares added a commit that referenced this pull request Feb 25, 2026
* CI: Unify CI jobs to reduce machine count

Merge several pairs of CI jobs that run on the same Xcode version into
single jobs that execute tests sequentially, reducing the total number
of macOS machines needed per CI run.

Unified jobs:
- run-test-ios-14-and-15 (Xcode 14.3.1, includes background iOS 14 sim install)
- run-test-ios-16-and-17 (Xcode 15.4.0)
- run-test-ios-18-and-26 (Xcode 26.3)
- run-test-tvos-and-macos (Xcode 16.4)
- run-revenuecat-ui-ios-17-and-18 (Xcode 16.4)

Each unified job also runs an SPM release build as a preceding step,
absorbing the standalone spm-release-build jobs.

Other changes:
- compress_result_bundle guards against missing .xcresult directories
- push_snapshot_pr in Fastfile saves/restores the original branch

Co-authored-by: Cursor <cursoragent@cursor.com>

* Update Tuist swift-snapshot-testing dependency to match Package.swift

PR #6303 updated Package.swift to use exact: "1.18.9" but the
Tuist/Package.swift was left on the old revision pin.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Restructure iOS test jobs due to Xcode 26 + iOS 18.6 SK1 bug (FB22032719)

iOS 18 SK1 tests fail under Xcode 26 due to an Apple bug where
SKProductsRequest cannot parse the price field (missingValue for
StoreKit.ProductResponse.Key.price). Restructure jobs so iOS 18
runs under Xcode 16 instead:

- run-test-ios-26: standalone on Xcode 26 (iOS 26 only)
- run-test-ios-17-and-18: Xcode 16.4 (iOS 17 + 18)
- run-test-ios-14-15-and-16: Xcode 14.3.1 (iOS 14 + 15 + 16)

Co-authored-by: Cursor <cursoragent@cursor.com>

* Reorder test runs from newest to oldest iOS version

Rename and reorder unified jobs so the highest iOS version runs first:
- run-test-ios-18-and-17 (was 17-and-18)
- run-test-ios-16-15-and-14 (was 14-15-and-16)
- run-revenuecat-ui-ios-18-and-17 (was 17-and-18)

iOS 14 still runs last since its runtime needs to be downloaded first.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Address review: remove temp workflow, make snapshot PR branch restore exception-safe

- Remove test-all-unified workflow and action parameter (was temporary)
- Wrap push_snapshot_pr git operations in begin/ensure to guarantee
  the original branch is restored even if push or PR creation fails

Co-authored-by: Cursor <cursoragent@cursor.com>

* Add SPM release build for Xcode 15 to spm-revenuecat-ui-ios-16 job

The standalone spm-release-build-xcode-15 was removed during
unification but no other Xcode 15 job picked up the RevenueCat
release build. Add it to the only remaining Xcode 15 job.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Split run-test-ios-16 into standalone job and replace validate-package-swift with swift package describe

- Extract iOS 16 tests from run-test-ios-16-15-and-14 into a standalone
  run-test-ios-16 job on Xcode 15.4.0 with SPM release build
- Rename remaining job to run-test-ios-15-and-14 (Xcode 14.3.1)
- Add `swift package describe` as first step in all run-test jobs,
  making the validate-package-swift job redundant
- Remove validate-package-swift job and all workflow references
- Remove SPM RevenueCat build from spm-revenuecat-ui-ios-16 (now
  covered by run-test-ios-16)

Co-authored-by: Cursor <cursoragent@cursor.com>

* Add missing swift package describe to run-test-tvos-and-macos

Co-authored-by: Cursor <cursoragent@cursor.com>

* Limit swift package describe to run-test-ios jobs only

The iOS test jobs already cover all 4 Xcode versions (14, 15, 16, 26),
so tvOS, macOS and watchOS jobs don't need to duplicate the validation.

Co-authored-by: Cursor <cursoragent@cursor.com>

* [TEMP] Corrupt testNestedDictionary snapshots to test generate_snapshots workflow

Co-authored-by: Cursor <cursoragent@cursor.com>

* Fix snapshot PR git add glob to match deeply nested __Snapshots__ dirs

The old pattern `../*/__Snapshots__/*` only matched one directory level
on each side of __Snapshots__, but snapshot files are 3+ levels deep
(e.g. Tests/UnitTests/Misc/__Snapshots__/AnyEncodableTests/file.json).
Use `**` for recursive matching in the git pathspec.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Revert "Fix snapshot PR git add glob to match deeply nested __Snapshots__ dirs"

This reverts commit 7cf475f.

* [TEMP] Remove X-Retry-Count from some testRepeatedRequestsLogDebugMessage snapshots

Co-authored-by: Cursor <cursoragent@cursor.com>

* Remove run step for `swift build -c release --target RevenueCat` from `run-test-tvos-and-macos` already present in `run-test-ios-18-and-17` for Xcode 16

* Add snapshot generation support for tvOS tests

- Update test_tvos lane to use CI-RevenueCat-Snapshots test plan when
  generate_snapshots is true (matching test_watchos and test_macos)
- Disable retries and fail_build in snapshot mode
- Add create-snapshot-pr-if-needed step after tvOS tests in CI job

Co-authored-by: Cursor <cursoragent@cursor.com>

* Revert "[TEMP] Remove X-Retry-Count from some testRepeatedRequestsLogDebugMessage snapshots"

This reverts commit 3fef904.

* Revert "[TEMP] Corrupt testNestedDictionary snapshots to test generate_snapshots workflow"

This reverts commit 7a695c0.

* TEMP: corrupt some macOS snapshots to test the snapshot generation

* TEMP: only run `run-test-tvos-and-macos` for snapshots generation

* Add tvOS-specific snapshot file prefix

tvOS was falling through to the #else branch in osVersionAndTestName,
generating snapshots with the iOS prefix (e.g. iOS18-testFoo.1.json).
This meant tvOS snapshots were indistinguishable from iOS snapshots.

Add a #elseif os(tvOS) branch to produce tvOS-prefixed snapshot files
(e.g. tvOS18-testFoo.1.json), enabling independent tvOS snapshot
generation and validation.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Revert "TEMP: only run `run-test-tvos-and-macos` for snapshots generation"

This reverts commit c06e438.

* Revert "TEMP: corrupt some macOS snapshots to test the snapshot generation"

This reverts commit c486f7f.

* [skip ci] Generating new test snapshots (#6351)

* Add 30-minute timeout to iOS 14 runtime wait loop

The existing no_output_timeout doesn't apply because the loop prints
periodic "Still waiting..." messages. Add an explicit elapsed-time
check so the step fails cleanly instead of waiting indefinitely.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Remove redundant no_output_timeout from iOS 14 runtime wait step

The loop prints output every 10 seconds, so no_output_timeout never
triggers. The explicit 30-minute elapsed-time check is sufficient.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: RevenueCat Git Bot <72824662+RCGitBot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants