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

Surface browser version change information #94

Merged
merged 2 commits into from
Jul 26, 2023
Merged

Conversation

dlockhart
Copy link
Member

This adds a tracker .vdiff.json file to the root of the repo-under-test that is intended to get checked into source control. In there we can put whatever we like, but for now it contains the browsers and versions of those browsers from the previous test run.

We can then use that information to surface any changes in the versions in the report -- both in the side panel but also when viewing a particular browser's results:

Screenshot 2023-07-24 at 3 13 26 PM

@dlockhart dlockhart requested a review from a team as a code owner July 24, 2023 19:21
Base automatically changed from dlockhart/iterators to main July 25, 2023 13:32
Copy link
Member Author

Choose a reason for hiding this comment

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

This is what the file we'll be checking into each repo would look like. Open to other suggestions around naming, etc.

@dlockhart dlockhart merged commit 3e79e64 into main Jul 26, 2023
1 check passed
@dlockhart dlockhart deleted the dlockhart/browser-info branch July 26, 2023 13:40
@ghost
Copy link

ghost commented Jul 26, 2023

🎉 This PR is included in version 0.21.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ghost ghost added the released label Jul 26, 2023
@svanherk
Copy link
Contributor

Sorry, somehow totally missed this PR. So when do we commit the updates to this file? Only when tests have failed?

@dlockhart
Copy link
Member Author

So when do we commit the updates to this file?

I was hoping it would get committed as part of the visual-diff action just like a change to a golden?

@dlockhart
Copy link
Member Author

But yeah... that make me think we don't want to update it unless the --golden flag is passed eh.

@svanherk
Copy link
Contributor

svanherk commented Jul 26, 2023

Locally, I guess we never want to update it because we don't want people accidentally committing it?

Then in CI, if the tests passed the action isn't currently set up to commit anything (even orphaned goldens) because those would just be handled next time there's an actual failure or update needed. But in this case, it's a bit more awkward - you could run into a scenario where the goldens have passed for a few weeks and the next failure jumps up a few browser versions. In practice, maybe this would never ever happen - a new browser version always causes a failure.

And technically, the data is still accurate - the previous goldens were generated at that browser version from weeks ago.

So maybe what we want is:

  • If running locally, have a copy of the config in .vdiff that's created with --golden and updated during runs, but not committed (handled by the .gitignore of .vdiff)
  • If running in CI, always create/update/use the .vdiff.json copy, and if any goldens are updated it will be committed alongside it

?

@dlockhart
Copy link
Member Author

I think that makes sense! I'll put up a PR to at least handle the local scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants