Skip to content

Commit

Permalink
Tests pass in cargo-mutants crate tarballs (#422)
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`.

The motivation is that Debian, or others who package from the tarball,
can run the tests after building.

Most of the changes in this are connected to `cargo publish` behavior of
excluding various files including symlinks, `Cargo.toml`, and `.cargo`.
This PR renames them and recreates them during the test, in the process
of copying the testdata to a temporary directory. Aside from anything
else this should make sure that each test is hermetic.

Also, since we're now creating the symlink from a test, we can also do
that on Windows and check that it works.

This also adds a CI job that checks that the tests do pass in an
unpacked tarball.

Also, delete a lot of Insta snapshots that had accumulated and are no
longer needed.
  • Loading branch information
sourcefrog authored Oct 13, 2024
2 parents 7277e55 + d3e4cf6 commit d9e9c0b
Show file tree
Hide file tree
Showing 165 changed files with 456 additions and 23,531 deletions.
49 changes: 47 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ jobs:
run: cargo build --all-targets
- name: Test
run: cargo test --workspace
- run: cargo update
- run: cargo build --all-targets

minimal-versions:
strategy:
Expand All @@ -86,6 +84,53 @@ jobs:
tool: nextest
- run: cargo test

# Run `cargo update` and check the tests still pass
maximal-versions:
strategy:
matrix:
os: [macOS-latest, ubuntu-latest, windows-latest]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@nightly
- uses: Swatinem/rust-cache@v2
- run: cargo update
- uses: taiki-e/install-action@v2
name: Install nextest using install-action
with:
tool: nextest
- run: cargo test

tests-from-tarball:
strategy:
matrix:
os: [ubuntu-latest]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@nightly
- uses: Swatinem/rust-cache@v2
- run: cargo package --no-verify
- name: Upload package artifact
uses: actions/upload-artifact@v4
with:
name: cargo-mutants-package
path: |
target/package
- name: Unpack package
run: |
cd target/package
ls -l
tar xvf cargo-mutants*.crate
- name: Install nextest using install-action
uses: taiki-e/install-action@v2
with:
tool: nextest
- name: Run tests from package
run: |
cd target/package/cargo-mutants-*.*.[0-9]
cargo test
# Install from a checkout of the source, to find broken dependencies etc.
# We run this on various versions because some dependencies might have changed
# their MSRV, and on every platform because there are platform-specific
Expand Down
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 2 additions & 47 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ regex = "1.10"
serde_json = "1.0.117"
similar = "2.1"
strum = { version = "0.26", features = ["derive"] }
tempfile = "3.4"
tempfile = "3.8"
time = "0.3"
toml = "0.8"
tracing = "0.1.40"
Expand Down Expand Up @@ -79,7 +79,7 @@ nix = { version = "0.28", features = ["process", "signal"] }

[dev-dependencies]
assert_cmd = "2.0"
cp_r = "0.5.1"
cp_r = { version = "0.5.2" } # git = "https://github.com/sourcefrog/cp_r"
insta = "1.12"
lazy_static = "1.4"
predicates = "3"
Expand All @@ -91,51 +91,6 @@ walkdir = "2.5"
members = ["mutants_attrs"]
resolver = "2"

# Exclude all testdata, so that they're more isolated from the real tree, and
# so that support for testing workspaces does not try to test the whole
# cargo-mutants tree.
exclude = [
"testdata/already_failing_tests",
"testdata/already_hangs",
"testdata/cdylib",
"testdata/cfg_attr_mutants_skip",
"testdata/cfg_attr_test_skip",
"testdata/cfg_test_inner",
"testdata/custom_top_file",
"testdata/dangling_mod",
"testdata/dependency",
"testdata/diff0",
"testdata/diff1",
"testdata/error_value",
"testdata/everything_skipped",
"testdata/factorial",
"testdata/fails_without_feature",
"testdata/hang_avoided_by_attr/",
"testdata/hang_when_mutated",
"testdata/insta",
"testdata/integration_tests",
"testdata/many_patterns",
"testdata/missing_test",
"testdata/missing_test_fixed",
"testdata/mut_ref",
"testdata/nested_mod",
"testdata/never_type",
"testdata/override_dependency",
"testdata/package-fails/",
"testdata/patch_dependency",
"testdata/proc_macro",
"testdata/relative_dependency",
"testdata/replace_dependency",
"testdata/small_well_tested",
"testdata/strict_warnings",
"testdata/struct_with_no_default",
"testdata/symlink",
"testdata/unapply",
"testdata/unsafe",
"testdata/well_tested",
"testdata/with_child_directories",
]

# Config for 'cargo dist'
[workspace.metadata.dist]
# The preferred cargo-dist version to use in CI (Cargo.toml SemVer syntax)
Expand Down
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

- New: Write diffs to dedicated files under `mutants.out/diff/`. The filename is included in the mutant json output.

- New: The package tarball on `crates.io` now includes all the test data, so that the tests can be run on the unpacked tarball. This may be helpful for people packaging `cargo-mutants` for distributions, and keeps an accurate record of the whole tree separate from the git history.

## 24.9.0

- Fixed: Avoid generating empty string elements in `ENCODED_RUSTFLAGS` when `--cap-lints` is set. In some situations these could cause a compiler error complaining about the empty argument.
Expand Down
15 changes: 10 additions & 5 deletions src/build_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,16 @@ impl BuildDir {

#[cfg(test)]
mod test {
use test_util::copy_of_testdata;

use super::*;

#[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 tmp = copy_of_testdata("factorial");
let workspace = Workspace::open(tmp.path()).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 @@ -106,13 +110,14 @@ mod test {

#[test]
fn build_dir_in_place() -> Result<()> {
let workspace = Workspace::open("testdata/factorial")?;
let build_dir = BuildDir::in_place(&workspace.dir)?;
let tmp = copy_of_testdata("factorial");
let workspace = Workspace::open(tmp.path())?;
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
11 changes: 7 additions & 4 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 @@ -315,7 +316,8 @@ mod test {

#[test]
fn filter_by_attributes() {
let mutants = Workspace::open(Utf8Path::new("testdata/hang_avoided_by_attr"))
let tmp = copy_of_testdata("hang_avoided_by_attr");
let mutants = Workspace::open(tmp.path())
.unwrap()
.mutants(&PackageFilter::All, &Options::default(), &Console::new())
.unwrap();
Expand All @@ -334,7 +336,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
27 changes: 1 addition & 26 deletions src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::time::{Duration, Instant};
use anyhow::{bail, Context};
use camino::Utf8Path;
use serde::Serialize;
use tracing::{debug, debug_span, error, span, trace, warn, Level};
use tracing::{debug, span, trace, warn, Level};

use crate::console::Console;
use crate::interrupt::check_interrupted;
Expand Down Expand Up @@ -200,31 +200,6 @@ impl ProcessStatus {
}
}

/// Run a command and return its stdout output as a string.
///
/// If the command exits non-zero, the error includes any messages it wrote to stderr.
///
/// The runtime is capped by [METADATA_TIMEOUT].
pub fn get_command_output(argv: &[&str], cwd: &Utf8Path) -> Result<String> {
// TODO: Perhaps redirect to files so this doesn't jam if there's a lot of output.
// For the commands we use this for today, which only produce small output, it's OK.
let _span = debug_span!("get_command_output", argv = ?argv).entered();
let output = Command::new(argv[0])
.args(&argv[1..])
.stderr(Stdio::inherit())
.current_dir(cwd)
.output()
.with_context(|| format!("failed to spawn {argv:?}"))?;
let exit = output.status;
if !exit.success() {
error!(?exit, "Child failed");
bail!("Child failed with status {exit:?}: {argv:?}");
}
let stdout = String::from_utf8(output.stdout).context("Child output is not UTF-8")?;
debug!("output: {}", stdout.trim());
Ok(stdout)
}

/// Quote an argv slice in Unix shell style.
///
/// This is not completely guaranteed, but is only for debug logs.
Expand Down
4 changes: 3 additions & 1 deletion src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,7 @@ fn find_path_attribute(attrs: &[Attribute]) -> std::result::Result<Option<Utf8Pa
mod test {
use indoc::indoc;
use itertools::Itertools;
use test_util::copy_of_testdata;

use super::*;
use crate::package::Package;
Expand Down Expand Up @@ -761,7 +762,8 @@ mod test {
fn no_mutants_in_files_with_inner_cfg_test_attribute() {
let options = Options::default();
let console = Console::new();
let workspace = Workspace::open("testdata/cfg_test_inner").unwrap();
let tmp = copy_of_testdata("cfg_test_inner");
let workspace = Workspace::open(tmp.path()).unwrap();
let discovered = workspace
.discover(&PackageFilter::All, &options, &console)
.unwrap();
Expand Down
Loading

0 comments on commit d9e9c0b

Please sign in to comment.