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

feat: add snapshot tests #122

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

TimothyMakkison
Copy link
Contributor

Added snapshot tests to detect changes in logic and formatting.

@martinothamar
Copy link
Owner

You are on fire 🔥
I've never worked with snapshot tests before, what's the workflow like when these are added? If we make a change to the generated output (that we want), how do you update the snapshots?

@TimothyMakkison
Copy link
Contributor Author

I've never worked with snapshot tests before, what's the workflow like when these are added? If we make a change to the generated output (that we want), how do you update the snapshots?

Verify will compare the generated files to the equivalent verified.cs file in the _snapshots folder. Any differences will result in the test failing and causes a difftool to run. Accepting or rejecting the changes has the workflow same as using git. In my case a new window pops up using either the VSCode or Rider difftool.
Try chaging Mediator.sbn-cs and running the tests to see how it works on your machine.

Example

image

what's the workflow like when these are added?

Imo it makes playing around with code generation a lot easier 😄. I've used this in a separate branch to remove unneeded blank space. I personally check the difftool and the use an extension to accept all changes.

I considered combining this with the SamplesTests WDYT?

@martinothamar
Copy link
Owner

Finally tested this out, yep I love it, really useful and simple to use aswell. Thanks! 🙏

I considered combining this with the SamplesTests WDYT?

Yes I think thats a good idea, so that when we add new samples we get both snapshot testing and diagnostics checking in one go.

I'll merge this for now, but feel free follow up with that if you have the time

@martinothamar martinothamar merged commit ad0f48c into martinothamar:main Dec 7, 2023
1 check passed
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.

2 participants