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

Integrate with snapshot testing #41

Closed
kvark opened this issue Mar 27, 2020 · 3 comments · Fixed by #317
Closed

Integrate with snapshot testing #41

kvark opened this issue Mar 27, 2020 · 3 comments · Fixed by #317
Labels
area: infra Project infrastructure help wanted Extra attention is needed kind: feature New feature or request

Comments

@kvark
Copy link
Member

kvark commented Mar 27, 2020

https://github.com/mitsuhiko/insta
Depends on #32

@kvark kvark added kind: feature New feature or request help wanted Extra attention is needed area: infra Project infrastructure labels Mar 27, 2020
paulkernfeld pushed a commit to paulkernfeld/naga that referenced this issue May 1, 2020
@paulkernfeld
Copy link
Contributor

I just pushed a branch with a prototype of this implementation, but maybe it doesn't make sense to open a PR until #48 is merged.

@kvark
Copy link
Member Author

kvark commented May 1, 2020

Ah I see. Yes, let's merge serialization first, then make it nice (the Handle stuff), then proceed with snapshots. And thank you for all the hard work here, it's amazing to see these changes coming ❤️

paulkernfeld pushed a commit to paulkernfeld/naga that referenced this issue May 4, 2020
@pjoe
Copy link
Collaborator

pjoe commented Aug 13, 2020

One issue: having the handle ids in the snapshots will fail the cases where ordering changes, but module is still semantically identical. Think e.g. a different glsl parser that orders expressions slightly different.

I'm not sure general snapshot testing can handle this. I've previously worked on some similar testing for a glTF exporter, where we ended up de-referencing indexed data before doing comparison between actual and expected results. In this way you 'abstract away' the specific ordering and index values, where they don't have any semantic difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: infra Project infrastructure help wanted Extra attention is needed kind: feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants