Repository navigation
Ignore whitespace-only lines in inline snapshot indentation - #949
Open
Shaisolaris wants to merge 1 commit into
Open
Shaisolaris wants to merge 1 commit into
Shaisolaris wants to merge 1 commit into
Conversation
Collaborator
|
I think you're right that there are a couple of values that violate the invariant that every accepted value also passes a test. but I think this does too much. (tables from LLM): Acceptance bug on the current default branch
Acceptance succeeds, but rerunning fails because the last two columns differ. Comparison that newly passes with #949
i.e. if the final column there is the reference snapshot, we would want a failure. I'd need to spend more time thinking whether this is possible to do both: I agree the invariant seems more important; but also we can't reverse a change once it's allowed; as the trail of deprecations demonstrates |
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
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.
A value with a whitespace-only line that is less indented than the rest fails against its own accepted inline snapshot.
min_indentationskips empty lines but counts whitespace-only lines. For" \n b"it picks one space, so the normalized value is"\n b". The whitespace-only line is now empty, so normalizing the written literal picks four spaces and gives"\nb". The two never compare equal, so accepting the snapshot does not make the test pass.This makes
min_indentationignore lines that contain only spaces and tabs, so normalization gives the same result on the second pass. Lines shorter than the indentation already become empty innormalize_inline.Testing:
test_min_indentation_ignores_whitespace_only_linesandtest_inline_snapshot_whitespace_only_line_roundtrip) fail before the change and pass after.cargo test -p instawith default,--all-features,--no-default-featuresand--features redactions, pluscargo test -p cargo-insta, all pass locally.to_inline,from_inline_literal, normalize; about 200k generated values, escaped format excluded) went from 3172 mismatches to 0. The fuzz is not part of this change.