getsentry / action-visual-snapshot

Save and compare your visual snapshots
MIT License
38 stars 7 forks source link

ref(perf): add early termination option #62

Closed JonasBa closed 2 years ago

JonasBa commented 2 years ago

Some test runs have a large amount of flakes (idk if we have stats on this, but I had a run with ~160 changed snapshots today). When this happens, it is unlikely that all of the snapshots will be useful and actionable to us. This PR introduces the mechanism to early terminate such runs and only keep the first N changed snapshots. I added a note to the gallery based on the terminationReason and made sure that snapshots that are never processed are not marked as missing by adding that second "cleanup" loop.

Currently, the PR sets 30 as the "magic number" for max changed snapshots, I'm open to changing that if we have some data to back this up.

JonasBa commented 2 years ago

I'll make sure to trigger a change from sentry run before merging this

JonasBa commented 2 years ago

@billyvg this is now ready to review. The runs from sentry look good and properly stop the run at 30 changed snapshots

billyvg commented 2 years ago

@JonasBa can you link the test PR please?

JonasBa commented 2 years ago

@billyvg sure, I made a bunch of runs here https://github.com/getsentry/sentry/pull/35209

JonasBa commented 2 years ago

@billyvg I re-ran after merging the changes to the server and this is the output. It's a little big, but since this is a special case , I'd rather have it be super obvious

CleanShot 2022-06-07 at 09 15 10@2x