Skip to content

Commit

Permalink
Rename factorial/Cargo.toml to Cargo_test.toml
Browse files Browse the repository at this point in the history
Always test against a copy of the testdata.

This should avoid problems with the testdata being trimmed out by `cargo
publish`.
  • Loading branch information
sourcefrog committed Oct 5, 2024
1 parent 2ff12a2 commit 6833360
Show file tree
Hide file tree
Showing 10 changed files with 120 additions and 80 deletions.
7 changes: 4 additions & 3 deletions src/build_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ mod test {
#[test]
fn build_dir_copy_from() {
let workspace = Workspace::open("testdata/factorial").unwrap();
let build_dir = BuildDir::copy_from(&workspace.dir, true, false, &Console::new()).unwrap();
let build_dir =
BuildDir::copy_from(workspace.root(), true, false, &Console::new()).unwrap();
let debug_form = format!("{build_dir:?}");
println!("debug form is {debug_form:?}");
assert!(debug_form.starts_with("BuildDir { path: "));
Expand All @@ -107,12 +108,12 @@ mod test {
#[test]
fn build_dir_in_place() -> Result<()> {
let workspace = Workspace::open("testdata/factorial")?;
let build_dir = BuildDir::in_place(&workspace.dir)?;
let build_dir = BuildDir::in_place(workspace.root())?;
// On Windows e.g. the paths might not have the same form, but they
// should point to the same place.
assert_eq!(
build_dir.path().canonicalize_utf8()?,
workspace.dir.canonicalize_utf8()?
workspace.root().canonicalize_utf8()?
);
Ok(())
}
Expand Down
9 changes: 6 additions & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ mod shard;
mod source;
mod span;
mod tail_file;
#[cfg(test)]
#[path = "../tests/util/mod.rs"]
mod test_util;
mod timeouts;
mod visit;
mod workspace;
Expand Down Expand Up @@ -412,7 +415,7 @@ fn main() -> Result<()> {
let config = if args.no_config {
config::Config::default()
} else {
config::Config::read_tree_config(&workspace.dir)?
config::Config::read_tree_config(workspace.root())?
};
debug!(?config);
debug!(?args.features);
Expand All @@ -429,7 +432,7 @@ fn main() -> Result<()> {
let output_parent_dir = options
.output_in_dir
.clone()
.unwrap_or_else(|| workspace.dir.clone());
.unwrap_or_else(|| workspace.root().to_owned());

let mut discovered = workspace.discover(&package_filter, &options, &console)?;

Expand Down Expand Up @@ -468,7 +471,7 @@ fn main() -> Result<()> {
output_dir.write_previously_caught(&previously_caught)?;
}
console.set_debug_log(output_dir.open_debug_log()?);
let lab_outcome = test_mutants(mutants, &workspace.dir, output_dir, options, &console)?;
let lab_outcome = test_mutants(mutants, workspace.root(), output_dir, options, &console)?;
exit(lab_outcome.exit_code());
}
Ok(())
Expand Down
8 changes: 5 additions & 3 deletions src/mutate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,13 +247,14 @@ mod test {
use indoc::indoc;
use itertools::Itertools;
use pretty_assertions::assert_eq;
use test_util::copy_of_testdata;

use crate::*;

#[test]
fn discover_factorial_mutants() {
let tree_path = Utf8Path::new("testdata/factorial");
let workspace = Workspace::open(tree_path).unwrap();
let tmp = copy_of_testdata("factorial");
let workspace = Workspace::open(tmp.path()).unwrap();
let options = Options::default();
let mutants = workspace
.mutants(&PackageFilter::All, &options, &Console::new())
Expand Down Expand Up @@ -334,7 +335,8 @@ mod test {

#[test]
fn mutate_factorial() -> Result<()> {
let tree_path = Utf8Path::new("testdata/factorial");
let temp = copy_of_testdata("factorial");
let tree_path = temp.path();
let mutants = Workspace::open(tree_path)?.mutants(
&PackageFilter::All,
&Options::default(),
Expand Down
4 changes: 2 additions & 2 deletions src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ mod test {
let tmp = minimal_source_tree();
let tmp_path: &Utf8Path = tmp.path().try_into().unwrap();
let workspace = Workspace::open(tmp_path).unwrap();
let output_dir = OutputDir::new(&workspace.dir).unwrap();
let output_dir = OutputDir::new(workspace.root()).unwrap();
assert_eq!(
list_recursive(tmp.path()),
&[
Expand All @@ -439,7 +439,7 @@ mod test {
"src/lib.rs",
]
);
assert_eq!(output_dir.path(), workspace.dir.join("mutants.out"));
assert_eq!(output_dir.path(), workspace.root().join("mutants.out"));
assert!(output_dir.path().join("lock.json").is_file());
}

Expand Down
58 changes: 31 additions & 27 deletions src/workspace.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// Copyright 2023 Martin Pool
// Copyright 2023-2024 Martin Pool

use std::fmt;
use std::panic::catch_unwind;
use std::path::Path;
use std::sync::Arc;

use anyhow::{anyhow, ensure, Context};
Expand All @@ -21,16 +22,11 @@ use crate::source::SourceFile;
use crate::visit::{walk_tree, Discovered};
use crate::Result;

pub struct Workspace {
pub dir: Utf8PathBuf,
metadata: cargo_metadata::Metadata,
}

impl fmt::Debug for Workspace {
#[mutants::skip]
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Workspace")
.field("dir", &self.dir)
.field("root", &self.root().to_string())
// .field("metadata", &self.metadata)
.finish()
}
Expand Down Expand Up @@ -106,20 +102,30 @@ struct PackageTop {
top_sources: Vec<Utf8PathBuf>,
}

pub struct Workspace {
metadata: cargo_metadata::Metadata,
}

impl Workspace {
/// The root directory of the workspace.
pub fn root(&self) -> &Utf8Path {
&self.metadata.workspace_root
}

/// Open the workspace containing a given directory.
pub fn open<P: AsRef<Utf8Path>>(start_dir: P) -> Result<Self> {
let dir = locate_project(start_dir.as_ref(), true)?;
pub fn open<P: AsRef<Path>>(start_dir: P) -> Result<Self> {
let start_dir = start_dir.as_ref();
let dir = locate_project(start_dir.try_into().expect("start_dir is UTF-8"), true)?;
let manifest_path = dir.join("Cargo.toml");
debug!(?manifest_path, ?dir, "Find root files");
debug!(?manifest_path, "Find root files");
check_interrupted()?;
let metadata = cargo_metadata::MetadataCommand::new()
.no_deps()
.manifest_path(&manifest_path)
.exec()
.with_context(|| format!("Failed to run cargo metadata on {:?}", manifest_path))?;
debug!(workspace_root = ?metadata.workspace_root, "Found workspace root");
Ok(Workspace { dir, metadata })
Ok(Workspace { metadata })
}

/// Find packages to mutate, subject to some filtering.
Expand Down Expand Up @@ -153,12 +159,12 @@ impl Workspace {
let manifest_path = &package_metadata.manifest_path;
debug!(%manifest_path, "walk package");
let relative_manifest_path = manifest_path
.strip_prefix(&self.dir)
.strip_prefix(self.root())
.map_err(|_| {
anyhow!(
"manifest path {manifest_path:?} for package {name:?} is not \
within the detected source root path {dir:?}",
dir = self.dir
dir = self.root(),
)
})?
.to_owned();
Expand All @@ -168,7 +174,7 @@ impl Workspace {
});
tops.push(PackageTop {
package,
top_sources: direct_package_sources(&self.dir, package_metadata)?,
top_sources: direct_package_sources(self.root(), package_metadata)?,
});
}
if let PackageFilter::Explicit(ref names) = package_filter {
Expand All @@ -191,7 +197,7 @@ impl Workspace {
{
for source_path in top_sources {
sources.extend(SourceFile::new(
&self.dir,
self.root(),
source_path.to_owned(),
&package,
true,
Expand All @@ -209,7 +215,7 @@ impl Workspace {
console: &Console,
) -> Result<Discovered> {
walk_tree(
&self.dir,
self.root(),
&self.top_sources(package_filter)?,
options,
console,
Expand Down Expand Up @@ -307,13 +313,12 @@ fn locate_project(path: &Utf8Path, workspace: bool) -> Result<Utf8PathBuf> {

#[cfg(test)]
mod test {
use std::ffi::OsStr;

use camino::Utf8Path;
use itertools::Itertools;

use crate::console::Console;
use crate::options::Options;
use crate::test_util::copy_of_testdata;
use crate::workspace::PackageFilter;

use super::Workspace;
Expand All @@ -325,20 +330,19 @@ mod test {

#[test]
fn open_subdirectory_of_crate_opens_the_crate() {
let workspace =
Workspace::open("testdata/factorial/src").expect("open source tree from subdirectory");
let root = &workspace.dir;
let tmp = copy_of_testdata("factorial");
let workspace = Workspace::open(&tmp).expect("open source tree from subdirectory");
let root = workspace.root();
assert!(root.is_dir());
assert!(root.join("Cargo.toml").is_file());
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 = Workspace::open("testdata/workspace/main")
.expect("Find root from within workspace/main")
.dir;
let workspace = Workspace::open("testdata/workspace/main")
.expect("Find root from within workspace/main");
let root = workspace.root();
assert_eq!(root.file_name(), Some("workspace"), "Wrong root: {root:?}");
}

Expand Down Expand Up @@ -403,7 +407,7 @@ mod test {
#[test]
fn filter_by_single_package() {
let workspace = Workspace::open("testdata/workspace/main").expect("Find workspace root");
let root_dir = &workspace.dir;
let root_dir = workspace.root();
assert_eq!(
root_dir.file_name(),
Some("workspace"),
Expand Down Expand Up @@ -434,7 +438,7 @@ mod test {
fn filter_by_multiple_packages() {
let workspace = Workspace::open("testdata/workspace/main").unwrap();
assert_eq!(
workspace.dir.file_name(),
workspace.root().file_name(),
Some("workspace"),
"found the workspace root"
);
Expand Down
11 changes: 11 additions & 0 deletions testdata/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# cargo-mutants testdata

Each directory below here is a Rust tree used by the cargo-mutants tests.

In these trees, the manifest file is called `Cargo_test.toml` (rather than `Cargo.toml`) for a couple of reasons:

1. `cargo publish` excludes directories containing `Cargo.toml`, on the grounds that each crate should be published separately, but we want to include these in the published tarball so that the tests can run and succeed in an unpacked tarball. (See https://github.com/sourcefrog/cargo-mutants/issues/355.)

2. We don't want cargo to look at these crates when building or resolving dependencies for cargo-mutants itself.

Since the `--manifest-path` of Cargo commands expects the manifest to be named `Cargo.toml` we have to always copy these trees before using them. The `copy_of_testdata` helper function copies them and fixes the manifest name. Copying the tree also avoids any conflicts between concurrent or consecutive tests.
File renamed without changes.
40 changes: 20 additions & 20 deletions tests/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,18 @@
use std::env;
use std::fmt::Write;

use path_slash::PathBufExt;
use predicates::prelude::*;
use pretty_assertions::assert_eq;

mod util;
use util::{all_testdata_tree_paths, copy_of_testdata, run, CommandInstaExt, OUTER_TIMEOUT};
use util::{all_testdata_tree_names, copy_of_testdata, run, CommandInstaExt, OUTER_TIMEOUT};

#[test]
fn list_diff_json_contains_diffs() {
let tmp = copy_of_testdata("factorial");
let cmd = run()
.args([
"mutants",
"--list",
"--json",
"--diff",
"-d",
"testdata/factorial",
])
.args(["mutants", "--list", "--json", "--diff", "-d"])
.arg(tmp.path())
.assert()
.success(); // needed for lifetime
let out = cmd.get_output();
Expand All @@ -44,13 +38,14 @@ fn list_mutants_in_all_trees_as_json() {
// review I want just a single snapshot, and json-inside-json has quoting
// that makes it harder to review.
let mut buf = String::new();
for dir_path in all_testdata_tree_paths() {
writeln!(buf, "## {}\n", dir_path.to_slash_lossy()).unwrap();
for tree_name in all_testdata_tree_names() {
writeln!(buf, "## testdata/{tree_name}\n").unwrap();
let tmp = copy_of_testdata(&tree_name);
let cmd_assert = run()
.arg("mutants")
.arg("--list")
.arg("--json")
.current_dir(&dir_path)
.current_dir(tmp.path())
.timeout(OUTER_TIMEOUT)
.assert()
.success();
Expand All @@ -63,12 +58,13 @@ fn list_mutants_in_all_trees_as_json() {
#[test]
fn list_mutants_in_all_trees_as_text() {
let mut buf = String::new();
for dir_path in all_testdata_tree_paths() {
writeln!(buf, "## {}\n\n```", dir_path.to_slash_lossy()).unwrap();
for tree_name in all_testdata_tree_names() {
writeln!(buf, "## testdata/{tree_name}\n\n```").unwrap();
let tmp = copy_of_testdata(&tree_name);
let cmd_assert = run()
.arg("mutants")
.arg("--list")
.current_dir(&dir_path)
.current_dir(tmp.path())
.timeout(OUTER_TIMEOUT)
.assert()
.success();
Expand All @@ -80,20 +76,22 @@ fn list_mutants_in_all_trees_as_text() {

#[test]
fn list_mutants_in_factorial() {
let tmp = copy_of_testdata("factorial");
run()
.arg("mutants")
.arg("--list")
.current_dir("testdata/factorial")
.current_dir(&tmp)
.assert_insta("list_mutants_in_factorial");
}

#[test]
fn list_mutants_in_factorial_json() {
let tmp = copy_of_testdata("factorial");
run()
.arg("mutants")
.arg("--list")
.arg("--json")
.current_dir("testdata/factorial")
.current_dir(&tmp.path())
.assert_insta("list_mutants_in_factorial_json");
}

Expand Down Expand Up @@ -141,21 +139,23 @@ fn list_mutants_in_cfg_attr_test_skip_json() {

#[test]
fn list_mutants_with_dir_option() {
let temp = copy_of_testdata("factorial");
run()
.arg("mutants")
.arg("--list")
.arg("--dir")
.arg("testdata/factorial")
.arg(temp.path())
.assert_insta("list_mutants_with_dir_option");
}

#[test]
fn list_mutants_with_diffs_in_factorial() {
let temp = copy_of_testdata("factorial");
run()
.arg("mutants")
.arg("--list")
.arg("--diff")
.current_dir("testdata/factorial")
.current_dir(&temp)
.assert_insta("list_mutants_with_diffs_in_factorial");
}

Expand Down
Loading

0 comments on commit 6833360

Please sign in to comment.