Skip to content

Fix RevenueCatUI snapshot tests#3526

Merged
NachoSoto merged 10 commits into
mainfrom
paywalls-snapshot-testing
Dec 20, 2023
Merged

Fix RevenueCatUI snapshot tests#3526
NachoSoto merged 10 commits into
mainfrom
paywalls-snapshot-testing

Conversation

@NachoSoto

@NachoSoto NachoSoto commented Dec 19, 2023

Copy link
Copy Markdown
Contributor

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 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
@NachoSoto NachoSoto force-pushed the paywalls-snapshot-testing branch from 27d6cf6 to 9f42a5c Compare December 20, 2023 19:40
Comment thread .circleci/config.yml
xcode_version: '14.3.0'
- spm-revenuecat-ui-ios-17:
xcode_version: '15.1'
- spm-revenuecat-ui-watchos:

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.

TODO: remove commit

@NachoSoto NachoSoto marked this pull request as ready for review December 20, 2023 19:48
@NachoSoto NachoSoto force-pushed the paywalls-snapshot-testing branch from 9f42a5c to 3826e04 Compare 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
Copy Markdown
Member

Choose a reason for hiding this comment

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

😍


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

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.

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

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.

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
@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants