Add --- to the end of snapshots
#506
Conversation
(requires some thought / discussion) This would allow us te handle ending newlines. We'd need to do some work - to check newlines - to handle the transition when some files won't have these ending items, to only check those that do (...possibly work we should do before we merge this?)
|
We have the same problem with leading whitespace too. I think it needs addressing but I'm not convinced that this is the right way to do it. |
No prob ofc
Right — just tbc, if we have delimiters at the start & end, then it becomes possible to check for the number of newlines. IIUC, we previously gave up and just (Note that this PR doesn't check newlines, it just adds the delimiters so that we can later check them...) |
|
I'm thinking the right solution here would be that if
We add a unicode character to mark the beginning or end of it. I was thinking of inserting |
How about discriminating between zero and one trailing newline? To ensure we're on the same page — at least for file snapshots — I think this is only a problem because of tools which change ending newlines (for example lots of repos have lints which enforce exactly one ending newline). Otherwise, we can just put the value after the header and we're done. Or am I missing something? If that's the case, why do we need to handle the beginning? |
|
So to be specific on the challenge: the leading whitespace is a problem in inline cases because of whitespace removal for indentation handling. If the leading whitespace is meaningful you lose the meaning in the assertions. The trailing one has similar challenges, but the newline specific case is indeed just for file snapshots due to editor settings. |
I would claim newlines in inline snapshots are a very tractable problem: #457 (comment) — inline snapshots are either short and contain zero newlines, or long and always have exactly one newline trimmed at the start & end... (I agree indentation isn't possible to control like this, though also that's a less frequent issue) |
This is an example of what I've been suggesting at #506 (+ some linked issues): - ~Stacked on #528 so that would need to merge first~ - Enforces the correct number of newlines at the start of snapshots. The start is easier because we don't need to worry about editor issues in trailing newlines (and don't need to worry about how `Lines` deals ambiguously with trailing new lines) - Works for both inline & file snapshots - Adds a `matches_legacy` method on snapshot contents — this encapsulates the older formats we still support and warns about them ~It's not ready to merge yet, because we don't seem to actually support `--force-update-snapshots` on inline values, which is somewhat required. But I'm publishing for feedback & to put some weight on #528 in case we don't want to pursue.~ --------- Co-authored-by: Claude <no-reply@anthropic.com>
Co-authored-by: Claude <no-reply@anthropic.com>
- Add closing --- marker when writing new snapshots - Maintain backward compatibility with old snapshots without the marker - Handle edge case where old snapshot content naturally ends with --- - Add comprehensive functional tests for all scenarios - Test all macro types (snapshot, yaml, json, debug) get the marker - Binary snapshots correctly don't get the text closing marker
Replace backslashes with forward slashes in CARGO_MANIFEST_DIR path to avoid TOML parsing errors on Windows
The standard test framework already handles Windows path replacement correctly via the $PROJECT_PATH placeholder.
Only test debug snapshots which don't need serde or special cargo features. This makes the test simpler and more consistent with other tests in the file.
Co-authored-by: Claude <no-reply@anthropic.com>
2 participants
(requires some thought / discussion)
This would allow us to handle ending newlines.
We'd need to do some work
(...possibly work we should do before we merge this?)