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

add visual regression testing #132

Merged
merged 6 commits into from
Feb 3, 2023
Merged

Conversation

chris48s
Copy link
Member

@chris48s chris48s commented Nov 27, 2022

Following up on a suggestion from #61 (comment) this PR adds some black box visual regression tests that compare the images generated by squint against reference images using pixelmatch.

Example output from passing tests:

pass

Example output with failing test:

fail

and also the image diffs for any cases that failed will be saved to a build artifact. Example image diff:

diff

I've pushed a commit that intentionally causes a failure and then reverted it so you can poke about in examples of both.

Some other thoughts:

  • This build is quite slow to run (~10 mins), nearly all of which is the docker build step. It would be nice to see if we could improve this with a bit of caching. If we could cache the layers up to RUN apt update && apt install -y libgtk-3-dev between builds that would give us some improvement, but anything beyond that is going to change every PR anyway so it is never going to be rapid. I haven't done anything about this yet.
  • To add more tests, we can add an object to test_cases.json and a corresponding image in /reference_images. Add static page that displays badges #35 probably indicates a good selection of cases we could aim to cover, but I've decided to start small. Once the basics are in place, adding more cases is comparatively easy.
  • I generated the reference images from https://raster.shields.io/ . Interestingly, if I run these tests locally some of the images are actually different due to minor kerning differences. I'm not really sure why this is, but the images from https://raster.shields.io/ are pixel-identical to the ones we get in CI so I'm not worrying too much about this. I guess the follow ups here are:
    • I'd be interested to know if you get the same behaviour as me (minor differences), or the same behaviour as CI (identical to production)
    • I think I'm going to hold off adding loads and loads of additional test cases until we've lived with this for a bit as I do worry this might end up being a bit brittle as we upgrade underlying libraries. If this turns out to be the case, we might want to either keep the number of cases small or think about tooling to make it really easy to re-snapshot all the images if we need to.

Final note: The lint job is failing, but I think the failing lint check is unrelated to the changes in this PR.

@shields-cd shields-cd temporarily deployed to squint-pr-132 November 27, 2022 20:38 Inactive
@shields-cd shields-cd temporarily deployed to squint-pr-132 November 27, 2022 20:48 Inactive
@shields-cd shields-cd temporarily deployed to squint-pr-132 November 27, 2022 20:59 Inactive
@chris48s chris48s changed the title WIP add visual regression testing add visual regression testing Nov 27, 2022
@calebcartwright calebcartwright merged commit a2e1cbc into main Feb 3, 2023
@calebcartwright calebcartwright deleted the visual_regression_testing branch February 3, 2023 03:16
@chris48s
Copy link
Member Author

chris48s commented Feb 3, 2023

Slowly working though that notification backlog 😄

Did you ever try this out locally to see if you ended up with the minor kerning difference, or not?

@calebcartwright
Copy link
Member

Slowly working though that notification backlog smile

Did you ever try this out locally to see if you ended up with the minor kerning difference, or not?

"Progress is slow but I'm in it for the long haul" 😁

I haven't tried this locally yet but I'd anticipate some similar differences just based on my experience during development. There's obviously a heavy reliance on lower level system packages, and that's why I've tried to pivot to using the docker image for as much normalization as possible.

I can try giving the tests a whirl locally later this week

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.

None yet

3 participants