-
Notifications
You must be signed in to change notification settings - Fork 1.1k
add docs for snapshot test management #49302
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds documentation on how snapshot-based testing is managed in the .NET SDK, focusing on CLI completions and the usage of Verify for snapshot comparisons.
- Adds a new guide explaining the snapshot testing process and baselines.
- Details the MSBuild Targets used for diffing and updating CLI snapshots.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
To fix these tests, you need to diff the two files and visually inspect the changes. If the changes to the 'received' file are what you want to see (new commands, new options, renames, etc) then you rename the 'received' file to 'verified' and commit that change to the 'verified' file. There are two MSBuild Targets on the dotnet.Tests project that you can use to help you do this, both of which are intended to be run after you run the snapshot tests locally: | ||
|
||
* [CompareCliSnapshots][compare] - this Target copies the .received. files from the artifacts directory, where they are created due to the way we run tests, to the [snapshots][snapshots] directory in the dotnet.Tests project. This makes it much easier to diff the two. | ||
* [UpdateCliSnapshots][update] - this Target renames the .received. files to .verified. in the local [snapshots][snapshots] directory, and so acts as a giant 'I accept these changes' button. Only use this if you've diffed the snapshots and are sure they match your expectations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* [UpdateCliSnapshots][update] - this Target renames the .received. files to .verified. in the local [snapshots][snapshots] directory, and so acts as a giant 'I accept these changes' button. Only use this if you've diffed the snapshots and are sure they match your expectations. | |
* [UpdateCliSnapshots][update] - this Target renames the .received. files to .verified. in the local [snapshots][snapshots] directory, and so acts as a giant 'I accept these changes' button. Only use this if you've diffed the snapshots and are sure they match your expectations. | |
The snapshot commands can be utilized like so from the `Test` directory, only _after running the tests_: | |
`dotnet restore .\dotnet.Tests\ /t:CompareCliSnapshots & dotnet restore .\dotnet.Tests\ /t:UpdateCliSnapshots` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's just me, but I'd commit other things first, then run UpdateCliSnapshots directly and git diff to see what changed, only then checking if it looks right and committing. I feel like this implies that that would be wrong, whereas I see it as the equivalent of making any other might-be-bad change that you haven't committed.
Otherwise good 🙂
|
||
These tests use [Verify][Verify] to perform snapshot testing - storing the results of a known good 'baseline' as a snapshot and comparing the output of the same action performed with changes in your PR. Verify calls these baselines 'verified' files. When the test computes a new snapshot for comparison, this is called a 'received' file. Verify compares the 'verified' file to the 'received' file for each test, and if they are not the same provides a git-diff in the console output for the test. | ||
|
||
To fix these tests, you need to diff the two files and visually inspect the changes. If the changes to the 'received' file are what you want to see (new commands, new options, renames, etc) then you rename the 'received' file to 'verified' and commit that change to the 'verified' file. There are two MSBuild Targets on the dotnet.Tests project that you can use to help you do this, both of which are intended to be run after you run the snapshot tests locally: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I tried this on my Windows machine, while the test itself and VSCode showed only a few lines different (for the command I added in my PR), when I renamed the file, git diff
showed the entire file changed for the zsh
and bash
. I'm guessing because of line endings. I see that .gitattributes
defines a specific line ending and encoding for the .verified.
files, so I'm not sure if I commit the changes if it would have been sorted out. But I didn't want to risk my PR completely rewriting the file, so I ended up putting in extra effort of cloning the repo in WSL, configuring git in WSL with my email and name, commiting, and pushing, including getting the auth to work for the push.
So, if gitattributes would have sorted out the commit on Windows, these docs reassuring me would have helped here, because I've had line ending issues in the NuGet.Client repo causing entire files appear to be changed because of line endings. If I did need to run the tests in WSL to avoid the line ending issue, then it would be nice for the process to be improved, because this type of development barrier discourages me from making non-essential contributions.
Also, FWIW, I don't understand why the fish
and nushell
tests passed without modification. I'm still waiting for my PR's CI to finish to see if they fail in CI despite passing on my local machine (both in Windows and WSL).
Finally, an FYI, I keep telling everyone in NuGet that in markdown you can do one sentence per line, and the HTML rendering will be unmodified. But one sentence per line makes it easier to comment on a specific sentence within the paragraph. I'm not sure if GitHub has fixed it, but in the past GitHub's website wouldn't highlight the substring within the line that has been modified when very long lines are changed, so one sentence per line reduces the risk of that. So, in my opinion there are multiple advantages to having one sentence per line in the markdown and no downside.
/ba-g stuck |
No description provided.