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.

Sign up for GitHub

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

tests: run insta --force-update-snapshots #5558

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

tests: run insta --force-update-snapshots #5558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 14 additions & 16 deletions cli/src/formatter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ mod tests {
formatter.pop_label().unwrap();
write!(formatter, " after ").unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" before  inside  after ");
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" before  inside  after");
}

#[test]
Expand Down Expand Up @@ -907,15 +907,15 @@ mod tests {
formatter.pop_label().unwrap();
writeln!(formatter).unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @r###"
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @r"
 fg only 
 bg only 
 bold only 
 italic only 
 underlined only 
 single rule 
 two rules 
"###);
");
}

#[test]
Expand Down Expand Up @@ -1005,7 +1005,7 @@ mod tests {
write!(formatter, " after outer ").unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(),
@" before outer  before inner  inside inner  after inner  after outer ");
@" before outer  before inner  inside inner  after inner  after outer");
}

#[test]
Expand All @@ -1027,7 +1027,7 @@ mod tests {
formatter.pop_label().unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(),
@" not colored  colored  not colored ");
@" not colored  colored  not colored");
}

#[test]
Expand Down Expand Up @@ -1106,10 +1106,10 @@ mod tests {
write!(formatter, " and back.").unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(),
@r###"
@r"
Blue on yellow,  default fg,  and back.
Blue on yellow,  default bg,  and back.
"###);
");
}

#[test]
Expand Down Expand Up @@ -1149,7 +1149,7 @@ mod tests {
formatter.pop_label().unwrap();
formatter.pop_label().unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" hello ");
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" hello");
}

#[test]
Expand Down Expand Up @@ -1241,9 +1241,7 @@ mod tests {
write!(writer, "Message").unwrap();
writeln!(writer, " continues").unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @r###"
Heading: Message continues
"###);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @"Heading: Message continues");
}

#[test]
Expand All @@ -1259,7 +1257,7 @@ mod tests {
write!(writer, "").unwrap();
write!(writer, "").unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @"Heading: ");
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @"Heading:");
}

#[test]
Expand All @@ -1274,7 +1272,7 @@ mod tests {

insta::assert_snapshot!(
str::from_utf8(recorder.data()).unwrap(),
@" outer1 inner1 inner2 outer2 ");
@" outer1 inner1 inner2 outer2");
Copy link
Contributor

@yuja yuja Feb 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: It's not important that the trailing space is preserved here, but it looks a bit odd. We might have to add leading/trailing non-space character to the snapshot sample.

Copy link
Contributor

@ilyagr ilyagr Feb 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also file a bug to insta. (Update: Thought I'm not 100% sure they would consider it a bug. See below for another possibility)

Copy link
Contributor

@ilyagr ilyagr Feb 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a better option is to change the test (in a separate commit, like with inserting allow_duplicates!), so that we are in fact testing everything:

insta::assert_snapshot!(
            format!("|{}|",str::from_utf8(recorder.data()).unwrap()),
             @"| outer1  inner1  inner2  outer2 |")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. iirc, insta isn't strict about leading/trailing whitespaces, and I assume that's by design. I just pointed that out because this patch contained whitespace changes like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. iirc, insta isn't strict about leading/trailing whitespaces

What's weird is that it strips the whitespace at the end, but not at the start. I would have expected either both or neither.


// Replayed output should be labeled.
let config = config_from_string(r#" colors.inner = "red" "#);
Expand All @@ -1284,7 +1282,7 @@ mod tests {
drop(formatter);
insta::assert_snapshot!(
String::from_utf8(output).unwrap(),
@" outer1  inner1 inner2  outer2 ");
@" outer1  inner1 inner2  outer2");

// Replayed output should be split at push/pop_label() call.
let mut output: Vec<u8> = vec![];
Expand Down Expand Up @@ -1319,7 +1317,7 @@ mod tests {
recorder.replay(&mut formatter).unwrap();
drop(formatter);
insta::assert_snapshot!(
String::from_utf8(output).unwrap(), @" outer1  inner1 inner2  outer2 ");
String::from_utf8(output).unwrap(), @" outer1  inner1 inner2  outer2");

let mut output: Vec<u8> = vec![];
let mut formatter = ColorFormatter::for_config(&mut output, &config, false).unwrap();
Expand All @@ -1333,6 +1331,6 @@ mod tests {
.unwrap();
drop(formatter);
insta::assert_snapshot!(
String::from_utf8(output).unwrap(), @" outer1  inner1 inner2  outer2 ");
String::from_utf8(output).unwrap(), @" outer1  inner1 inner2  outer2");
}
}
2 changes: 1 addition & 1 deletion cli/src/git_util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ mod tests {
};
// First output is after the initial delay
assert_snapshot!(update(crate::progress::INITIAL_DELAY - Duration::from_millis(1), 0.1), @"");
assert_snapshot!(update(Duration::from_millis(1), 0.10), @"[?25l\r 10% [█▊ ]");
assert_snapshot!(update(Duration::from_millis(1), 0.10), @"\u{1b}[?25l\r 10% [█▊ ]\u{1b}[K");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be changed by mitsuhiko/insta#715 ? If that's the case, it's probably better to merge this PR after new insta is released.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also not very difficult to compile cargo-insta from source. I do it for mitsuhiko/insta#438.

