Repository: mitsuhiko/insta (rust)

Issue:
Respect INSTA_WORKSPACE_ROOT at compile time
## Problem description

I'm using insta in a rust project that we build using [Yocto](https://www.yoctoproject.org/).

Yocto does some strict checks on binaries to ensure reproducibility and pollution by the host system. So, it will produce warnings if the final binary contains references to the build directory of the host.

In my case also build and package all our tests as binaries in order to run them on a target device. This works and I set the `INSTA_WORKSPACE_ROOT` variable to the correct location on the target file system.

Now the problem I'm facing is that the binary still contains a reference to the build directory if any of the `insta::assert_*` macros is used.

I think this i because the macro by default uses a reference to `CARGO_MANIFEST_DIR` in https://github.com/mitsuhiko/insta/blob/master/insta/src/macros.rs#L27

## Proposal

If `INSTA_WORKSPACE_ROOT` is set at compile time use it instead of `CARGO_MANIFEST_DIR` as the default in the macro.
This way the final binary would not contain the reference to the build directory.

Pull request description:
Allow setting INSTA_WORKSPACE_ROOT at compile time
This is useful if you already know the intended target location at compile time. For example when installed as a package in a linux distro.

It also avoids some reproducible build issues by avoiding embedding the value of CARGO_MANIFEST_DIR in the final binary.

Fixes #725 

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 2ce4ca7..d24b36c 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -77,6 +77,7 @@ dependencies = [
  "open",
  "os_pipe",
  "proc-macro2",
+ "rust-strings",
  "semver",
  "serde",
  "serde_json",
@@ -649,6 +650,12 @@ dependencies = [
  "serde",
 ]
 
+[[package]]
+name = "rust-strings"
+version = "0.6.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d17559aaea24b0a7e99482f82273ea3177cea900dcdf78ebef7eef05dfda1476"
+
 [[package]]
 name = "rustc_version"
 version = "0.4.0"
diff --git a/cargo-insta/Cargo.toml b/cargo-insta/Cargo.toml
index 44c7e72..99b6110 100644
--- a/cargo-insta/Cargo.toml
+++ b/cargo-insta/Cargo.toml
@@ -43,3 +43,4 @@ similar = "2.2.1"
 itertools = "0.10.0"
 termcolor = "1.1.2"
 os_pipe = "1.2.1"
+rust-strings = "0.6.0"
diff --git a/insta/src/env.rs b/insta/src/env.rs
index 49fa7f0..fd7e7a2 100644
--- a/insta/src/env.rs
+++ b/insta/src/env.rs
@@ -430,11 +430,19 @@ pub fn snapshot_update_behavior(tool_config: &ToolConfig, unseen: bool) -> Snaps
     }
 }
 
