Self-tests can run from a simple string (#439)

Many of the tests are concerned with what mutations are generated from
some source code, given some options. For many of these we don't really
need a source tree on disk and certainly not to run cargo: it's enough
just to have the one file we're mutating.

This adds a new API making just the mutation code accessible to unit
tests.

This ought to make tests faster to run, and potentially easier to write
because they can be more self contained.

Fixes #433

DESIGN.md

@@ -190,7 +190,16 @@ Therefore when running `rustc` we configure all warnings off, with `--cap-lints`
 
 ## Testing
 
-Cargo-mutants is primarily tested on its public interface, which is the command line. These tests live in `tests/cli` and generally have the form of:
+Testing cargo-mutants is of course important: both so that it works reliably and also so that it's a good example of a well-tested project that should have few missed mutants.
+
+### integration and unit tests
+
+The tests have a combination of:
+
+1. Rust "unit" tests run in-process, declared in `mod test{}` within various files in `src`.
+2. Rust "integration" tests in the `tests` directory that actually run `cargo-mutants` as a subprocess and that test its overall behavior.
+
+The integration tests are in some sense more realistic because they drive the same interface as real users, but they will also typically be slower because they spawn subprocesses. As a result, my general idea is to have at least basic coverage of each feature with an integration test, and then to fill in the other cases with unit tests.
 
 1. Make a copy of a `testdata` tree, so that it's not accidentally modified.
 2. Run a `cargo-mutants` command on it.
@@ -212,29 +221,25 @@ To manage test time:
 
 ### `testdata` trees
 
-The primary means of testing is Rust source trees under `testdata`: you can copy an existing tree and modify it to show the new behavior that you want to test.
+Many tests run against trees under `testdata`.
 
-A selection of test trees are available for testing different scenarios. If there is an existing suitable tree, please use it. If you need to test a situation that is not covered yet, please add a new tree.
+These have been "disarmed" by renaming `Cargo.toml` to `Cargo_test.toml`, so `cargo` won't normally run them, or see them as part of the main workspace. (See <https://github.com/sourcefrog/cargo-mutants/issues/355> for context.)
+
+Tests should always run against a copy of these trees using `copy_of_testdata`, to make sure their work has no side effects on the main tree.
 
 Please describe the purpose of the testdata tree inside the tree, either in `Cargo.toml` or a `README.md` file.
 
 To make a new tree you can copy an existing tree, but make sure to change the package name in its `Cargo.toml`.
 
-All the trees need to be excluded from the overall workspace in the top-level `Cargo.toml`.
-
 ### `--list` tests
 
 There is a general test that runs `cargo mutants --list` on each tree and compares the result to an Insta snapshot, for both the text and json output formats.
 
 Many features can be tested adequately by only looking at the list of mutants produced, with no need to actually test the mutants. In this case the generic list tests might be enough.
 
-### Unit tests
-
-Although we primarily want to test the public interface (which is the command line), unit tests can be added in a `mod test {}` within the source tree for any behavior that is inconvenient or overly slow to exercise from the command line.
-
 ### Nextest tests
 
-cargo-mutants tests require `nextest` to be installed.
+cargo-mutants tests require `nextest` to be installed so that we can test the integration.
 
 ## UI Style
 

src/mutate.rs

@@ -237,8 +237,9 @@ mod test {
     use indoc::indoc;
     use itertools::Itertools;
     use pretty_assertions::assert_eq;
-    use test_util::copy_of_testdata;
 
+    use crate::test_util::copy_of_testdata;
+    use crate::visit::mutate_source_str;
     use crate::*;
 
     #[test]
@@ -326,6 +327,23 @@ mod test {
         );
     }
 
+    #[test]
+    fn always_skip_constructors_called_new() {
+        let code = indoc! { r#"
+            struct S {
+                x: i32,
+            }
+
+            impl S {
+                fn new(x: i32) -> Self {
+                    Self { x }
+                }
+            }
+        "# };
+        let mutants = mutate_source_str(code, &Options::default()).unwrap();
+        assert_eq!(mutants, []);
+    }
+
     #[test]
     fn mutate_factorial() -> Result<()> {
         let temp = copy_of_testdata("factorial");

src/options.rs

@@ -11,6 +11,7 @@ use globset::GlobSet;
 use regex::RegexSet;
 use serde::Deserialize;
 use strum::{Display, EnumString};
+use syn::Expr;
 use tracing::warn;
 
 use crate::config::Config;
@@ -307,6 +308,17 @@ impl Options {
             &[Phase::Build, Phase::Test]
         }
     }
+
+    /// Return the syn ASTs for the error values, which should be inserted as return values
+    /// from functions returning `Result`.
+    pub(crate) fn parsed_error_exprs(&self) -> Result<Vec<Expr>> {
+        self.error_values
+            .iter()
+            .map(|e| {
+                syn::parse_str(e).with_context(|| format!("Failed to parse error value {e:?}"))
+            })
+            .collect()
+    }
 }
 
 /// If the first slices is non-empty, return that, otherwise the second.

src/source.rs

@@ -2,6 +2,7 @@
 
 //! Access to a Rust source tree and files.
 
+use std::fs::read_to_string;
 use std::sync::Arc;
 
 use anyhow::{Context, Result};
@@ -42,33 +43,52 @@ impl SourceFile {
     /// Construct a SourceFile representing a file within a tree.
     ///
     /// This eagerly loads the text of the file.
-    pub fn new(
+    ///
+    /// This also skip files outside of the tree, returning `Ok(None)`.
+    pub fn load(
         tree_path: &Utf8Path,
-        tree_relative_path: Utf8PathBuf,
+        tree_relative_path: &Utf8Path,
         package_name: &str,
         is_top: bool,
     ) -> Result<Option<SourceFile>> {
-        if ascent(&tree_relative_path) > 0 {
+        // TODO: Perhaps the caller should be responsible for checking this?
+        if ascent(tree_relative_path) > 0 {
             warn!(
                 "skipping source outside of tree: {:?}",
                 tree_relative_path.to_slash_path()
             );
             return Ok(None);
         }
-        let full_path = tree_path.join(&tree_relative_path);
+        let full_path = tree_path.join(tree_relative_path);
         let code = Arc::new(
-            std::fs::read_to_string(&full_path)
+            read_to_string(&full_path)
                 .with_context(|| format!("failed to read source of {full_path:?}"))?
                 .replace("\r\n", "\n"),
         );
         Ok(Some(SourceFile {
-            tree_relative_path,
+            tree_relative_path: tree_relative_path.to_owned(),
             code,
             package_name: package_name.to_owned(),
             is_top,
         }))
     }
 
+    /// Construct from in-memory text.
+    #[cfg(test)]
+    pub fn from_str(
+        tree_relative_path: &Utf8Path,
+        code: &str,
+        package_name: &str,
+        is_top: bool,
+    ) -> SourceFile {
+        SourceFile {
+            tree_relative_path: tree_relative_path.to_owned(),
+            code: Arc::new(code.to_owned()),
+            package_name: package_name.to_owned(),
+            is_top,
+        }
+    }
+
     /// Return the path of this file relative to the tree root, with forward slashes.
     pub fn tree_relative_slashes(&self) -> String {
         self.tree_relative_path.to_slash_path()
@@ -109,9 +129,9 @@ mod test {
             .write_all(b"fn main() {\r\n    640 << 10;\r\n}\r\n")
             .unwrap();
 
-        let source_file = SourceFile::new(
+        let source_file = SourceFile::load(
             temp_dir_path,
-            file_name.parse().unwrap(),
+            Utf8Path::new(file_name),
             "imaginary-package",
             true,
         )
@@ -122,9 +142,9 @@ mod test {
 
     #[test]
     fn skips_files_outside_of_workspace() {
-        let source_file = SourceFile::new(
-            &Utf8PathBuf::from("unimportant"),
-            "../outside_workspace.rs".parse().unwrap(),
+        let source_file = SourceFile::load(
+            Utf8Path::new("unimportant"),
+            Utf8Path::new("../outside_workspace.rs"),
             "imaginary-package",
             true,
         )

src/visit.rs

@@ -58,11 +58,7 @@ pub fn walk_tree(
     options: &Options,
     console: &Console,
 ) -> Result<Discovered> {
-    let error_exprs = options
-        .error_values
-        .iter()
-        .map(|e| syn::parse_str(e).with_context(|| format!("Failed to parse error value {e:?}")))
-        .collect::<Result<Vec<Expr>>>()?;
+    let error_exprs = options.parsed_error_exprs()?;
     console.walk_tree_start();
     let mut file_queue: VecDeque<SourceFile> = top_source_files.iter().cloned().collect();
     let mut mutants = Vec::new();
@@ -77,9 +73,9 @@ pub fn walk_tree(
         // `--list-files`.
         for mod_namespace in &external_mods {
             if let Some(mod_path) = find_mod_source(workspace_dir, &source_file, mod_namespace)? {
-                file_queue.extend(SourceFile::new(
+                file_queue.extend(SourceFile::load(
                     workspace_dir,
-                    mod_path,
+                    &mod_path,
                     &source_file.package_name,
                     false,
                 )?)
@@ -117,7 +113,7 @@ pub fn walk_tree(
 fn walk_file(
     source_file: &SourceFile,
     error_exprs: &[Expr],
-) -> Result<(Vec<Mutant>, Vec<Vec<ModNamespace>>)> {
+) -> Result<(Vec<Mutant>, Vec<ExternalModRef>)> {
     let _span = debug_span!("source_file", path = source_file.tree_relative_slashes()).entered();
     debug!("visit source file");
     let syn_file = syn::parse_str::<syn::File>(source_file.code())
@@ -135,6 +131,32 @@ fn walk_file(
     Ok((visitor.mutants, visitor.external_mods))
 }
 
+/// For testing: parse and generate mutants from one single file provided as a string.
+///
+/// The source code is assumed to be named `src/main.rs` with a fixed package name.
+#[cfg(test)]
+pub fn mutate_source_str(code: &str, options: &Options) -> Result<Vec<Mutant>> {
+    let source_file = SourceFile::from_str(
+        Utf8Path::new("src/main.rs"),
+        code,
+        "cargo-mutants-testdata-internal",
+        true,
+    );
+    let (mutants, _) = walk_file(&source_file, &options.parsed_error_exprs()?)?;
+    Ok(mutants)
+}
+
+/// Reference to an external module from a source file.
+///
+/// This is approximately a list of namespace components like `["foo", "bar"]` for
+/// `foo::bar`, but each may also be decorated with a `#[path="..."]` attribute,
+/// and they're attributed to a location in the source.
+#[derive(Clone, Debug, PartialEq, Eq)]
+struct ExternalModRef {
+    /// Namespace components of the module path
+    parts: Vec<ModNamespace>,
+}
+
 /// Namespace for a module defined in a `mod foo { ... }` block or `mod foo;` statement
 ///
 /// In the context of resolving modules, a module "path" (and to some extent "name") is ambiguous:
@@ -154,6 +176,7 @@ struct ModNamespace {
     /// Location of the module definition in the source file
     source_location: Span,
 }
+
 impl ModNamespace {
     /// Returns the name of the module for filesystem purposes
     fn get_filesystem_name(&self) -> &Utf8Path {
@@ -200,7 +223,7 @@ struct DiscoveryVisitor<'o> {
 
     /// The names from `mod foo;` statements that should be visited later,
     /// namespaced relative to the source file
-    external_mods: Vec<Vec<ModNamespace>>,
+    external_mods: Vec<ExternalModRef>,
 
     /// Parsed error expressions, from the config file or command line.
     error_exprs: &'o [Expr],
@@ -436,7 +459,9 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> {
         if node.content.is_none() {
             // If we're already inside `mod a { ... }` and see `mod b;` then
             // remember [a, b] as an external module to visit later.
-            self.external_mods.push(self.mod_namespace_stack.clone());
+            self.external_mods.push(ExternalModRef {
+                parts: self.mod_namespace_stack.clone(),
+            });
         }
         self.in_namespace(&mod_namespace.name, |v| syn::visit::visit_item_mod(v, node));
         assert_eq!(self.mod_namespace_stack.pop(), Some(mod_namespace));
@@ -534,7 +559,7 @@ fn function_body_span(block: &Block) -> Option<Span> {
 fn find_mod_source(
     tree_root: &Utf8Path,
     parent: &SourceFile,
-    mod_namespace: &[ModNamespace],
+    mod_namespace: &ExternalModRef,
 ) -> Result<Option<Utf8PathBuf>> {
     // First, work out whether the mod will be a sibling in the same directory, or
     // in a child directory.
@@ -562,7 +587,10 @@ fn find_mod_source(
     // Having determined the right directory then we can follow the path attribute, or
     // if no path is specified, then look for either `foo.rs` or `foo/mod.rs`.
 
-    let (mod_child, mod_parents) = mod_namespace.split_last().expect("mod namespace is empty");
+    let (mod_child, mod_parents) = mod_namespace
+        .parts
+        .split_last()
+        .expect("mod namespace is empty");
 
     // TODO: Beyond #115, we should probably remove all special handling of
     // `mod.rs` here by remembering how we found this file, and whether it
@@ -836,4 +864,21 @@ mod test {
             Err("/leading_slash/../and_dots.rs".to_owned())
         );
     }
+
+    /// Demonstrate that we can generate mutants from a string, without needing a whole tree.
+    #[test]
+    fn mutants_from_test_str() {
+        let options = Options::default();
+        let mutants = mutate_source_str(
+            indoc! {"
+                fn always_true() -> bool { true }
+            "},
+            &options,
+        )
+        .expect("walk_file_string");
+        assert_eq!(
+            mutants.iter().map(|m| m.name(false, false)).collect_vec(),
+            ["src/main.rs: replace always_true -> bool with false"]
+        );
+    }
 }

src/workspace.rs

@@ -259,12 +259,9 @@ fn packages_from_metadata(metadata: &Metadata) -> Result<Vec<Package>> {
 /// Find all the top source files for selected packages.
 fn top_sources(root: &Utf8Path, packages: &[Package]) -> Result<Vec<SourceFile>> {
     let mut sources = Vec::new();
-    for Package {
-        name, top_sources, ..
-    } in packages
-    {
-        for source_path in top_sources {
-            sources.extend(SourceFile::new(root, source_path.to_owned(), name, true)?);
+    for package in packages {
+        for source_path in &package.top_sources {
+            sources.extend(SourceFile::load(root, source_path, &package.name, true)?);
         }
     }
     Ok(sources)