Fix finding the workspace root from a subdirectory (#145)

NEWS.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+- Fixed a bug causing an assertion failure when cargo-mutants was run from a
+  subdirectory of a workspace. Thanks to Adam Chalmers!
+
 - Generate `HttpResponse::Ok().finish()` as a mutation of an Actix `HttpResponse`.
 
 ## 23.6.0

book/src/workspaces.md

@@ -2,6 +2,14 @@
 
 cargo-mutants supports testing Cargo workspaces that contain multiple packages. The entire workspace tree is copied.
 
-All source files in all packages in the workspace are tested.
+By default, all source files in all packages in the workspace are tested.
 
-For each mutant, only the containing package's tests are run, on the theory that each package's tests are responsible for testing the package's code.
+**NOTE: This behavior might not be the best choice, and this may change in future.**
+
+You can use the `--file` options to restrict cargo-mutants to testing only files
+from some subdirectory, e.g. with `-f "utils/**/*.rs"`. (Remember to quote globs
+on the command line, so that the shell doesn't expand them.) You can use `--list` or
+`--list-files` to preview the effect of filters.
+
+For each mutant, only the containing package's tests are run, on the theory that
+each package's tests are responsible for testing the package's code.

src/cargo.rs

@@ -5,8 +5,7 @@
 use std::env;
 use std::sync::Arc;
 
-#[allow(unused_imports)]
-use anyhow::{anyhow, bail, Context, Result};
+use anyhow::{anyhow, ensure, Context, Result};
 use camino::{Utf8Path, Utf8PathBuf};
 use serde_json::Value;
 use tracing::debug_span;
@@ -44,12 +43,31 @@ impl Tool for CargoTool {
     }
 
     fn find_root(&self, path: &Utf8Path) -> Result<Utf8PathBuf> {
-        let cargo_toml_path = locate_cargo_toml(path)?;
+        ensure!(path.is_dir(), "{path:?} is not a directory");
+        let cargo_bin = cargo_bin(); // needed for lifetime
+        let argv: Vec<&str> = vec![&cargo_bin, "locate-project", "--workspace"];
+        let stdout = get_command_output(&argv, path)
+            .with_context(|| format!("run cargo locate-project in {path:?}"))?;
+        let val: Value =
+            serde_json::from_str(&stdout).context("parse cargo locate-project output")?;
+        let cargo_toml_path: Utf8PathBuf = val["root"]
+            .as_str()
+            .with_context(|| format!("cargo locate-project output has no root: {stdout:?}"))?
+            .to_owned()
+            .into();
+        debug!(?cargo_toml_path, "Found workspace root manifest");
+        ensure!(
+            cargo_toml_path.is_file(),
+            "cargo locate-project root {cargo_toml_path:?} is not a file"
+        );
         let root = cargo_toml_path
             .parent()
-            .expect("cargo_toml_path has a parent")
+            .ok_or_else(|| anyhow!("cargo locate-project root {cargo_toml_path:?} has no parent"))?
             .to_owned();
-        assert!(root.is_dir());
+        ensure!(
+            root.is_dir(),
+            "apparent project root directory {root:?} is not a directory"
+        );
         Ok(root)
     }
 
@@ -65,9 +83,9 @@ impl Tool for CargoTool {
     /// After this, there is one more level of discovery, by walking those root files
     /// to find `mod` statements, and then recursively walking those files to find
     /// all source files.
-    fn root_files(&self, source_root_path: &Utf8Path) -> Result<Vec<Arc<SourceFile>>> {
+    fn top_source_files(&self, source_root_path: &Utf8Path) -> Result<Vec<Arc<SourceFile>>> {
         let cargo_toml_path = source_root_path.join("Cargo.toml");
-        debug!(?cargo_toml_path, ?source_root_path, "find root files");
+        debug!(?cargo_toml_path, ?source_root_path, "Find root files");
         check_interrupted()?;
         let metadata = cargo_metadata::MetadataCommand::new()
             .manifest_path(&cargo_toml_path)
@@ -78,11 +96,16 @@ impl Tool for CargoTool {
         for package_metadata in &metadata.workspace_packages() {
             check_interrupted()?;
             let _span = debug_span!("package", name = %package_metadata.name).entered();
-            debug!(manifest_path = %package_metadata.manifest_path, "walk package");
-            let relative_manifest_path = package_metadata
-                .manifest_path
+            let manifest_path = &package_metadata.manifest_path;
+            debug!(%manifest_path, "walk package");
+            let relative_manifest_path = manifest_path
                 .strip_prefix(source_root_path)
-                .expect("manifest_path is within source_root_path")
+                .map_err(|_| {
+                    anyhow!(
+                        "manifest path {manifest_path:?} for package {name:?} is not within the detected source root path {source_root_path:?}",
+                        name = package_metadata.name
+                    )
+                })?
                 .to_owned();
             let package = Arc::new(Package {
                 name: package_metadata.name.clone(),
@@ -192,25 +215,6 @@ fn rustflags() -> String {
     rustflags.join("\x1f")
 }
 
-/// Run `cargo locate-project` to find the path of the `Cargo.toml` enclosing this path.
-fn locate_cargo_toml(path: &Utf8Path) -> Result<Utf8PathBuf> {
-    let cargo_bin = cargo_bin();
-    if !path.is_dir() {
-        bail!("{} is not a directory", path);
-    }
-    let argv: Vec<&str> = vec![&cargo_bin, "locate-project"];
-    let stdout = get_command_output(&argv, path)
-        .with_context(|| format!("run cargo locate-project in {path:?}"))?;
-    let val: Value = serde_json::from_str(&stdout).context("parse cargo locate-project output")?;
-    let cargo_toml_path: Utf8PathBuf = val["root"]
-        .as_str()
-        .context("cargo locate-project output has no root: {stdout:?}")?
-        .to_owned()
-        .into();
-    assert!(cargo_toml_path.is_file());
-    Ok(cargo_toml_path)
-}
-
 /// Find all the files that are named in the `path` of targets in a Cargo manifest that should be tested.
 ///
 /// These are the starting points for discovering source files.
@@ -252,6 +256,7 @@ fn should_mutate_target(target: &cargo_metadata::Target) -> bool {
 mod test {
     use std::ffi::OsStr;
 
+    use itertools::Itertools;
     use pretty_assertions::assert_eq;
 
     use crate::{Options, Phase};
@@ -365,4 +370,32 @@ mod test {
         assert!(root.join("src/bin/factorial.rs").is_file());
         assert_eq!(root.file_name().unwrap(), OsStr::new("factorial"));
     }
+
+    #[test]
+    fn find_root_from_subdirectory_of_workspace_finds_the_workspace_root() {
+        let root = CargoTool::new()
+            .find_root(Utf8Path::new("testdata/tree/workspace/main"))
+            .expect("Find root from within workspace/main");
+        assert_eq!(root.file_name(), Some("workspace"), "Wrong root: {root:?}");
+    }
+
+    #[test]
+    fn find_top_source_files_from_subdirectory_of_workspace() {
+        let tool = CargoTool::new();
+        let root_dir = tool
+            .find_root(Utf8Path::new("testdata/tree/workspace/main"))
+            .expect("Find workspace root");
+        let top_source_files = tool.top_source_files(&root_dir).expect("Find root files");
+        println!("{top_source_files:#?}");
+        assert_eq!(top_source_files.len(), 3);
+        let paths: Vec<String> = top_source_files
+            .iter()
+            .map(|sf| sf.tree_relative_path.to_string())
+            .sorted()
+            .collect_vec();
+        assert_eq!(
+            paths,
+            ["main/src/main.rs", "main2/src/main.rs", "utils/src/lib.rs"]
+        );
+    }
 }

src/main.rs

@@ -270,7 +270,8 @@ fn list_files(tool: &dyn Tool, source: &Utf8Path, options: &Options, json: bool)
                 })
                 .collect(),
         );
-        serde_json::to_writer_pretty(out, &json_list)?;
+        serde_json::to_writer_pretty(&mut out, &json_list)?;
+        writeln!(out)?;
     } else {
         for file in discovered.files {
             writeln!(out, "{}", file.tree_relative_path)?;

src/tool.rs

@@ -21,19 +21,28 @@ use crate::SourceFile;
 use crate::{build_dir, Result};
 
 pub trait Tool: Debug + Send + Sync {
+    /// A short name for this tool, like "cargo".
     fn name(&self) -> &str;
 
-    /// Find the root of the package enclosing a given path.
+    /// Find the root of the source tree enclosing a given path.
     ///
     /// The root is the enclosing directory that needs to be copied to make a self-contained
     /// scratch directory, and from where source discovery begins.
+    ///
+    /// This may include more directories than will actually be tested, sufficient to allow
+    /// the build to work. For Cargo, we copy the whole workspace.
     fn find_root(&self, path: &Utf8Path) -> Result<Utf8PathBuf>;
 
-    /// Find all the root files from whence source discovery should begin.
+    /// Find the top-level files for each package within a tree.
+    ///
+    /// The path is the root returned by [find_root].
     ///
     /// For Cargo, this is files like `src/bin/*.rs`, `src/lib.rs` identified by targets
-    /// in the manifest.
-    fn root_files(&self, path: &Utf8Path) -> Result<Vec<Arc<SourceFile>>>;
+    /// in the manifest for each package.
+    ///
+    /// From each of these top files, we can discover more source by following `mod`
+    /// statements.
+    fn top_source_files(&self, path: &Utf8Path) -> Result<Vec<Arc<SourceFile>>>;
 
     /// Compose argv to run one phase in this tool.
     fn compose_argv(

src/visit.rs

@@ -47,7 +47,7 @@ pub fn walk_tree(tool: &dyn Tool, root: &Utf8Path, options: &Options) -> Result<
         .collect::<Result<Vec<Expr>>>()?;
     let mut mutants = Vec::new();
     let mut files: Vec<Arc<SourceFile>> = Vec::new();
-    let mut file_queue: VecDeque<Arc<SourceFile>> = tool.root_files(root)?.into();
+    let mut file_queue: VecDeque<Arc<SourceFile>> = tool.top_source_files(root)?.into();
     while let Some(source_file) = file_queue.pop_front() {
         check_interrupted()?;
         let (mut file_mutants, more_files) =

tests/cli/main.rs

@@ -457,6 +457,30 @@ fn list_files_json_workspace() {
         .assert_insta("list_files_json_workspace");
 }
 
+#[test]
+fn list_files_as_json_in_workspace_subdir() {
+    run()
+        .args(["mutants", "--list-files", "--json"])
+        .current_dir("testdata/tree/workspace/main2")
+        .assert()
+        .stdout(indoc! {r#"
+            [
+              {
+                "package": "cargo_mutants_testdata_workspace_utils",
+                "path": "utils/src/lib.rs"
+              },
+              {
+                "package": "main",
+                "path": "main/src/main.rs"
+              },
+              {
+                "package": "main2",
+                "path": "main2/src/main.rs"
+              }
+            ]
+        "#});
+}
+
 #[test]
 fn workspace_tree_is_well_tested() {
     let tmp_src_dir = copy_of_testdata("workspace");