diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs index 893e4849..38b3a6ea 100644 --- a/cargo-insta/tests/main.rs +++ b/cargo-insta/tests/main.rs @@ -27,10 +27,10 @@ /// the same...). This also causes issues when running the same tests /// concurrently. use std::collections::HashMap; -use std::env; use std::fs; use std::path::{Path, PathBuf}; use std::process::Command; +use std::{env, fs::remove_dir_all}; use console::style; use ignore::WalkBuilder; @@ -102,6 +102,16 @@ fn assert_success(output: &std::process::Output) { eprint!("{}", stderr); } +fn assert_failure(output: &std::process::Output) { + eprint!("{}", String::from_utf8_lossy(&output.stderr)); + eprint!("{}", String::from_utf8_lossy(&output.stdout)); + assert!( + !output.status.success(), + "Tests unexpectedly succeeded: {}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} struct TestProject { /// Temporary directory where the project is created workspace_dir: PathBuf, @@ -131,23 +141,26 @@ impl TestProject { workspace_dir, } } - fn cmd(&self) -> Command { - let mut command = Command::new(env!("CARGO_BIN_EXE_cargo-insta")); + fn clean_env(cmd: &mut Command) { // Remove environment variables so we don't inherit anything (such as // `INSTA_FORCE_PASS` or `CARGO_INSTA_*`) from a cargo-insta process // which runs this integration test. for (key, _) in env::vars() { if key.starts_with("CARGO_INSTA") || key.starts_with("INSTA") { - command.env_remove(&key); + cmd.env_remove(&key); } } // Turn off CI flag so that cargo insta test behaves as we expect // under normal operation - command.env("CI", "0"); + cmd.env("CI", "0"); // And any others that can affect the output - command.env_remove("CARGO_TERM_COLOR"); - command.env_remove("CLICOLOR_FORCE"); - command.env_remove("RUSTDOCFLAGS"); + cmd.env_remove("CARGO_TERM_COLOR"); + cmd.env_remove("CLICOLOR_FORCE"); + cmd.env_remove("RUSTDOCFLAGS"); + } + fn cmd(&self) -> Command { + let mut command = Command::new(env!("CARGO_BIN_EXE_cargo-insta")); + Self::clean_env(&mut command); command.current_dir(self.workspace_dir.as_path()); // Use the same target directory as other tests, consistent across test @@ -1166,3 +1179,127 @@ fn test_hashtag_escape() { } "####); } + +// Can't get the test binary discovery to work, don't have a windows machine to +// hand, others are welcome to fix it. (No specific reason to think that insta +// doesn't work on windows, just that the test doesn't work.) +#[cfg(not(target_os = "windows"))] +#[test] +fn test_insta_workspace_root() { + // This function locates the compiled test binary in the target directory. + // It's necessary because the exact filename of the test binary includes a hash + // that we can't predict, so we need to search for it. + fn find_test_binary(dir: &Path) -> PathBuf { + dir.join("target/debug/deps") + .read_dir() + .unwrap() + .filter_map(Result::ok) + .find(|entry| { + let file_name = entry.file_name(); + let file_name_str = file_name.to_str().unwrap_or(""); + // We're looking for a file that: + file_name_str.starts_with("insta_workspace_root_test-") // Matches our test name + && !file_name_str.contains('.') // Doesn't have an extension (it's the executable, not a metadata file) + && entry.metadata().map(|m| m.is_file()).unwrap_or(false) // Is a file, not a directory + }) + .map(|entry| entry.path()) + .expect("Failed to find test binary") + } + + fn run_test_binary( + binary_path: &Path, + current_dir: &Path, + env: Option<(&str, &str)>, + ) -> std::process::Output { + let mut cmd = Command::new(binary_path); + TestProject::clean_env(&mut cmd); + cmd.current_dir(current_dir); + if let Some((key, value)) = env { + cmd.env(key, value); + } + cmd.output().unwrap() + } + + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" + [package] + name = "insta_workspace_root_test" + version = "0.1.0" + edition = "2021" + + [dependencies] + insta = { path = '$PROJECT_PATH' } + "# + .to_string(), + ) + .add_file( + "src/lib.rs", + r#" + #[cfg(test)] + mod tests { + use insta::assert_snapshot; + + #[test] + fn test_snapshot() { + assert_snapshot!("Hello, world!"); + } + } + "# + .to_string(), + ) + .create_project(); + + let mut cargo_cmd = Command::new("cargo"); + TestProject::clean_env(&mut cargo_cmd); + let output = cargo_cmd + .args(["test", "--no-run"]) + .current_dir(&test_project.workspace_dir) + .output() + .unwrap(); + assert_success(&output); + + let test_binary_path = find_test_binary(&test_project.workspace_dir); + + // Run the test without snapshot (should fail) + assert_failure(&run_test_binary( + &test_binary_path, + &test_project.workspace_dir, + None, + )); + + // Create the snapshot + assert_success(&run_test_binary( + &test_binary_path, + &test_project.workspace_dir, + Some(("INSTA_UPDATE", "always")), + )); + + // Verify snapshot creation + assert!(test_project.workspace_dir.join("src/snapshots").exists()); + assert!(test_project + .workspace_dir + .join("src/snapshots/insta_workspace_root_test__tests__snapshot.snap") + .exists()); + + // Move the workspace + let moved_workspace = { + let moved_workspace = PathBuf::from("/tmp/cargo-insta-test-moved"); + remove_dir_all(&moved_workspace).ok(); + fs::create_dir(&moved_workspace).unwrap(); + fs::rename(&test_project.workspace_dir, &moved_workspace).unwrap(); + moved_workspace + }; + let moved_binary_path = find_test_binary(&moved_workspace); + + // Run test in moved workspace without INSTA_WORKSPACE_ROOT (should fail) + assert_failure(&run_test_binary(&moved_binary_path, &moved_workspace, None)); + + // Run test in moved workspace with INSTA_WORKSPACE_ROOT (should pass) + assert_success(&run_test_binary( + &moved_binary_path, + &moved_workspace, + Some(("INSTA_WORKSPACE_ROOT", moved_workspace.to_str().unwrap())), + )); +} diff --git a/insta/src/env.rs b/insta/src/env.rs index 776ef8b0..63ea8c4e 100644 --- a/insta/src/env.rs +++ b/insta/src/env.rs @@ -12,18 +12,16 @@ use crate::{ lazy_static::lazy_static! { static ref WORKSPACES: Mutex>> = Mutex::new(BTreeMap::new()); - static ref TOOL_CONFIGS: Mutex>> = Mutex::new(BTreeMap::new()); + static ref TOOL_CONFIGS: Mutex>> = Mutex::new(BTreeMap::new()); } -pub fn get_tool_config(manifest_dir: &str) -> Arc { - let mut configs = TOOL_CONFIGS.lock().unwrap(); - if let Some(rv) = configs.get(manifest_dir) { - return rv.clone(); - } - let config = - Arc::new(ToolConfig::from_manifest_dir(manifest_dir).expect("failed to load tool config")); - configs.insert(manifest_dir.to_string(), config.clone()); - config +pub fn get_tool_config(workspace_dir: &Path) -> Arc { + TOOL_CONFIGS + .lock() + .unwrap() + .entry(workspace_dir.to_path_buf()) + .or_insert_with(|| ToolConfig::from_workspace(workspace_dir).unwrap().into()) + .clone() } /// The test runner to use. @@ -125,11 +123,6 @@ pub struct ToolConfig { } impl ToolConfig { - /// Loads the tool config for a specific manifest. - pub fn from_manifest_dir(manifest_dir: &str) -> Result { - ToolConfig::from_workspace(&get_cargo_workspace(manifest_dir)) - } - /// Loads the tool config from a cargo workspace. pub fn from_workspace(workspace_dir: &Path) -> Result { let mut cfg = None; @@ -411,43 +404,61 @@ pub fn snapshot_update_behavior(tool_config: &ToolConfig, unseen: bool) -> Snaps /// Returns the cargo workspace for a manifest pub fn get_cargo_workspace(manifest_dir: &str) -> Arc { - // we really do not care about poisoning here. - let mut workspaces = WORKSPACES.lock().unwrap_or_else(|x| x.into_inner()); - if let Some(rv) = workspaces.get(manifest_dir) { - rv.clone() - } else { - // If INSTA_WORKSPACE_ROOT environment variable is set, use the value - // as-is. This is useful for those users where the compiled in - // CARGO_MANIFEST_DIR points to some transient location. This can easily - // happen if the user builds the test in one directory but then tries to - // run it in another: even if sources are available in the new - // directory, in the past we would always go with the compiled-in value. - // The compiled-in directory may not even exist anymore. - let path = if let Ok(workspace_root) = std::env::var("INSTA_WORKSPACE_ROOT") { - Arc::new(PathBuf::from(workspace_root)) - } else { + // If INSTA_WORKSPACE_ROOT environment variable is set, use the value as-is. + // This is useful where CARGO_MANIFEST_DIR at compilation points to some + // transient location. This can easily happen when building the test in one + // directory but running it in another. + if let Ok(workspace_root) = env::var("INSTA_WORKSPACE_ROOT") { + return PathBuf::from(workspace_root).into(); + } + + let error_message = || { + format!( + "`cargo metadata --format-version=1 --no-deps` in path `{}`", + manifest_dir + ) + }; + + WORKSPACES + .lock() + // we really do not care about poisoning here. + .unwrap() + .entry(manifest_dir.to_string()) + .or_insert_with(|| { let output = std::process::Command::new( - env::var("CARGO") - .ok() - .unwrap_or_else(|| "cargo".to_string()), + env::var("CARGO").unwrap_or_else(|_| "cargo".to_string()), ) - .arg("metadata") - .arg("--format-version=1") - .arg("--no-deps") + .args(["metadata", "--format-version=1", "--no-deps"]) .current_dir(manifest_dir) .output() - .unwrap(); - let docs = crate::content::yaml::vendored::yaml::YamlLoader::load_from_str( + .unwrap_or_else(|e| panic!("failed to run {}\n\n{}", error_message(), e)); + + crate::content::yaml::vendored::yaml::YamlLoader::load_from_str( std::str::from_utf8(&output.stdout).unwrap(), ) - .unwrap(); - let manifest = docs.first().expect("Unable to parse cargo manifest"); - let workspace_root = PathBuf::from(manifest["workspace_root"].as_str().unwrap()); - Arc::new(workspace_root) - }; - workspaces.insert(manifest_dir.to_string(), path.clone()); - path - } + .map_err(|e| e.to_string()) + .and_then(|docs| { + docs.into_iter() + .next() + .ok_or_else(|| "No content found in yaml".to_string()) + }) + .and_then(|metadata| { + metadata["workspace_root"] + .clone() + .into_string() + .ok_or_else(|| "Couldn't find `workspace_root`".to_string()) + }) + .map(|path| PathBuf::from(path).into()) + .unwrap_or_else(|e| { + panic!( + "failed to parse cargo metadata output from {}: {}\n\n{:?}", + error_message(), + e, + output.stdout + ) + }) + }) + .clone() } #[cfg(feature = "_cargo_insta_internal")] diff --git a/insta/src/glob.rs b/insta/src/glob.rs index e901702d..faec3a8b 100644 --- a/insta/src/glob.rs +++ b/insta/src/glob.rs @@ -38,7 +38,7 @@ lazy_static::lazy_static! { }; } -pub fn glob_exec(manifest_dir: &str, base: &Path, pattern: &str, mut f: F) { +pub fn glob_exec(workspace_dir: &Path, base: &Path, pattern: &str, mut f: F) { // If settings.allow_empty_glob() == true and `base` doesn't exist, skip // everything. This is necessary as `base` is user-controlled via `glob!/3` // and may not exist. @@ -61,7 +61,7 @@ pub fn glob_exec(manifest_dir: &str, base: &Path, pattern: &str GLOB_STACK.lock().unwrap().push(GlobCollector { failed: 0, show_insta_hint: false, - fail_fast: get_tool_config(manifest_dir).glob_fail_fast(), + fail_fast: get_tool_config(workspace_dir).glob_fail_fast(), }); // step 1: collect all matching files diff --git a/insta/src/macros.rs b/insta/src/macros.rs index ba837e40..fd453acb 100644 --- a/insta/src/macros.rs +++ b/insta/src/macros.rs @@ -15,6 +15,19 @@ macro_rules! _function_name { }}; } +#[doc(hidden)] +#[macro_export] +macro_rules! _get_workspace_root { + () => {{ + use std::env; + + // Note the `env!("CARGO_MANIFEST_DIR")` needs to be in the macro (in + // contrast to a function in insta) because the macro needs to capture + // the value in the caller library, an exclusive property of macros. + $crate::_macro_support::get_cargo_workspace(env!("CARGO_MANIFEST_DIR")) + }}; +} + /// Asserts a `Serialize` snapshot in CSV format. /// /// **Feature:** `csv` (disabled by default) @@ -349,7 +362,7 @@ macro_rules! _assert_snapshot_base { $name.into(), #[allow(clippy::redundant_closure_call)] &$transform(&$value), - env!("CARGO_MANIFEST_DIR"), + $crate::_get_workspace_root!().as_path(), $crate::_function_name!(), module_path!(), file!(), @@ -485,9 +498,13 @@ macro_rules! with_settings { #[cfg_attr(docsrs, doc(cfg(feature = "glob")))] #[macro_export] macro_rules! glob { + // TODO: I think we could remove the three-argument version of this macro + // and just support a pattern such as + // `glob!("../test_data/inputs/*.txt"...`. ($base_path:expr, $glob:expr, $closure:expr) => {{ use std::path::Path; - let base = $crate::_macro_support::get_cargo_workspace(env!("CARGO_MANIFEST_DIR")) + + let base = $crate::_get_workspace_root!() .join(Path::new(file!()).parent().unwrap()) .join($base_path) .to_path_buf(); @@ -495,7 +512,12 @@ macro_rules! glob { // we try to canonicalize but on some platforms (eg: wasm) that might not work, so // we instead silently fall back. let base = base.canonicalize().unwrap_or_else(|_| base); - $crate::_macro_support::glob_exec(env!("CARGO_MANIFEST_DIR"), &base, $glob, $closure); + $crate::_macro_support::glob_exec( + $crate::_get_workspace_root!().as_path(), + &base, + $glob, + $closure, + ); }}; ($glob:expr, $closure:expr) => {{ diff --git a/insta/src/runtime.rs b/insta/src/runtime.rs index c2c363ed..4b5eba60 100644 --- a/insta/src/runtime.rs +++ b/insta/src/runtime.rs @@ -8,14 +8,14 @@ use std::str; use std::sync::{Arc, Mutex}; use std::{borrow::Cow, env}; -use crate::output::SnapshotPrinter; use crate::settings::Settings; use crate::snapshot::{MetaData, PendingInlineSnapshot, Snapshot, SnapshotContents}; use crate::utils::{path_to_storage, style}; +use crate::{env::get_tool_config, output::SnapshotPrinter}; use crate::{ env::{ - get_cargo_workspace, get_tool_config, memoize_snapshot_file, snapshot_update_behavior, - OutputBehavior, SnapshotUpdateBehavior, ToolConfig, + memoize_snapshot_file, snapshot_update_behavior, OutputBehavior, SnapshotUpdateBehavior, + ToolConfig, }, snapshot::SnapshotKind, }; @@ -218,7 +218,7 @@ fn get_snapshot_filename( #[derive(Debug)] struct SnapshotAssertionContext<'a> { tool_config: Arc, - cargo_workspace: Arc, + workspace: &'a Path, module_path: &'a str, snapshot_name: Option>, snapshot_file: Option, @@ -233,14 +233,13 @@ struct SnapshotAssertionContext<'a> { impl<'a> SnapshotAssertionContext<'a> { fn prepare( refval: ReferenceValue<'a>, - manifest_dir: &'a str, + workspace: &'a Path, function_name: &'a str, module_path: &'a str, assertion_file: &'a str, assertion_line: u32, ) -> Result, Box> { - let tool_config = get_tool_config(manifest_dir); - let cargo_workspace = get_cargo_workspace(manifest_dir); + let tool_config = get_tool_config(workspace); let snapshot_name; let mut duplication_key = None; let mut snapshot_file = None; @@ -268,7 +267,7 @@ impl<'a> SnapshotAssertionContext<'a> { module_path, assertion_file, &name, - &cargo_workspace, + workspace, assertion_file, is_doctest, ); @@ -290,7 +289,7 @@ impl<'a> SnapshotAssertionContext<'a> { snapshot_name = detect_snapshot_name(function_name, module_path) .ok() .map(Cow::Owned); - let mut pending_file = cargo_workspace.join(assertion_file); + let mut pending_file = workspace.join(assertion_file); pending_file.set_file_name(format!( ".{}.pending-snap", pending_file @@ -311,7 +310,7 @@ impl<'a> SnapshotAssertionContext<'a> { Ok(SnapshotAssertionContext { tool_config, - cargo_workspace, + workspace, module_path, snapshot_name, snapshot_file, @@ -326,8 +325,8 @@ impl<'a> SnapshotAssertionContext<'a> { /// Given a path returns the local path within the workspace. pub fn localize_path(&self, p: &Path) -> Option { - let workspace = self.cargo_workspace.canonicalize().ok()?; - let p = self.cargo_workspace.join(p).canonicalize().ok()?; + let workspace = self.workspace.canonicalize().ok()?; + let p = self.workspace.join(p).canonicalize().ok()?; p.strip_prefix(&workspace).ok().map(|x| x.to_path_buf()) } @@ -454,11 +453,8 @@ impl<'a> SnapshotAssertionContext<'a> { /// This prints the information about the snapshot fn print_snapshot_info(&self, new_snapshot: &Snapshot) { - let mut printer = SnapshotPrinter::new( - self.cargo_workspace.as_path(), - self.old_snapshot.as_ref(), - new_snapshot, - ); + let mut printer = + SnapshotPrinter::new(self.workspace, self.old_snapshot.as_ref(), new_snapshot); printer.set_line(Some(self.assertion_line)); printer.set_snapshot_file(self.snapshot_file.as_deref()); printer.set_title(Some("Snapshot Summary")); @@ -572,8 +568,7 @@ fn record_snapshot_duplicate( if let Some(prev_snapshot) = results.get(key) { if prev_snapshot.contents() != snapshot.contents() { println!("Snapshots in allow-duplicates block do not match."); - let mut printer = - SnapshotPrinter::new(ctx.cargo_workspace.as_path(), Some(prev_snapshot), snapshot); + let mut printer = SnapshotPrinter::new(ctx.workspace, Some(prev_snapshot), snapshot); printer.set_line(Some(ctx.assertion_line)); printer.set_snapshot_file(ctx.snapshot_file.as_deref()); printer.set_title(Some("Differences in Block")); @@ -623,7 +618,7 @@ where pub fn assert_snapshot( refval: ReferenceValue, new_snapshot_value: &str, - manifest_dir: &str, + workspace: &Path, function_name: &str, module_path: &str, assertion_file: &str, @@ -632,7 +627,7 @@ pub fn assert_snapshot( ) -> Result<(), Box> { let ctx = SnapshotAssertionContext::prepare( refval, - manifest_dir, + workspace, function_name, module_path, assertion_file,