Skip to content
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

Fix --require-full-match on inline snapshots with leading newlines #639

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 82 additions & 40 deletions cargo-insta/tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1024,20 +1024,27 @@ fn test_linebreaks() {

assert!(&output.status.success());

assert_snapshot!(test_project.diff("src/lib.rs"), @r#####"
--- Original: src/lib.rs
+++ Updated: src/lib.rs
@@ -1,8 +1,5 @@

#[test]
fn test_linebreaks() {
- insta::assert_snapshot!("foo", @r####"
- foo
-
- "####);
+ insta::assert_snapshot!("foo", @"foo");
}
"#####);
// When #563 merges, or #630 is resolved, this will change the snapshot. I
// also think it's possible to have it work sooner, but have iterated quite
// a few times trying to get this to work, and then finding something else
// without test coverage didn't work; so not sure it's a great investment of
// time.
assert_snapshot!(test_project.diff("src/lib.rs"), @"");

// assert_snapshot!(test_project.diff("src/lib.rs"), @r#####"
// --- Original: src/lib.rs
// +++ Updated: src/lib.rs
// @@ -1,8 +1,5 @@

// #[test]
// fn test_linebreaks() {
// - insta::assert_snapshot!("foo", @r####"
// - foo
// -
// - "####);
// + insta::assert_snapshot!("foo", @"foo");
// }
// "#####);
}

#[test]
Expand Down Expand Up @@ -1123,48 +1130,83 @@ fn test_wrong_indent_force() {
)
.create_project();

// Confirm the test passes despite the indent
// ...and that it passes with `--require-full-match`. Note that ideally this
// would fail, but we can't read the desired indent without serde, which is
// in `cargo-insta` only. So this tests the current state rather than the
// ideal state (and I don't think there's a reasonable way to get the ideal state)
// Now confirm that `--require-full-match` passes
let output = test_project
.insta_cmd()
.args(["test", "--check", "--", "--nocapture"])
.args([
"test",
"--check",
"--require-full-match",
"--",
"--nocapture",
])
.output()
.unwrap();
assert!(&output.status.success());
}

#[test]
fn test_matches_fully_linebreaks() {
// Until #563 merges, we should be OK with different leading newlines, even
// in exact / full match mode.
let test_project = TestFiles::new()
.add_file(
"Cargo.toml",
r#"
[package]
name = "exact-match-inline"
version = "0.1.0"
edition = "2021"

[lib]
doctest = false

[dependencies]
insta = { path = '$PROJECT_PATH' }
"#
.to_string(),
)
.add_file(
"src/lib.rs",
r#####"
#[test]
fn test_additional_linebreak() {
// Additional newline here
insta::assert_snapshot!(r#"

(
"name_foo",
"insta_tests__tests",
)
"#, @r#"
(
"name_foo",
"insta_tests__tests",
)
"#);
}
"#####
.to_string(),
)
.create_project();

// Then run the test with --force-update-snapshots and --accept to confirm
// the new snapshot is written
// Confirm the test passes despite the indent
let output = test_project
.insta_cmd()
.args([
"test",
"--force-update-snapshots",
"--accept",
"--check",
"--require-full-match",
"--",
"--nocapture",
])
.output()
.unwrap();
assert!(&output.status.success());

// https://github.com/mitsuhiko/insta/pull/563 will fix the starting &
// ending newlines
assert_snapshot!(test_project.diff("src/lib.rs"), @r##"
--- Original: src/lib.rs
+++ Updated: src/lib.rs
@@ -4,8 +4,8 @@
insta::assert_snapshot!(r#"
foo
foo
- "#, @r#"
- foo
- foo
- "#);
+ "#, @r"
+ foo
+ foo
+ ");
}
"##);
}

#[test]
Expand Down
18 changes: 15 additions & 3 deletions insta/src/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,20 @@ impl Snapshot {
pub fn matches_fully(&self, other: &Self) -> bool {
match (self.contents(), other.contents()) {
(SnapshotContents::Text(self_contents), SnapshotContents::Text(other_contents)) => {
let contents_match_exact = self_contents == other_contents;
// Note that we previously would match the exact values of the
// unnormalized text. But that's too strict — it means we can
// never match a snapshot that has leading/trailing whitespace.
// So instead we check it matches on the latest format.
// Generally those should be the same — latest should be doing
// the minimum normalization; if they diverge we could update
// this to be stricter.
//
// (I think to do this perfectly, we'd want to match the
// _reference_ value unnormalized, but the _generated_ value
// normalized. That way, we can get the But at the moment we
// don't distinguish between which is which in our data
// structures.)
let contents_match_exact = self_contents.matches_latest(other_contents);
match self_contents.kind {
TextSnapshotKind::File => {
self.metadata.trim_for_persistence()
Expand Down Expand Up @@ -822,8 +835,7 @@ fn min_indentation(snapshot: &str) -> usize {
.unwrap_or(0)
}

// Removes excess indentation, removes excess whitespace at start & end
// and changes newlines to \n.
/// Removes excess indentation, and changes newlines to \n.
fn normalize_inline_snapshot(snapshot: &str) -> String {
let indentation = min_indentation(snapshot);
snapshot
Expand Down
Loading