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

--force-update-snapshots always writes, and implies --accept #644

Merged
merged 5 commits into from
Oct 7, 2024
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
14 changes: 10 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,16 @@ All notable changes to insta and cargo-insta are documented here.

- Experimental support for binary snapshots. #610 (Florian Plattner)

- `--force-update-snapshots` has more conservative and consistent behavior for
inline snapshots. As a side-effect of this, only the content within the inline
snapshot delimiters are assessed for changes, not the delimiters (e.g. `###`).
#581
- `--force-update-snapshots` now writes every snapshot, regardless of whether
`insta` evaluates a write is required, and now implies `--accept`. This
allows for `--force-update-snapshots` to update inline snapshots when
delimiters or indents require updating.

For the existing behavior of limiting writes which `insta` can evaluate are
required, use `--require-full-match`. The main difference between
`--require-full-match` and the existing behavior of `--force-update-snapshots`
is that the test run will return a non-zero exit code if any snapshots don't
match fully. #644

- Inline snapshots only use `#` characters as delimiters when required. #603

Expand Down
7 changes: 6 additions & 1 deletion cargo-insta/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ struct TestCommand {
/// Do not reject pending snapshots before run.
#[arg(long)]
keep_pending: bool,
/// Update all snapshots even if they are still matching.
/// Update all snapshots even if they are still matching; implies `--accept`.
#[arg(long)]
force_update_snapshots: bool,
/// Handle unreferenced snapshots after a successful test run.
Expand Down Expand Up @@ -618,6 +618,7 @@ fn process_snapshots(
Ok(())
}

/// Run the tests
fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box<dyn Error>> {
let loc = handle_target_args(&cmd.target_args, &cmd.test_runner_options.package)?;
match loc.tool_config.snapshot_update() {
Expand All @@ -644,6 +645,10 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box<dyn Error>
cmd.force_update_snapshots = true;
}
}
// `--force-update-snapshots` implies `--accept`
if cmd.force_update_snapshots {
cmd.accept = true;
}
if cmd.no_force_pass {
cmd.check = true;
eprintln!(
Expand Down
68 changes: 29 additions & 39 deletions cargo-insta/tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1012,39 +1012,27 @@ fn test_linebreaks() {
// Run the test with --force-update-snapshots and --accept
let output = test_project
.insta_cmd()
.args([
"test",
"--force-update-snapshots",
"--accept",
"--",
"--nocapture",
])
.args(["test", "--force-update-snapshots", "--", "--nocapture"])
.output()
.unwrap();

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

// 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");
// }
// "#####);
// Linebreaks should be reset
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 @@ -1078,22 +1066,24 @@ fn test_excessive_hashes() {
// Run the test with --force-update-snapshots and --accept
let output = test_project
.insta_cmd()
.args([
"test",
"--force-update-snapshots",
"--accept",
"--",
"--nocapture",
])
.args(["test", "--force-update-snapshots", "--", "--nocapture"])
.output()
.unwrap();

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

// TODO: we would like to update the number of hashes, but that's not easy
// given the reasons at https://github.com/mitsuhiko/insta/pull/573. So this
// result asserts the current state rather than the desired state.
assert_snapshot!(test_project.diff("src/lib.rs"), @"");
// `--force-update-snapshots` should remove the hashes
assert_snapshot!(test_project.diff("src/lib.rs"), @r#####"
--- Original: src/lib.rs
+++ Updated: src/lib.rs
@@ -1,5 +1,5 @@

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

#[test]
Expand Down
12 changes: 1 addition & 11 deletions insta/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -838,17 +838,7 @@ pub fn assert_snapshot(
ctx.tool_config.snapshot_update(),
crate::env::SnapshotUpdate::Force
) {
// Avoid creating new files if contents match exactly. In
// particular, this would otherwise create lots of unneeded files
// for inline snapshots
let matches_fully = &ctx
.old_snapshot
.as_ref()
.map(|x| x.matches_fully(&new_snapshot))
.unwrap_or(false);
if !matches_fully {
ctx.update_snapshot(new_snapshot)?;
}
ctx.update_snapshot(new_snapshot)?;
}
// otherwise print information and update snapshots.
} else {
Expand Down
Loading