-
-
Notifications
You must be signed in to change notification settings - Fork 136
Unify handling of file & inline snapshots #468
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
This is the code for mitsuhiko#456 (comment), as mentioned in mitsuhiko#466. There's a very small change in yaml inline snapshots — shown here in the tests. In return, it makes the macros simpler & more maintainable.
|
I'm not a huge fan of this. The current logic means that leading whitespace is handled better for yaml snapshots. What's the benefit of changing this? |
Couple of reasons:
(not my most important contribution, won't be offended if you close! Though I also don't quite see the reason to have the existing code...) |
|
Yeah this might be right. I'm generally quite hesitant to changing formatting the format. Today this choice means that every line starts with |
|
OK! I don't think there are any cases that would be worse like this, but if you have any lingering doubts then happy to test them... |
|
I think I would be okay making this change with insta 2. |
039e148 to
038b8b2
Compare
This is a version of #468 that doesn't break compatibility — the change in snapshot (removing the leading `---` in inline snapshots) is optional, and snapshots still pass if they have it. Otherwise it has the same benefits of #468 (There is a tiny corner case that if a snapshot is intended to test whether there's a leading `---`, we could get a false match, but this is a very small possibility — much much smaller than removing the line endings we've done) As part of that, this adds `as_str_exact` & `matches_fully` methods to `SnapshotContents`, which allows us to unify our normalization of snapshots contents, and allow detecting snapshots that pass but don't completely match; for example if we want to restore checking for line endings.
|
Closed in favor of #528 |
This is the code for #456 (comment), as mentioned in #466.
There's a very small breaking change in YAML inline snapshots — shown here in the tests. In return, it cuts some complicated code from
macros.rs, making them more maintainable and less likely to hit some of our recent issues, such as inconsistent support for trailing commas etc