percy / percy-ember

Ember addon for visual regression testing with Percy
https://docs.percy.io/docs/ember
MIT License
73 stars 44 forks source link

Version 1.0.0-beta.55 seems to have problems with identifying snapshots that are not new #335

Closed eramod closed 3 years ago

eramod commented 3 years ago

After upgrading from 1.0.0-beta.52 to 1.0.0-beta.55, Percy tests suddenly identify most of our snapshots as new on every run. Reverting back to 1.0.0-beta.52 seems to fix the issue.

Robdel12 commented 3 years ago

Hey @eramod! Can you share build details? This happens if the names don't match or if there are new/different widths from the baseline that's been set.

eramod commented 3 years ago

Hey @eramod! Can you share build details? This happens if the names don't match or if there are new/different widths from the baseline that's been set.

Hi there. I'm not sure what build details you would need. Could you be more specific? The only change we made was upgrading from 1.0.0-beta.52 to 1.0.0-beta.55. We did not add any new snapshots or change any Percy width settings. And when I revert to the older version, Percy seems to function just fine and recognize snapshots we've had in the app for a long time, which is why I thought it might be a new bug introduced in the latest version.

Robdel12 commented 3 years ago

It does sound like a bug but we'll need more info to debug. If you could share the build URL or ID where the snapshots came up as new that would be helpful.

Robdel12 commented 3 years ago

Alright, so I did some debugging (saw you're working at Tilde, so I went and looked at some builds). This isn't an SDK bug -- but it is a bit confusing as to what's happening.

This is a confusion caused by the baseline picking logic. Build #9880 has 112 new snapshots because its using Build #9842 as its baseline (which is missing 113 snapshots from it). Percy's baseline picking logic uses git to pick its baselines. That build was a baseline because it had a common commit between the baseline branch (master) and when that branch was created/branched off of the master branch. Rebasing the dependabot/npm_and_yarn/types/d3-drag-3.0.0 branch (which build #9880 is using) onto the latest master will then pick the latest baseline for the project the next time tests are run on that branch.

More info about the baseline picking logic: https://docs.percy.io/docs/baseline-picking-logic

You're safe to move back to beta.55 :)

Edit: If I can try to sum it up nicely, this happened because a baseline was used that had missing snapshots. When a new build ran and used that broken build as a baseline, the "missing" snapshots will show up as new.

eramod commented 3 years ago

Alright, so I did some debugging (saw you're working at Tilde, so I went and looked at some builds). This isn't an SDK bug -- but it is a bit confusing as to what's happening.

This is a confusion caused by the baseline picking logic. Build #9880 has 112 new snapshots because its using Build #9842 as its baseline (which is missing 113 snapshots from it). Percy's baseline picking logic uses git to pick its baselines. That build was a baseline because it had a common commit between the baseline branch (master) and when that branch was created/branched off of the master branch. Rebasing the dependabot/npm_and_yarn/types/d3-drag-3.0.0 branch (which build #9880 is using) onto the latest master will then pick the latest baseline for the project the next time tests are run on that branch.

More info about the baseline picking logic: https://docs.percy.io/docs/baseline-picking-logic

You're safe to move back to beta.55 :)

Edit: If I can try to sum it up nicely, this happened because a baseline was used that had missing snapshots. When a new build ran and used that broken build as a baseline, the "missing" snapshots will show up as new.

Thanks for the explanation and sorry for the radio silence. I was on vacation last week.

Robdel12 commented 3 years ago

No worries! I also just finished up a vacation 🎉