Skip to content

Commit

Permalink
Consolidate onto INSTA_WORKSPACE_ROOT
Browse files Browse the repository at this point in the history
Now a single place where `INSTA_WORKSPACE_ROOT` & `CARGO_MANIFEST_DIR` are checked, with priority to the former.

Fixes mitsuhiko#575

Could do with an integration test
  • Loading branch information
max-sixty committed Sep 25, 2024
1 parent 8753f2c commit ea2fe51
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 37 deletions.
49 changes: 19 additions & 30 deletions insta/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub struct ToolConfig {
impl ToolConfig {
/// Loads the tool config for a specific manifest.
pub fn from_manifest_dir(manifest_dir: &str) -> Result<ToolConfig, Error> {
ToolConfig::from_workspace(&get_cargo_workspace(manifest_dir))
Self::from_workspace(&get_cargo_workspace(manifest_dir))
}

/// Loads the tool config from a cargo workspace.
Expand Down Expand Up @@ -416,35 +416,24 @@ pub fn get_cargo_workspace(manifest_dir: &str) -> Arc<PathBuf> {
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 {
let output = std::process::Command::new(
env::var("CARGO")
.ok()
.unwrap_or_else(|| "cargo".to_string()),
)
.arg("metadata")
.arg("--format-version=1")
.arg("--no-deps")
.current_dir(manifest_dir)
.output()
.unwrap();
let docs = 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)
};
let output = std::process::Command::new(
env::var("CARGO")
.ok()
.unwrap_or_else(|| "cargo".to_string()),
)
.arg("metadata")
.arg("--format-version=1")
.arg("--no-deps")
.current_dir(manifest_dir)
.output()
.unwrap();
let docs = 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());
let path = Arc::new(workspace_root);
workspaces.insert(manifest_dir.to_string(), path.clone());
path
}
Expand Down
24 changes: 21 additions & 3 deletions insta/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,24 @@ macro_rules! _function_name {
}};
}

// 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.
#[doc(hidden)]
#[macro_export]
macro_rules! _get_workspace_root {
() => {{
use std::env;

if let Ok(workspace_root) = env::var("INSTA_WORKSPACE_ROOT") {
workspace_root.to_string()
} else {
env!("CARGO_MANIFEST_DIR").to_string()
}
}};
}

/// Asserts a `Serialize` snapshot in CSV format.
///
/// **Feature:** `csv` (disabled by default)
Expand Down Expand Up @@ -349,7 +367,7 @@ macro_rules! _assert_snapshot_base {
$name.into(),
#[allow(clippy::redundant_closure_call)]
&$transform(&$value),
env!("CARGO_MANIFEST_DIR"),
&$crate::_get_workspace_root!(),
$crate::_function_name!(),
module_path!(),
file!(),
Expand Down Expand Up @@ -487,15 +505,15 @@ macro_rules! with_settings {
macro_rules! glob {
($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::_macro_support::get_cargo_workspace(&$crate::_get_workspace_root!())
.join(Path::new(file!()).parent().unwrap())
.join($base_path)
.to_path_buf();

// 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!(), &base, $glob, $closure);
}};

($glob:expr, $closure:expr) => {{
Expand Down
6 changes: 2 additions & 4 deletions insta/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,8 +639,6 @@ pub fn assert_snapshot(
assertion_line,
)?;

let tool_config = get_tool_config(manifest_dir);

// apply filters if they are available
#[cfg(feature = "filters")]
let new_snapshot_value =
Expand Down Expand Up @@ -671,7 +669,7 @@ pub fn assert_snapshot(
.old_snapshot
.as_ref()
.map(|x| {
if tool_config.require_full_match() {
if ctx.tool_config.require_full_match() {
x.matches_fully(&new_snapshot)
} else {
x.matches(&new_snapshot)
Expand All @@ -683,7 +681,7 @@ pub fn assert_snapshot(
ctx.cleanup_passing()?;

if matches!(
tool_config.snapshot_update(),
ctx.tool_config.snapshot_update(),
crate::env::SnapshotUpdate::Force
) {
// Avoid creating new files if contents match exactly. In
Expand Down

0 comments on commit ea2fe51

Please sign in to comment.