-
Notifications
You must be signed in to change notification settings - Fork 3
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
Introduce proper snapshot testing #80
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
awelzel
reviewed
Jul 16, 2024
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.
I have no strong opinion or knowledge if syrupy is the proper tool to use, but seems quite nice. It does add a dependency on pytest, but appears ok to me, too.
bbannier
force-pushed
the
topic/bbannier/snapshot-testing
branch
from
July 16, 2024 13:22
c805016
to
cc30511
Compare
awelzel
approved these changes
Jul 16, 2024
We are currently using a huge file `tests/data/test1.zeek` for manual "snapshot testing". The idea is that one would add samples to the file and then manually update the baseline `tests/data/test1.zeek.out`. As this file grows this manual process becomes very cumbersome. Additionally when the test fails it can be very hard to identify which parts of the baseline differed. This patch introduces a dedicated test `test_samples` which automates this. The idea is that one would add new compact samples to `test/samples` which the test discovers automatically. When the test is run equality assertions are automatically routed to checks against managed snapshots which can be updated with `pytest --snapshot-update`. This should reduce overhead from adding/updating snapshots which should make it easier to add coverage. We will migrate the existing tests over in follow-up commits.
This test file is used for e.g., CLI tests and we want to keep it even with snapshot tests. This patch simplifies it so it is easier to maintain while still performing some formatting.
All source changes in this patch are created automatically from `black`.
These test files were used in a couple of tests but didn't need to be standalone. This patch moves them into `testutils`.
bbannier
force-pushed
the
topic/bbannier/snapshot-testing
branch
from
July 16, 2024 17:30
cc30511
to
fbb835c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds "proper" snapshot testing instead of the manual process of updating
tests/data/test1.zeek
andtests/data/test1.zeek.out
.