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

Canonicalize snapshot paths before reducing #626

Merged
merged 2 commits into from
Oct 1, 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
1 change: 1 addition & 0 deletions cargo-insta/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ tempfile = "3.5.0"
semver = {version = "1.0.7", features = ["serde"]}
lazy_static = "1.4.0"
clap = { workspace=true }
itertools = "0.10.0"

[dev-dependencies]
walkdir = "2.3.1"
Expand Down
28 changes: 18 additions & 10 deletions cargo-insta/src/cargo.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use std::path::PathBuf;

pub(crate) use cargo_metadata::Package;
use itertools::Itertools;

/// Find snapshot roots within a package
// (I'm not sure how necessary this is; relative to just using all paths?)
// We need this because paths are not always conventional — for example cargo
// can reference artifacts that are outside of the package root.
pub(crate) fn find_snapshot_roots(package: &Package) -> Vec<PathBuf> {
let mut roots = Vec::new();

Expand Down Expand Up @@ -38,13 +40,19 @@ pub(crate) fn find_snapshot_roots(package: &Package) -> Vec<PathBuf> {
// we would encounter paths twice. If we don't skip them here we run
// into issues where the existence of a build script causes a snapshot
// to be picked up twice since the same path is determined. (GH-15)
roots.sort_by_key(|x| x.as_os_str().len());
let mut reduced_roots = vec![];
for root in roots {
if !reduced_roots.iter().any(|x| root.starts_with(x)) {
reduced_roots.push(root);
}
}

reduced_roots
let canonical_roots: Vec<_> = roots
.iter()
.filter_map(|x| x.canonicalize().ok())
.sorted_by_key(|x| x.as_os_str().len())
.collect();

canonical_roots
.clone()
.into_iter()
.filter(|root| {
!canonical_roots
.iter()
.any(|x| root.starts_with(x) && root != x)
})
.collect()
}
113 changes: 113 additions & 0 deletions cargo-insta/tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1303,3 +1303,116 @@ fn test_insta_workspace_root() {
Some(("INSTA_WORKSPACE_ROOT", moved_workspace.to_str().unwrap())),
));
}

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

[dependencies]
insta = { path = '$PROJECT_PATH' }

[[test]]
name = "tlib"
path = "../tests/lib.rs"
"#
.to_string(),
)
.add_file(
"proj/src/lib.rs",
r#"
pub fn hello() -> String {
"Hello, world!".to_string()
}
"#
.to_string(),
)
.add_file(
"tests/lib.rs",
r#"
use external_test_path::hello;

#[test]
fn test_hello() {
insta::assert_snapshot!(hello());
}
"#
.to_string(),
)
.create_project();

// Change to the proj directory for running cargo commands
let proj_dir = test_project.workspace_dir.join("proj");

// Initially, the test should fail
let output = test_project
.cmd()
.current_dir(&proj_dir)
.args(["test", "--"])
.output()
.unwrap();

assert_failure(&output);

// Verify that the snapshot was created in the correct location
assert_snapshot!(TestProject::current_file_tree(&test_project.workspace_dir), @r"
proj
proj/Cargo.lock
proj/Cargo.toml
proj/src
proj/src/lib.rs
tests
tests/lib.rs
tests/snapshots
tests/snapshots/tlib__hello.snap.new
");

// Run cargo insta accept
let output = test_project
.cmd()
.current_dir(&proj_dir)
.args(["test", "--accept"])
.output()
.unwrap();

assert_success(&output);

// Verify that the snapshot was created in the correct location
assert_snapshot!(TestProject::current_file_tree(&test_project.workspace_dir), @r"
proj
proj/Cargo.lock
proj/Cargo.toml
proj/src
proj/src/lib.rs
tests
tests/lib.rs
tests/snapshots
tests/snapshots/tlib__hello.snap
");

// Run the test again, it should pass now
let output = Command::new(env!("CARGO_BIN_EXE_cargo-insta"))
.current_dir(&proj_dir)
.args(["test"])
.output()
.unwrap();

assert_success(&output);

let snapshot_path = test_project
.workspace_dir
.join("tests/snapshots/tlib__hello.snap");
assert_snapshot!(fs::read_to_string(snapshot_path).unwrap(), @r#"
---
source: "../tests/lib.rs"
expression: hello()
---
Hello, world!
"#);
}
Loading