-
Notifications
You must be signed in to change notification settings - Fork 724
chore: implement a snapshot testing utility #5266
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
Draft
dougch
wants to merge
18
commits into
aws:main
Choose a base branch
from
dougch:snapshots
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
+2,598
−137
Conversation
This file contains hidden or 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
- Add #[allow(dead_code)] attributes to unused functions and variants - Fix unused variable warning by prefixing with underscore - Make TestSuites fields optional with default values - Fix Option<e> typo to Option<Error> in model.rs - Add test for parsing integrationv2_sni_match.xml - Mark tests with missing files as ignored
- Add test_parse_integration_files.rs to verify parsing of all integration XML files - Add test_snapshot_integration.rs to test snapshot creation and storage - Add test_cli_integration.rs to test CLI functionality - Update main.rs to support --dir parameter in capture and list commands This ensures the junit-snapshot tool can successfully parse and use all the integration test XML files.
- Add all integration test XML files as test inputs - Move snapshot_testing_utility_design_doc.md from docs/ to tools/ - These XML files are used by the new test suite to verify parsing and snapshot functionality
Add a comprehensive guide for new developers on how to use the junit-snapshot tool, including: - Initial setup and building - Creating and managing snapshots - Comparing test results against snapshots - Updating snapshots as tests evolve - Best practices and workflow examples This guide will help onboard new developers and provide a reference for working with the snapshot testing utility.
Replace symlink and PATH instructions with cargo install instructions, which is the recommended way to install Rust binaries. This provides a cleaner installation process that leverages Cargo's built-in package management capabilities.
- Add integrationv2_happy_path.xml (1.2MB) as a test input file - Modify parser to use buffered reading for large XML files - This change enables the previously ignored tests that depend on this file - The buffered approach is more memory efficient for large files
- Remove ignore annotations from tests that use integrationv2_happy_path.xml - Now that the large test file is available and the parser is improved, these tests can run successfully - This completes the test coverage for the snapshot functionality
This commit addresses two issues in the junit-snapshot tool: 1. Fix type error in model.rs where 'error: Option<e>' was incorrectly defined 2. Improve XML parsing to handle large files by: - Using BufReader with streaming parser instead of loading entire file - Calculating total test counts from individual test suites when not set in root - Properly handling memory-efficient deserialization These changes allow the parser to successfully process large XML files like integrationv2_happy_path.xml without memory issues while maintaining accurate test statistics.
Remove unused Read import and add #[allow(dead_code)] attribute to parse_junit_xml function to eliminate compiler warnings.
This commit adds a new 'compare' command to the JUnit snapshot utility that: - Compares a new JUnit file against a baseline snapshot - Detects matching, different, new, and missing tests - Shows detailed information about test status changes - Uses colored output to highlight differences - Supports a diff-only flag to show only differences - Supports a fail-on-diff flag for CI integration Also adds comprehensive tests for the compare command and updates documentation.
dougch
commented
Apr 22, 2025
@@ -32,7 +32,13 @@ | |||
# We're not including openssl1.1.1 in our package list to avoid confusing cmake. | |||
# It will be in the PATH of our devShell for use in tests. | |||
pythonEnv | |||
pkgs.valgrind | |||
# Pin to a specific version of valgrind that's maintained | |||
(pkgs.valgrind.overrideAttrs (oldAttrs: { |
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.
yes, this belongs elsewhere- I'll pull this out when we get further along.
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.
Release Summary:
Resolved issues:
#5097
Description of changes:
We'd like a tool like insta, but for our C project. Because pytest can output junit, I'm attempting to use that as a data-source to track test invocation.
Example use from my testing:
List output, note that numbers captured for each test end state; the test names are present in the snapshot file:
I then changed the CIPHERS in the ocsp test from ALL_CIPHERS to TLS13_CIPHERS, reran the ocsp test and compared the resulting xml file against the named snapshot
So it correctly identified that 201 tests were missed because of the change in ciphers.
Call-outs:
There are bugs. I don't love the name.
Testing:
How is this change tested (unit tests, fuzz tests, etc.)? What manual testing was performed? Are there any testing steps to be verified by the reviewer?
How can you convince your reviewers that this PR is safe and effective?
Is this a refactor change? If so, how have you proved that the intended behavior hasn't changed?
Remember:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.