cargo build -p cargo-insta --release && cp target/release/cargo-insta ~/.cargo/bin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be changed by mitsuhiko/insta#715?

Yes, this is with mitsuhiko/insta#716 merged.

The current logic is that if there are any bad control characters, then everything will be escaped.
So, because the string contains \r, the \x1b will be escaped as well.

One could imagine another solution where even if there are unrenderable control characters, only they are escaped, and \n, \t, \x1b are kept as is, but that is not implemented in insta right now.

// No updates for the next 30 milliseconds
assert_snapshot!(update(Duration::from_millis(10), 0.11), @"");
assert_snapshot!(update(Duration::from_millis(10), 0.12), @"");
Expand Down
20 changes: 10 additions & 10 deletions cli/src/merge_tools/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ mod tests {
ConflictMarkerStyle::Diff,
)
.unwrap();
insta::assert_debug_snapshot!(files, @r###"
insta::assert_debug_snapshot!(files, @r#"
[
File {
old_path: None,
Expand Down Expand Up @@ -821,7 +821,7 @@ mod tests {
],
},
]
"###);
"#);

let no_changes_tree_id = apply_diff_builtin(
store,
Expand Down Expand Up @@ -870,7 +870,7 @@ mod tests {
ConflictMarkerStyle::Diff,
)
.unwrap();
insta::assert_debug_snapshot!(files, @r###"
insta::assert_debug_snapshot!(files, @r#"
[
File {
old_path: None,
Expand All @@ -893,7 +893,7 @@ mod tests {
],
},
]
"###);
"#);
let no_changes_tree_id = apply_diff_builtin(
store,
&left_tree,
Expand Down Expand Up @@ -941,7 +941,7 @@ mod tests {
ConflictMarkerStyle::Diff,
)
.unwrap();
insta::assert_debug_snapshot!(files, @r###"
insta::assert_debug_snapshot!(files, @r#"
[
File {
old_path: None,
Expand All @@ -964,7 +964,7 @@ mod tests {
],
},
]
"###);
"#);
let no_changes_tree_id = apply_diff_builtin(
store,
&left_tree,
Expand Down Expand Up @@ -1013,7 +1013,7 @@ mod tests {
ConflictMarkerStyle::Diff,
)
.unwrap();
insta::assert_debug_snapshot!(files, @r###"
insta::assert_debug_snapshot!(files, @r#"
[
File {
old_path: None,
Expand All @@ -1036,7 +1036,7 @@ mod tests {
],
},
]
"###);
"#);
let no_changes_tree_id = apply_diff_builtin(
store,
&left_tree,
Expand Down Expand Up @@ -1103,7 +1103,7 @@ mod tests {
.unwrap();
let merge_result = files::merge(&content);
let sections = make_merge_sections(merge_result).unwrap();
insta::assert_debug_snapshot!(sections, @r###"
insta::assert_debug_snapshot!(sections, @r#"
[
Changed {
lines: [
Expand Down Expand Up @@ -1151,6 +1151,6 @@ mod tests {
],
},
]
"###);
"#);
}
}
12 changes: 6 additions & 6 deletions cli/src/merge_tools/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,11 +703,11 @@ mod tests {
insta::assert_debug_snapshot!(get(":builtin", "").unwrap(), @"Builtin");

// Just program name
insta::assert_debug_snapshot!(get("my diff", "").unwrap_err(), @r###"
insta::assert_debug_snapshot!(get("my diff", "").unwrap_err(), @r#"
MergeArgsNotConfigured {
tool_name: "my diff",
}
"###);
"#);

// Pick from merge-tools
insta::assert_debug_snapshot!(get(
Expand Down Expand Up @@ -763,11 +763,11 @@ mod tests {
insta::assert_debug_snapshot!(get("").unwrap(), @"Builtin");

// Just program name
insta::assert_debug_snapshot!(get(r#"ui.merge-editor = "my-merge""#).unwrap_err(), @r###"
insta::assert_debug_snapshot!(get(r#"ui.merge-editor = "my-merge""#).unwrap_err(), @r#"
MergeArgsNotConfigured {
tool_name: "my-merge",
}
"###);
"#);

// String args
insta::assert_debug_snapshot!(
Expand Down Expand Up @@ -871,11 +871,11 @@ mod tests {

// List args should never be a merge-tools key
insta::assert_debug_snapshot!(
get(r#"ui.merge-editor = ["meld"]"#).unwrap_err(), @r###"
get(r#"ui.merge-editor = ["meld"]"#).unwrap_err(), @r#"
MergeArgsNotConfigured {
tool_name: "meld",
}
"###);
"#);

// Invalid type
assert!(get(r#"ui.merge-editor.k = 0"#).is_err());
Expand Down
Loading