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

Test snapshots #323

Merged
merged 6 commits into from
Mar 30, 2022
Merged

Test snapshots #323

merged 6 commits into from
Mar 30, 2022

Conversation

wezm
Copy link
Contributor

@wezm wezm commented Mar 29, 2022

This is an initial pass at adding and verifying test output via snapshots. snap files currently capture expected stdout and stderr for each test. A rudimentary diff is shown when a mismatch occurs. To update snapshots/create missing snap shots run the test with FATHOM_UPDATE_SNAP set (any value will do).

It's likely that there are further extensions/improvements that can be done to this. I figured these can be driven by the experience of using this initial version.

@brendanzab
Copy link
Member

Really cool stuff!

I wonder if we could add a hint to .gitattributes to render the *.snap files as TOML?

@wezm
Copy link
Contributor Author

wezm commented Mar 29, 2022

I wonder if we could add a hint to .gitattributes to render the *.snap files as TOML?

I wondered exactly this too! But my searches didn't yield anything useful. Are you aware if it's actually possible?

Edit: I tried again and found out how to do it.

@brendanzab
Copy link
Member

Hmm weirdly Github doesn't seem to be using it in the diffs. Maybe it needs to be committed first, or a caching issue? Anyway, not a massive deal.

Copy link
Member

@brendanzab brendanzab left a comment

Choose a reason for hiding this comment

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

Looks great!

@wezm
Copy link
Contributor Author

wezm commented Mar 29, 2022

Hmm weirdly Github doesn't seem to be using it in the diffs. Maybe it needs to be committed first, or a caching issue?

It's done on a low priority background job so might take a bit to update:

https://github.com/github/linguist/blob/master/docs/how-linguist-works.md#how-linguist-works-on-githubcom

@wezm
Copy link
Contributor Author

wezm commented Mar 29, 2022

@brendanzab any idea why the output would be different on different Rust versions?

The cube.stl test is failing the snapshot because at least the initial line is different ({ / [):

        1| - 0 = {

vs.

        96| + 0 = [

@brendanzab
Copy link
Member

brendanzab commented Mar 29, 2022

Ohhhh this seems like it might be testing the merge commit using #322? Where I now have a list of parsed expressions per position (we might have to revisit this approach to rendering these). So we might need to regenerate the snapshot. Not sure why it is different per rust version though - that is very strange 😵‍💫

@wezm
Copy link
Contributor Author

wezm commented Mar 30, 2022

Righto finally got a passing build

@wezm wezm merged commit eecb229 into main Mar 30, 2022
@wezm wezm deleted the wezm/snapshot-tests branch March 30, 2022 00:53
@brendanzab
Copy link
Member

Yay :)

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

2 participants