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

[MOB-3026] - Snapshot Testing Improvements #815

Merged
merged 20 commits into from
Nov 19, 2024

Conversation

d4r1091
Copy link
Member

@d4r1091 d4r1091 commented Nov 13, 2024

MOB-3026

Context

We want to introduce a series of improvements to reduce the CI time and improve test reliability.

Approach

  • Overall improvement to the script
  • Aggregates intuitively the device against which run the maximum tests possible
  • In case of the need for a specific device, the runsOn overrides the default device and executes a dedicated build (useful for iPad-specific tests for instance that will produce a dedicated UI)
  • Better validation and safe checks for the script
  • introduced the possibility to intuitively select "all portrait" devices by simply passing ["all" "portrait"] as part of the devices list for a certain test class
  • Updated iPhone devices removing the 14 (based on the 13) and adding the iPhone 15 from an opened PR in the Snapshot Testing library.

Have a look at the decrease in timing https://github.com/ecosia/ios-browser/actions/runs/11826256998

Compared to an old successful snapshot run https://github.com/ecosia/ios-browser/actions/runs/11775291296

Other

Next Steps? Have a dedicated branch for storing Snapshots. Ticket incoming 🎟️ .

The change is too big because of the newer snapshots. To review the files feel free to check the newer script and the newer configuration.

🤖 Same reason for the PR Agent failing. 🤖

Before merging

Checklist

  • I performed some relevant testing on a real device and/or simulator

@d4r1091 d4r1091 requested a review from a team November 13, 2024 21:49
@d4r1091 d4r1091 force-pushed the dc-mob-3026-snapshot-testing-improvements branch from 3f0e85d to e9f4741 Compare November 13, 2024 22:03
@d4r1091 d4r1091 marked this pull request as ready for review November 13, 2024 22:18
@lucaschifino
Copy link
Collaborator

Have a look at the decrease in timing https://github.com/ecosia/ios-browser/actions/runs/11826256998

Compared to an old successful snapshot run https://github.com/ecosia/ios-browser/actions/runs/11775291296

Looks like the first one linked did not run the snapshot tests at all, apparently because there was no version update, probably you had already re-introduced the correct versioning logic then?

I found this older one (https://github.com/ecosia/ios-browser/actions/runs/11825657659) which seems like what you want, still a huge improvement from 1h to 18min 🎉 👏

Copy link
Collaborator

@lucaschifino lucaschifino left a comment

Choose a reason for hiding this comment

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

I've looked into the new script and the new configuration files and they look good 🙌

The performance improvement is huge 👏 🎉

Can't look into much else due to the change size, but from my side all good to go ✅

@d4r1091 d4r1091 merged commit 63ee9e0 into main Nov 19, 2024
4 of 5 checks passed
@d4r1091 d4r1091 deleted the dc-mob-3026-snapshot-testing-improvements branch November 19, 2024 15:32
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