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 --unreferenced flag #634

Merged
merged 2 commits into from
Oct 4, 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
11 changes: 8 additions & 3 deletions cargo-insta/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ struct TestCommand {
test_runner: TestRunner,
#[arg(long)]
test_runner_fallback: Option<bool>,
/// Delete unreferenced snapshots after a successful test run.
/// Delete unreferenced snapshots after a successful test run (deprecated)
#[arg(long, hide = true)]
delete_unreferenced_snapshots: bool,
/// Disable force-passing of snapshot tests (deprecated)
Expand Down Expand Up @@ -769,7 +769,7 @@ fn handle_unreferenced_snapshots(
for package in loc.packages.clone() {
let unreferenced_snapshots = make_snapshot_walker(
package.manifest_path.parent().unwrap().as_std_path(),
&[".snap"],
&["snap"],
FindFlags {
include_ignored: true,
include_hidden: true,
Expand All @@ -780,7 +780,11 @@ fn handle_unreferenced_snapshots(
.filter(|e| {
e.file_name()
.to_str()
.map(|name| name.ends_with(".snap"))
.map(|name| {
loc.exts
.iter()
.any(|ext| name.ends_with(&format!(".{}", ext)))
})
.unwrap_or(false)
})
.filter_map(|e| e.path().canonicalize().ok())
Expand Down Expand Up @@ -1131,6 +1135,7 @@ fn show_undiscovered_hint(
.filter_map(|e| e.ok())
.filter(|x| {
let fname = x.file_name().to_string_lossy();
// TODO: use `extensions` here
fname.ends_with(".snap.new") || fname.ends_with(".pending-snap")
}) {
if !found_snapshots.contains(snapshot.path()) {
Expand Down
28 changes: 13 additions & 15 deletions cargo-insta/src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ pub(crate) fn find_pending_snapshots<'a>(
})
}

/// Creates a walker for snapshots & pending snapshots within a package.
/// Creates a walker for snapshots & pending snapshots within a package. The
/// walker returns snapshots ending in any of the supplied extensions, any of
/// the supplied extensions with a `.new` suffix, and `.pending-snap` files.
pub(crate) fn make_snapshot_walker(
package_root: &Path,
extensions: &[&str],
Expand All @@ -71,30 +73,26 @@ pub(crate) fn make_snapshot_walker(
}

let mut override_builder = OverrideBuilder::new(package_root);

extensions
.iter()
.map(|ext| format!("*.{}.new", ext))
.chain(
["*.pending-snap", "*.snap.new"]
.iter()
.map(ToString::to_string),
)
.flat_map(|ext| [format!("*.{}", ext), format!("*.{}.new", ext)])
.chain(std::iter::once("*.pending-snap".to_string()))
.for_each(|pattern| {
override_builder.add(&pattern).unwrap();
});

builder.overrides(override_builder.build().unwrap());
let root_path = package_root.to_path_buf();

let root_path = package_root.to_path_buf();
// Add a custom filter to skip interior crates; otherwise we get duplicate
// snapshots (https://github.com/mitsuhiko/insta/issues/396)
builder.filter_entry(move |entry| {
if entry.file_type().map_or(false, |ft| ft.is_dir()) {
let cargo_toml_path = entry.path().join("Cargo.toml");
if cargo_toml_path.exists() && entry.path() != root_path {
// Skip this directory if it contains a Cargo.toml and is not the root
return false;
}
if entry.file_type().map_or(false, |ft| ft.is_dir())
&& entry.path().join("Cargo.toml").exists()
&& entry.path() != root_path
{
// Skip this directory if it contains a Cargo.toml and is not the root
return false;
}
// We always want to skip `target` even if it was not excluded by
// ignore files.
Expand Down
103 changes: 103 additions & 0 deletions cargo-insta/tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1454,3 +1454,106 @@ fn test_hello() {
Hello, world!
"#);
}

#[test]
fn test_unreferenced_delete() {
let test_project = TestFiles::new()
.add_file(
"Cargo.toml",
r#"
[package]
name = "test_unreferenced_delete"
version = "0.1.0"
edition = "2021"

[lib]
doctest = false

[dependencies]
insta = { path = '$PROJECT_PATH' }
"#
.to_string(),
)
.add_file(
"src/lib.rs",
r#"
#[cfg(test)]
mod tests {
#[test]
fn test_snapshot() {
insta::assert_snapshot!("Hello, world!");
}
}
"#
.to_string(),
)
.create_project();

// Run tests to create snapshots
let output = test_project
.insta_cmd()
.args(["test", "--accept"])
.output()
.unwrap();

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

// Manually add an unreferenced snapshot
let unreferenced_snapshot_path = test_project
.workspace_dir
.join("src/snapshots/test_unreferenced_delete__tests__unused_snapshot.snap");
std::fs::create_dir_all(unreferenced_snapshot_path.parent().unwrap()).unwrap();
std::fs::write(
&unreferenced_snapshot_path,
r#"---
source: src/lib.rs
expression: "Unused snapshot"
---
Unused snapshot
"#,
)
.unwrap();

assert_snapshot!(test_project.file_tree_diff(), @r"
--- Original file tree
+++ Updated file tree
@@ -1,4 +1,8 @@

+ Cargo.lock
Cargo.toml
src
src/lib.rs
+ src/snapshots
+ src/snapshots/test_unreferenced_delete__tests__snapshot.snap
+ src/snapshots/test_unreferenced_delete__tests__unused_snapshot.snap
");

// Run cargo insta test with --unreferenced=delete
let output = test_project
.insta_cmd()
.args([
"test",
"--unreferenced=delete",
"--accept",
"--",
"--nocapture",
])
.output()
.unwrap();

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

// We should now see the unreferenced snapshot deleted
assert_snapshot!(test_project.file_tree_diff(), @r"
--- Original file tree
+++ Updated file tree
@@ -1,4 +1,7 @@

+ Cargo.lock
Cargo.toml
src
src/lib.rs
+ src/snapshots
+ src/snapshots/test_unreferenced_delete__tests__snapshot.snap
");
}
Loading