Disable record on regular snapshot tests to prevent from passing after retry#6303
Disable record on regular snapshot tests to prevent from passing after retry#6303
Conversation
vegaro
changed the title
vegaro
changed the title
|
@RCGitBot please test |
1 similar comment
|
@RCGitBot please test |
fastlane/Fastfile
Outdated
| **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" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
yes, that way we can add more tests there easily
There was a problem hiding this comment.
Ohh right right. I see what you mean. Makes total sense 👌
|
@RCGitBot please test |
vegaro
force-pushed
the
cesar/remove-rcui-snapshot-retries
branch
from
8588f44 to
fd3e371
Compare
vegaro
force-pushed
the
cesar/remove-rcui-snapshot-retries
branch
from
fd3e371 to
b5f06ae
Compare
vegaro
changed the title
|
@RCGitBot please test |
tonidero
left a comment
tonidero
left a comment
There was a problem hiding this comment.
I believe this makes sense!
| if ProcessInfo.processInfo.environment["CIRCLECI_TESTS_GENERATE_REVENUECAT_UI_SNAPSHOTS"] == "1" { | ||
| isRecording = true | ||
| } | ||
| return value == "1" || value == "true" |
There was a problem hiding this comment.
Do we need both? Not sure what value it's actually passed I guess 😅
There was a problem hiding this comment.
we are setting to 1 I believe as seen in the previous code, but let me double check
Package.swift
Outdated
| @@ -50,7 +50,7 @@ var dependencies: [Package.Dependency] = [ | |||
| // SST requires iOS 13 starting from version 1.13.0 | |||
There was a problem hiding this comment.
Maybe we can remove this comment?
| .package( | ||
| url: "https://github.com/pointfreeco/swift-snapshot-testing", | ||
| revision: "26ed3a2b4a2df47917ca9b790a57f91285b923fb" | ||
| exact: "1.18.9" |
There was a problem hiding this comment.
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)
|
good catch @ajpallares , will try |
|
@RCGitBot please test |
|
@RCGitBot please test |
📸 Snapshot Test242 unchanged
🛸 Powered by Emerge Tools |
|
@RCGitBot please test |
3 participants
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
__recorddefaults to.missing(unlessSNAPSHOT_TESTING_RECORDis set)isRecordingis deprecated and maps to.all/.missing. This is what we were doing, so it was always recordingrecord == .neveris what we wantUpdates
Snapshot mode now uses
CIRCLECI_TESTS_GENERATE_REVENUECAT_UI_SNAPSHOTSandCIRCLECI_TESTS_GENERATE_SNAPSHOTSlike this: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.