Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RevenueCatUI snapshot tests #3526

Merged
merged 10 commits into from
Dec 20, 2023
Merged

Fix RevenueCatUI snapshot tests #3526

merged 10 commits into from
Dec 20, 2023

Conversation

NachoSoto
Copy link
Contributor

@NachoSoto NachoSoto commented Dec 19, 2023

Turns out that because neither pointfreeco/swift-snapshot-testing#702 or pointfreeco/swift-snapshot-testing#666 had been merged, we had been getting false positives on snapshot tests on CI since the very beginning.

The perceptualPrecision implementation requires Metal support, and without it swift-snapshot-testing was silently passing.

This fixes that, and prevents it from happening again.

Changes:

  • Changed CircleCI to use M1 instances, which do support Metal
  • Added assertion to catch this from happening again
  • Replaced ProgressView with a static version in snapshots to prevent failures from the spinning indicator
  • Re-generated snapshots from M1 instances

@NachoSoto NachoSoto force-pushed the paywalls-snapshot-testing branch 2 times, most recently from dfda8c8 to 2657ef2 Compare December 19, 2023 20:34
@NachoSoto NachoSoto added test Adding missing tests or correcting existing tests and removed do not merge labels Dec 19, 2023
@NachoSoto NachoSoto force-pushed the paywalls-snapshot-testing branch 2 times, most recently from c491b54 to 42795db Compare December 20, 2023 18:59
@NachoSoto NachoSoto changed the title [DO NOT MERGE] Testing Paywall snapshot tests [WIP] Fix RevenueCatUI snapshot tests Dec 20, 2023
@NachoSoto NachoSoto requested a review from a team December 20, 2023 19:20
- spm-revenuecat-ui-ios-15:
xcode_version: '14.3.0'
- spm-revenuecat-ui-ios-16:
xcode_version: '14.3.0'
- spm-revenuecat-ui-ios-17:
xcode_version: '15.1'
- spm-revenuecat-ui-watchos:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: remove commit

@NachoSoto NachoSoto marked this pull request as ready for review December 20, 2023 19:48
@NachoSoto NachoSoto changed the title [WIP] Fix RevenueCatUI snapshot tests Fix RevenueCatUI snapshot tests Dec 20, 2023
expect(MTLCreateSystemDefaultDevice()).toNot(
beNil(),
description: "Metal is required for perceptuallyCompare, but not available on this machine."
)
Copy link
Member

Choose a reason for hiding this comment

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

😍

@@ -155,8 +155,8 @@ private let traits: UITraitCollection = .init(displayScale: 1)

#endif

private let perceptualPrecision: Float = 0.97
private let timeout: DispatchTimeInterval = .seconds(5)
private let perceptualPrecision: Float = 0.94
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity, what failed if we keep it at 97?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea, the diff produced showing the inconsistency was completely black to the naked eye. So just very tiny color differences.

@NachoSoto NachoSoto enabled auto-merge (squash) December 20, 2023 23:22
@NachoSoto NachoSoto merged commit d672dae into main Dec 20, 2023
19 of 20 checks passed
@NachoSoto NachoSoto deleted the paywalls-snapshot-testing branch December 20, 2023 23:26
This was referenced Dec 21, 2023
NachoSoto pushed a commit that referenced this pull request Dec 22, 2023
**This is an automatic release.**

### RevenueCatUI
* `Paywalls`: add header image to `watchOS` paywalls (#3542) via
NachoSoto (@NachoSoto)
* `Paywalls`: improve template 5 landscape layout (#3534) via NachoSoto
(@NachoSoto)
* `Paywalls`: fix template 5 footer loading view alignment (#3537) via
NachoSoto (@NachoSoto)
* `Paywalls`: improve template 1 landscape layout (#3532) via NachoSoto
(@NachoSoto)
* `Paywalls`: fix `ColorInformation.multiScheme` on `watchOS` (#3530)
via NachoSoto (@NachoSoto)
### Other Changes
* `Trusted Entitlements`: tests for signature verification without
header hash (#3505) via NachoSoto (@NachoSoto)
* `.debugRevenueCatOverlay`: added `Locale` (#3539) via NachoSoto
(@NachoSoto)
* `Trusted Entitlements`: add support for signing request headers
(#3424) via NachoSoto (@NachoSoto)
* `CI`: Add architecture to cache keys (#3538) via Mark Villacampa
(@MarkVillacampa)
* `Paywalls Tester`: remove double close button (#3531) via NachoSoto
(@NachoSoto)
* Fix `RevenueCatUI` snapshot tests (#3526) via NachoSoto (@NachoSoto)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Adding missing tests or correcting existing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants