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

Native visual regression testing (take two) #6952

Closed
wants to merge 15 commits into from

Conversation

drewvolz
Copy link
Member

@drewvolz drewvolz commented Feb 18, 2023

To see the feature in action, checkout the target PR to view the functionality.

This PR performs an e2e visual regression check by verifying the app is able to successfully set a background image for the app. When the default background image is changed from the initial value to a photo from the simulator's camera roll, swift-snapshot-testing quickly performs a pixel check to verify against an image of the desired outcome.

The e2e test introduced in this PR performs an end to end check for:

  • Asserting that the context menu works.
  • Ensuring the settings screen opens.
  • Guaranteeing an image can be chosen.
  • Asserting the new background of the app matches the expected outcome pixel for pixel.

The output file we are comparing against lives at the following path (customizable, currently stored with the tests).

ios/AllAboutOlafUITests/__Snapshots__/CustomBackgroundImageTests/*.heic

Things to note

Swift Package Manager additions

Why SwiftPM? Neither of these packages support Cocoapods.

I've made sure to add these two SwiftPM additions to the AllAboutOlafUITests target and not AllAboutOlaf.

  1. Instead of rolling our own regression checking (WIP native visual regression testing #6920), we can use swift-snapshot-testing from pointfree. It's very customizable, well supported, and gives us a lot of control over formats we can diff views in.

  2. We are storing the output file as an HEIC instead of a PNG (with SnapshotTestingHEIC) because the HEIC storage format reduces file sizes in comparison to PNG.

Ultimately we will want to store and perform regression checking in a separate repository, but this at least lets us begin performing these types of checks in the codebase today.

Preventing false positives

To avoid issues of

this test runs on the component library version of the background image picker. It changes both the library sample and real background of the app, but if something were to change the e2e flow of the real component, perhaps we wouldn't know immediately.

I feel the compromise of knowing that (i) the data has changed, (ii) the hooks are functioning, and (iii) the image is visually checked may be a good set of tradeoffs in lieu of testing the actual homescreen but that is open for discussion!

Jest vs XCUITest

As described in #6916 (comment), challenges were presented that required us to go native for this test; Apple renders the native image picker view out of the view hierarchy process, which means we do not have access to inspect PHPickerViewController with detox. A native test here-and-there won't hurt, XCUITest doesn't bite!

Reusability

To get this in state where it could be tested and reusable across the component library and settings, a few changes were made across the app.

  • testID is now an available prop for <PushButtonCell /> components
  • The navigation bar context menu is separated into its own <RightHomeContextMenu /> component

@codecov
Copy link

codecov bot commented Feb 18, 2023

Codecov Report

Merging #6952 (2951411) into drew/background (656dff9) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

Additional details and impacted files
@@                Coverage Diff                 @@
##           drew/background   #6952      +/-   ##
==================================================
- Coverage             8.43%   8.40%   -0.04%     
==================================================
  Files                  304     306       +2     
  Lines                 5158    5178      +20     
  Branches              1389    1392       +3     
==================================================
  Hits                   435     435              
- Misses                4701    4721      +20     
  Partials                22      22              

@drewvolz
Copy link
Member Author

@hawkrives ideas on how to resolve package.resolved for Swift Package Manager in our build and caching steps?

Resolve Package Graph
xcodebuild: error: Could not resolve package dependencies:
  Package.resolved file is corrupted or malformed; fix or delete the file to continue: unsupported schema version 2

@hawkrives
Copy link
Member

hawkrives commented Feb 18, 2023

Does

xcodebuild -workspace ios/AllAboutOlaf.xcworkspace -scheme AllAboutOlaf -configuration Release -destination 'platform=iOS Simulator,name=iPhone 11 Pro,OS=15.0' -derivedDataPath ios/build build

pass if you run it locally?

Edit: because we may need to update the detox command string.

@drewvolz
Copy link
Member Author

drewvolz commented Feb 18, 2023

I know the pipeline is setup differently but xcodebuild worked fine locally when I built with iOS 16.2 and an iPhone 14 Pro.

That sounds like the problem if v2 package.resolved is invalid; this needs to be created with Xcode 13.

Update: that looked like it did the job to fix the previous error.

hoping to avoid downloading xcode 13
@drewvolz
Copy link
Member Author

I think you're still correct. We'll need to add a step for xcodebuild ... test to see the native e2e test on the pipeline.

@drewvolz
Copy link
Member Author

Evaluating this again.

Choosing the detox approach may be better in the end. We can assume the photo picker is out of our control, so if we assume it behaves correctly, we can go the mocked approach of detox and avoid adding native dependencies on SwiftPM and its packages.

Will try again with the approach laid out in #6916 (comment)

@drewvolz drewvolz closed this Feb 18, 2023
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