Skip to content

Commit

Permalink
Fix passing --workspace-root to cargo-insta (#658)
Browse files Browse the repository at this point in the history
This wasn't working, I think because of my recent changes -- now fixed
and tested
  • Loading branch information
max-sixty authored Oct 11, 2024
1 parent ab7957d commit a982c4c
Show file tree
Hide file tree
Showing 2 changed files with 195 additions and 13 deletions.
29 changes: 21 additions & 8 deletions cargo-insta/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ fn handle_color(color: Option<ColorWhen>) {
}
}

#[derive(Debug)]
struct LocationInfo<'a> {
tool_config: ToolConfig,
workspace_root: PathBuf,
Expand Down Expand Up @@ -445,6 +446,14 @@ fn handle_target_args<'a>(
let workspace_root = metadata.workspace_root.as_std_path().to_path_buf();
let tool_config = ToolConfig::from_workspace(&workspace_root)?;

let insta_version = metadata
.packages
.iter()
.find(|package| package.name == "insta")
.map(|package| package.version.clone())
.ok_or_else(|| eprintln!("insta not found in cargo metadata; defaulting to 1.0.0"))
.unwrap_or(Version::new(1, 0, 0));

// If `--workspace` is passed, or there's no root package, we include all
// packages. If packages are specified, we filter from all packages.
// Otherwise we use just the root package.
Expand All @@ -460,17 +469,17 @@ fn handle_target_args<'a>(
.into_iter()
.filter(|p| packages.is_empty() || packages.contains(&p.name))
.cloned()
.map(|mut p| {
// Dependencies aren't needed and bloat the object (but we can't pass
// `--no-deps` to the original command as we collect the insta
// version above...)
p.dependencies = vec![];
p
})
.collect()
} else {
vec![metadata.root_package().unwrap().clone()]
};
let insta_version = metadata
.packages
.iter()
.find(|package| package.name == "insta")
.map(|package| package.version.clone())
.ok_or_else(|| eprintln!("insta not found in cargo metadata; defaulting to 1.0.0"))
.unwrap_or(Version::new(1, 0, 0));

Ok(LocationInfo {
workspace_root,
Expand Down Expand Up @@ -710,6 +719,10 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box<dyn Error>
process_snapshots(true, None, &loc, Some(Operation::Reject))?;
}

if let Some(workspace_root) = &cmd.target_args.workspace_root {
proc.current_dir(workspace_root);
}

// Run the tests
let status = proc.status()?;
let mut success = status.success();
Expand Down Expand Up @@ -1006,7 +1019,7 @@ fn prepare_test_runner<'snapshot_ref>(
proc.arg("--exclude");
proc.arg(spec);
}
if let Some(ref manifest_path) = cmd.target_args.manifest_path {
if let Some(ref manifest_path) = &cmd.target_args.manifest_path {
proc.arg("--manifest-path");
proc.arg(manifest_path);
}
Expand Down
179 changes: 174 additions & 5 deletions cargo-insta/tests/functional/workspace.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use std::{
fs,
process::{Command, Stdio},
};
use std::{fs, process::Stdio};

use insta::assert_snapshot;

Expand Down Expand Up @@ -480,6 +477,8 @@ fn test_snapshot() {
.success());
}

/// A cargo target that references a file outside of the project's directory
/// should still work
#[test]
fn test_external_test_path() {
let test_project = TestFiles::new()
Expand Down Expand Up @@ -573,7 +572,8 @@ fn test_hello() {
");

// Run the test again, it should pass now
let output = Command::new(env!("CARGO_BIN_EXE_cargo-insta"))
let output = test_project
.insta_cmd()
.current_dir(&proj_dir)
.args(["test"])
.output()
Expand All @@ -593,3 +593,172 @@ fn test_hello() {
Hello, world!
"#);
}

/// Check that `--workspace-root` points `cargo-insta` at another path
#[test]
fn test_workspace_root_option() {
let test_project = TestFiles::new()
.add_file(
"Cargo.toml",
r#"
[package]
name = "workspace_root_test"
version = "0.1.0"
edition = "2021"
[lib]
doctest = false
[dependencies]
insta = { path = '$PROJECT_PATH' }
"#
.to_string(),
)
.add_file(
"src/lib.rs",
r#"
pub fn hello() -> String {
"Hello from workspace root!".to_string()
}
#[test]
fn test_hello() {
insta::assert_snapshot!(hello());
}
#[test]
fn test_inline() {
insta::assert_snapshot!("This is an inline snapshot", @"");
}
"#
.to_string(),
)
.create_project();

// Run the test with --workspace-root option
let output = test_project
.insta_cmd()
.current_dir(std::env::current_dir().unwrap()) // Run from the current directory
.args([
"test",
"--accept",
"--workspace-root",
test_project.workspace_dir.to_str().unwrap(),
])
.output()
.unwrap();

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

// Verify inline snapshot
assert_snapshot!(test_project.diff("src/lib.rs"), @r#"
--- Original: src/lib.rs
+++ Updated: src/lib.rs
@@ -10,5 +10,5 @@
#[test]
fn test_inline() {
- insta::assert_snapshot!("This is an inline snapshot", @"");
+ insta::assert_snapshot!("This is an inline snapshot", @"This is an inline snapshot");
}
"#);

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/workspace_root_test__hello.snap
");
}

/// Check that `--manifest` points `cargo-insta` at another path
#[test]
fn test_manifest_option() {
let test_project = TestFiles::new()
.add_file(
"Cargo.toml",
r#"
[package]
name = "manifest_path_test"
version = "0.1.0"
edition = "2021"
[lib]
doctest = false
[dependencies]
insta = { path = '$PROJECT_PATH' }
"#
.to_string(),
)
.add_file(
"src/lib.rs",
r#"
pub fn greeting() -> String {
"Greetings from manifest path!".to_string()
}
#[test]
fn test_greeting() {
insta::assert_snapshot!(greeting());
}
#[test]
fn test_inline() {
insta::assert_snapshot!("This is an inline snapshot for manifest path test", @"");
}
"#
.to_string(),
)
.create_project();

// Run the test with --manifest-path option
let output = test_project
.insta_cmd()
.current_dir(std::env::current_dir().unwrap()) // Run from the current directory
.args([
"test",
"--accept",
"--manifest-path",
test_project
.workspace_dir
.join("Cargo.toml")
.to_str()
.unwrap(),
])
.output()
.unwrap();

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

// Verify inline snapshot
assert_snapshot!(test_project.diff("src/lib.rs"), @r##"
--- Original: src/lib.rs
+++ Updated: src/lib.rs
@@ -10,5 +10,5 @@
#[test]
fn test_inline() {
- insta::assert_snapshot!("This is an inline snapshot for manifest path test", @"");
+ insta::assert_snapshot!("This is an inline snapshot for manifest path test", @"This is an inline snapshot for manifest path test");
}
"##);
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/manifest_path_test__greeting.snap
");
}

0 comments on commit a982c4c

Please sign in to comment.