+pub enum Workspace {
+    DetectWithCargo(&'static str),
+    UseAsIs(&'static str),
+}
+
 /// Returns the cargo workspace path for a crate manifest, like
 /// `/Users/janedoe/projects/insta` when passed
 /// `/Users/janedoe/projects/insta/insta/Cargo.toml`.
-pub fn get_cargo_workspace(manifest_dir: &str) -> Arc<PathBuf> {
-    // If INSTA_WORKSPACE_ROOT environment variable is set, use the value as-is.
+///
+/// If `INSTA_WORKSPACE_ROOT` environment variable is set at runtime, use the value as-is.
+/// If `INSTA_WORKSPACE_ROOT` environment variable is set at compile time, use the value as-is.
+/// If `INSTA_WORKSPACE_ROOT` environment variable is not set, use `cargo metadata` to find the workspace root.
+pub fn get_cargo_workspace(workspace: Workspace) -> Arc<PathBuf> {
     // 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.
@@ -442,6 +450,15 @@ pub fn get_cargo_workspace(manifest_dir: &str) -> Arc<PathBuf> {
         return PathBuf::from(workspace_root).into();
     }
 
+    // Distinguish if we need to run `cargo metadata`` or if we can return the workspace
+    // as is.
+    // This is useful if INSTA_WORKSPACE_ROOT was set at compile time, not pointing to
+    // the cargo manifest directory
+    let manifest_dir = match workspace {
+        Workspace::UseAsIs(workspace_root) => return PathBuf::from(workspace_root).into(),
+        Workspace::DetectWithCargo(manifest_dir) => manifest_dir,
+    };
+
     let error_message = || {
         format!(
             "`cargo metadata --format-version=1 --no-deps` in path `{}`",
@@ -492,12 +509,19 @@ pub fn get_cargo_workspace(manifest_dir: &str) -> Arc<PathBuf> {
 }
 
 #[test]
-fn test_get_cargo_workspace() {
-    let workspace = get_cargo_workspace(env!("CARGO_MANIFEST_DIR"));
+fn test_get_cargo_workspace_manifest_dir() {
+    let workspace = get_cargo_workspace(Workspace::DetectWithCargo(env!("CARGO_MANIFEST_DIR")));
     // The absolute path of the workspace, like `/Users/janedoe/projects/insta`
     assert!(workspace.ends_with("insta"));
 }
 
+#[test]
+fn test_get_cargo_workspace_insta_workspace() {
+    let workspace = get_cargo_workspace(Workspace::UseAsIs("/tmp/insta_workspace_root"));
+    // The absolute path of the workspace, like `/tmp/insta_workspace_root`
+    assert!(workspace.ends_with("insta_workspace_root"));
+}
+
 #[cfg(feature = "_cargo_insta_internal")]
 impl std::str::FromStr for TestRunner {
     type Err = ();
diff --git a/insta/src/lib.rs b/insta/src/lib.rs
index 12d4cd6..4cd8baa 100644
--- a/insta/src/lib.rs
+++ b/insta/src/lib.rs
@@ -345,7 +345,7 @@ pub use crate::redaction::{dynamic_redaction, rounded_redaction, sorted_redactio
 #[doc(hidden)]
 pub mod _macro_support {
     pub use crate::content::Content;
-    pub use crate::env::get_cargo_workspace;
+    pub use crate::env::{get_cargo_workspace, Workspace};
     pub use crate::runtime::{
         assert_snapshot, with_allow_duplicates, AutoName, BinarySnapshotValue, InlineValue,
         SnapshotValue,
diff --git a/insta/src/macros.rs b/insta/src/macros.rs
index af1e3fe..574fd8b 100644
--- a/insta/src/macros.rs
+++ b/insta/src/macros.rs
@@ -19,12 +19,20 @@ macro_rules! _function_name {
 #[macro_export]
 macro_rules! _get_workspace_root {
     () => {{
-        use std::env;
+        use std::{env, option_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"))
+        // By default the `CARGO_MANIFEST_DIR` environment variable is used as the workspace root.
+        // If the `INSTA_WORKSPACE_ROOT` environment variable is set at compile time it will override the default.
+        // This can be useful to avoid including local paths in the binary.
+        const WORKSPACE_ROOT: $crate::_macro_support::Workspace = if let Some(root) = option_env!("INSTA_WORKSPACE_ROOT") {
+            $crate::_macro_support::Workspace::UseAsIs(root)
+        } else {
+            $crate::_macro_support::Workspace::DetectWithCargo(env!("CARGO_MANIFEST_DIR"))
+        };
+        $crate::_macro_support::get_cargo_workspace(WORKSPACE_ROOT)
     }};
 }
 


Test patch:
diff --git a/cargo-insta/tests/functional/workspace.rs b/cargo-insta/tests/functional/workspace.rs
index b3976fe..e05c9e0 100644
--- a/cargo-insta/tests/functional/workspace.rs
+++ b/cargo-insta/tests/functional/workspace.rs
@@ -340,6 +340,54 @@ fn test_virtual_manifest_single_crate() {
     "###     );
 }
 
+use std::path::{Path, PathBuf};
+use std::process::Command;
+
+// 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.
+#[allow(dead_code)]
+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")
+}
+
+// This function runs the compiled binary with the given environment and working directory
+#[allow(dead_code)]
+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()
+}
+
+// This function extracts strings from a binary
+// Similar to the `strings` program
+#[allow(dead_code)]
+fn extract_strings(binary_path: &Path) -> Vec<(String, u64)> {
+    let config = rust_strings::FileConfig::new(binary_path).with_min_length(5);
+    rust_strings::strings(&config).expect("Unable to extract strings from binary")
+}
+
 // Can't get the test binary discovery to work on Windows, 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.)
@@ -348,46 +396,12 @@ fn test_virtual_manifest_single_crate() {
 fn test_insta_workspace_root() {
     use std::{
         fs::{self, remove_dir_all},
-        path::{Path, PathBuf},
+        path::PathBuf,
         process::Command,
     };
 
     use crate::TestProject;
 
-    // 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_cargo_toml("insta_workspace_root_test")
         .add_file(
@@ -404,10 +418,11 @@ fn test_snapshot() {
         )
         .create_project();
 
+    // Strip the binary to ensure no references to the workspace in the debug symbols
     let mut cargo_cmd = Command::new("cargo");
     TestProject::clean_env(&mut cargo_cmd);
     let output = cargo_cmd
-        .args(["test", "--no-run"])
+        .args(["test", "--no-run", "--config", "profile.test.strip=true"])
         .current_dir(&test_project.workspace_dir)
         .output()
         .unwrap();
@@ -415,6 +430,17 @@ fn test_snapshot() {
 
     let test_binary_path = find_test_binary(&test_project.workspace_dir);
 
+    let extracted_strings = extract_strings(&test_binary_path);
+
+    assert_eq!(
+        extracted_strings
+            .iter()
+            .filter(|(s, _)| s.contains(test_project.workspace_dir.to_str().unwrap()))
+            .count(),
+        1,
+        "The final doesn't contain only one reference to CARGO_MANIFEST_DIR"
+    );
+
     // Run the test without snapshot (should fail)
     assert!(
         !&run_test_binary(&test_binary_path, &test_project.workspace_dir, None,)
@@ -465,6 +491,137 @@ fn test_snapshot() {
     .success());
 }
 
+// Can't get the test binary discovery to work on Windows, 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_compile_time() {
+    use std::process::Command;
+
+    use crate::TestProject;
+
+    let test_project = TestFiles::new()
+        .add_cargo_toml("insta_workspace_root_test")
+        .add_file(
+            "src/lib.rs",
+            r#"
+use insta::assert_snapshot;
+
+#[test]
+fn test_snapshot() {
+    assert_snapshot!("Hello, world!");
+}
+    "#
+            .to_string(),
+        )
+        .create_project();
+
+    let moved_workspace_compile =
+        tempfile::tempdir().expect("Unable to create temporary test directory");
+
+    // Compile test binary with INSTA_WORKSPACE_ROOT set at compile time
+    // Strip the binary to ensure no references to the workspace in the debug symbols
+    let mut cargo_cmd = Command::new("cargo");
+    TestProject::clean_env(&mut cargo_cmd);
+    let output = cargo_cmd
+        .args(["test", "--no-run", "--config", "profile.test.strip=true"])
+        .env("INSTA_WORKSPACE_ROOT", moved_workspace_compile.path())
+        .current_dir(&test_project.workspace_dir)
+        .output()
+        .unwrap();
+    assert!(&output.status.success());
+
+    let test_binary_path = find_test_binary(&test_project.workspace_dir);
+
+    let extracted_strings = extract_strings(&test_binary_path);
+
+    assert_eq!(
+        extracted_strings
+            .iter()
+            .filter(|(s, _)| s.contains(test_project.workspace_dir.to_str().unwrap()))
+            .count(),
+        0,
+        "The final binary contains a references to CARGO_MANIFEST_DIR"
+    );
+
+    assert_eq!(
+        extracted_strings
+            .iter()
+            .filter(|(s, _)| s.contains(moved_workspace_compile.path().to_str().unwrap()))
+            .count(),
+        1,
+        "The final binary contains not exactly one references to moved workspace"
+    );
+
+    // Run the test without snapshot (should fail)
+    assert!(
+        !&run_test_binary(&test_binary_path, &test_project.workspace_dir, None,)
+            .status
+            .success()
+    );
+
+    // Create the snapshot
+    assert!(&run_test_binary(
+        &test_binary_path,
+        &test_project.workspace_dir,
+        Some(("INSTA_UPDATE", "always")),
+    )
+    .status
+    .success());
+
+    // Verify snapshot creation
+    assert!(moved_workspace_compile
+        .path()
+        .join("src/snapshots")
+        .exists());
+    assert!(moved_workspace_compile
+        .path()
+        .join("src/snapshots/insta_workspace_root_test__snapshot.snap")
+        .exists());
+
+    // Run test in compile time moved workspace without INSTA_WORKSPACE_ROOT (should pass)
+    assert!(
+        &run_test_binary(&test_binary_path, moved_workspace_compile.path(), None)
+            .status
+            .success()
+    );
+
+    // Move the workspace
+    let moved_workspace = {
+        let moved_workspace =
+            tempfile::tempdir().expect("Unable to create temporary test directory");
+        fs::rename(&moved_workspace_compile, &moved_workspace).unwrap();
+        moved_workspace
+    };
+
+    // Verify snapshot moved
+    assert!(moved_workspace.path().join("src/snapshots").exists());
+    assert!(moved_workspace
+        .path()
+        .join("src/snapshots/insta_workspace_root_test__snapshot.snap")
+        .exists());
+
+    // Run test in runtime moved workspace without INSTA_WORKSPACE_ROOT (should fail)
+    assert!(
+        !&run_test_binary(&test_binary_path, moved_workspace.path(), None)
+            .status
+            .success()
+    );
+
+    // Run test in moved workspace with INSTA_WORKSPACE_ROOT (should pass)
+    assert!(&run_test_binary(
+        &test_binary_path,
+        moved_workspace.path(),
+        Some((
+            "INSTA_WORKSPACE_ROOT",
+            moved_workspace.path().to_str().unwrap()
+        )),
+    )
+    .status
+    .success());
+}
+
 /// A cargo target that references a file outside of the project's directory
 /// should still work
 #[test]