Skip to content

Commit

Permalink
Merge branch 'lint' into timeouts
Browse files Browse the repository at this point in the history
  • Loading branch information
sourcefrog committed Apr 7, 2024
2 parents ace8b70 + dba2cab commit 2172d76
Show file tree
Hide file tree
Showing 28 changed files with 279 additions and 125 deletions.
1 change: 1 addition & 0 deletions .github/workflows/install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ jobs:
strategy:
matrix:
locked: ["", "--locked"]
fail-fast: false
runs-on: ubuntu-latest
steps:
- name: cargo-install
Expand Down
27 changes: 25 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ jobs:
matrix:
os: [macOS-latest, ubuntu-latest, windows-latest]
version: [stable, nightly, "1.74"]

runs-on: ${{ matrix.os }}

steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@master
Expand All @@ -68,6 +66,31 @@ jobs:
run: cargo build --all-targets
- name: Test
run: cargo test --workspace
- run: cargo update
- run: cargo build --all-targets

# 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
# dependencies.
install:
strategy:
matrix:
os: [macOS-latest, ubuntu-latest, windows-latest]
version: [stable, nightly, "1.74"]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.version }}
- name: Show Cargo and rustc version
run: |
cargo --version
rustc --version
- uses: Swatinem/rust-cache@v2
- run: cargo install --locked --path .
- run: cargo install --path .

release-binary:
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion CITATION.cff
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ message: Please cite this project using these information.
repository-code: https://github.com/sourcefrog/cargo-mutants
title: cargo-mutants
url: https://mutants.rs/
version: 24.2.1
version: 24.3.0
15 changes: 11 additions & 4 deletions Cargo.lock

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

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-mutants"
version = "24.2.1"
version = "24.3.0"
edition = "2021"
authors = ["Martin Pool"]
license = "MIT"
Expand Down Expand Up @@ -69,7 +69,7 @@ toml = "0.8"
tracing = "0.1"
tracing-appender = "0.2"
tracing-subscriber = "0.3"
whoami = "1.2"
whoami = "1.5"

[dependencies.nutmeg]
version = "0.1.4"
Expand All @@ -88,7 +88,7 @@ version = "2"
features = ["full", "extra-traits", "visit"]

[target.'cfg(unix)'.dependencies]
nix = { version="0.28", features = ["signal"] }
nix = { version="0.28", features = ["process", "signal"] }

[dev-dependencies]
assert_cmd = "2.0"
Expand Down
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

## 24.3.0

- Fixed: `cargo install cargo-mutants` without `--locked` was failing due to breaking API changes in some unstable dependencies.

- Changed: In globs, `*` no longer matches path separators, only parts of a filename. For example, `src/*.rs` will now only match files directly in `src/`, not in subdirectories. To include subdirectories, use `**` as in `src/**/*.rs`.

And, patterns that do not contain a path separator match directories at any level, and all files within them. For example, `-f db` will match `src/db.rs` and `src/db/mod.rs` and all files in `src/db/` or in `other/db`.
Expand Down
2 changes: 1 addition & 1 deletion book/src/attrs.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ To mark functions as skipped, so they are not mutated:
2. Mark functions with `#[mutants::skip]` or other attributes containing
`mutants::skip` (e.g. `#[cfg_attr(test, mutants::skip)]`).

The `mutants` create is tiny and the attribute has no effect on the compiled
The `mutants` crate is tiny and the attribute has no effect on the compiled
code. It only flags the function for cargo-mutants. However, you can avoid the
dependency by using the slightly longer `#[cfg_attr(test, mutants::skip)]` form.

Expand Down
45 changes: 14 additions & 31 deletions book/src/shards.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Sharding

In addition to [running multiple jobs locally](parallelism.md), cargo-mutants can also run jobs on multiple machines, to get an overall job faster.
In addition to [running multiple jobs locally](parallelism.md), cargo-mutants can also run jobs on multiple machines, to get an overall result faster by using more CPU cores.

Each job tests a subset of mutants, selected by a shard. Shards are described as `k/n`, where `n` is the number of shards and `k` is the index of the shard, from 0 to `n-1`.

Expand All @@ -11,7 +11,7 @@ If any shard fails then that would indicate that some mutants were missed, or th
## Consistency across shards

**CAUTION:**
All shards must be run with the same arguments, and the same sharding `k`, or the results will be meaningless, as they won't agree on how to divide the work.
All shards must be run with the same arguments, and the same sharding denominator `n`, or the results will be meaningless, as they won't agree on how to divide the work.

Sharding can be combined with filters or shuffling, as long as the filters are set consistently in all shards. Sharding can also combine with `--in-diff`, again as long as all shards see the same diff.

Expand All @@ -22,39 +22,22 @@ Your CI system or other tooling is responsible for launching multiple shards, an
For example, in GitHub Actions, you could use a matrix job to run multiple shards:

```yaml
cargo-mutants:
runs-on: ubuntu-latest
# needs: [build, incremental-mutants]
strategy:
matrix:
shard: [0, 1, 2, 3, 4, 5, 6, 7]
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@master
with:
toolchain: beta
- uses: Swatinem/rust-cache@v2
- run: cargo install cargo-mutants
- name: Mutants
run: |
cargo mutants --no-shuffle -vV --shard ${{ matrix.shard }}/8
- name: Archive mutants.out
uses: actions/upload-artifact@v4
if: always()
with:
name: mutants.out
path: mutants.out
{{#include ../../examples/workflows/sharded.yml}}
```

Note that the number of shards is set to match the `/8` in the `--shard` argument.

## Skipping the baseline

[Sharding works with `--baseline=skip`](baseline.md), to avoid the cost of running the baseline on every shard. But, if you do this, then you must ensure that the tests suite is passing in the baseline, for example by checking it in a previous CI step.

## Performance of sharding

Each mutant does some constant upfront work:

* Any CI setup including starting the machine, getting a checkout, installing a Rust toolchain, and installing cargo-mutants
* An initial clean build of the code under test
* A baseline run of the unmutated code
* A baseline run of the unmutated code (unless this is skipped)

Then, for each mutant in its shard, it does an incremental build and runs all the tests.

Expand All @@ -63,30 +46,30 @@ Each shard runs the same number of mutants, +/-1. Typically this will mean they
A rough model for the overall execution time for all of the shards, allowing for this work occurring in parallel, is

```raw
SHARD_STARTUP + (CLEAN_BUILD + TEST) + (N_MUTANTS/K) * (INCREMENTAL_BUILD + TEST)
SHARD_STARTUP + (CLEAN_BUILD + TEST) + (N_TOTAL_MUTANTS / N_SHARDS) * (INCREMENTAL_BUILD + TEST)
```

The total cost in CPU seconds can be modelled as:

```raw
K * (SHARD_STARTUP + CLEAN_BUILD + TEST) + N_MUTANTS * (INCREMENTAL_BUILD + TEST)
N_SHARDS * (SHARD_STARTUP + CLEAN_BUILD + TEST) + N_MUTANTS * (INCREMENTAL_BUILD + TEST)
```

As a result, at very large `k` the cost of the initial setup work will dominate, but overall time to solution will be minimized.
As a result, if you use many shards the cost of the initial build will dominate, and the overall time will converge towards the time for a clean build, a baseline test, and the test of one mutant.

## Choosing a number of shards

Because there's some constant overhead for every shard there will be diminishing returns and increasing ineffiency if you use too many shards. (In the extreme cases where there are more shards than mutants, some of them will do the setup work, then find they have nothing to do and immediately exit.)
Because there's some constant overhead for every shard there will be diminishing returns and increasing ineffiency if you use too many shards. (In the extreme cases where there are more shards than mutants, some of them will find they have nothing to do and immediately exit.)

As a rule of thumb, you should probably choose `k` such that each worker runs at least 10 mutants, and possibly much more. 8 to 32 shards might be a good place to start.
As a rule of thumb, you should probably choose `n` such that each worker runs at least 10 mutants, and possibly much more. 8 to 32 shards might be a good place to start.

The optimal setting probably depends on how long your tree takes to build from zero and incrementally, how long the tests take to run, and the performance of your CI system.

If your CI system offers a choice of VM sizes you might experiment with using smaller or larger VMs and more or less shards: the optimal setting probably also depends on your tree's ability to exploit larger machines.

You should also think about cost and capacity constraints in your CI system, and the risk of starving out other users.

cargo-mutants has no internal scaling constraints to prevent you from setting `k` very large, if cost, efficiency and CI capacity are not a concern.
cargo-mutants has no internal scaling constraints to prevent you from setting `n` very large, if cost, efficiency and CI capacity are not a concern.

## Sampling mutants

Expand Down
2 changes: 1 addition & 1 deletion book/src/welcome.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ the tests might be insufficient.** ([More about these goals](goals.md).)
To get started:

1. [Install cargo-mutants](installation.md).
2. [Run `cargo mutants](getting-started.md) in your Rust source tree.
2. [Run `cargo mutants`](getting-started.md) in your Rust source tree.

For more resources see the repository at
<https://github.com/sourcefrog/cargo-mutants>.
81 changes: 81 additions & 0 deletions examples/workflows/sharded.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# Example of using a GitHub Actions matrix to shard the mutants run into 8 parts.

# See https://github.com/sourcefrog/cargo-mutants/blob/main/.github/workflows/tests.yml for a full example.

# Only run this on PRs or main branch commits that could affect the results,
# so we don't waste time on doc-only changes. Adjust these paths and branch names
# to suit your project.
on:
pull_request:
paths:
- ".cargo/*.toml"
- ".github/workflows/tests.yml"
- "Cargo.*"
- "mutants_attrs/**"
- "src/**"
- "testdata/**"
- "tests/**"
push:
branches:
- main
# Actions doesn't support YAML references, so it's repeated here
paths:
- ".cargo/*.toml"
- ".github/workflows/tests.yml"
- "Cargo.*"
- "mutants_attrs/**"
- "src/**"
- "testdata/**"
- "tests/**"

jobs:
# Before testing mutants, run the build and tests on all platforms.
# You probably already have CI configuration like this, so don't duplicate it,
# merge cargo-mutants into your existing workflow.
test:
strategy:
matrix:
os: [macOS-latest, ubuntu-latest, windows-latest]
version: [stable, nightly]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.version }}
components: rustfmt
- uses: swatinem/rust-cache@v2
- name: rustfmt
run: cargo fmt --all -- --check
- name: Build
run: cargo build --all-targets
- name: Test
run: cargo test --workspace
cargo-mutants:
runs-on: ubuntu-latest
# Often you'll want to only run this after the build is known to pass its basic tests,
# to avoid wasting time, and to allow using --baseline=skip.
needs: [test]
strategy:
fail-fast: false # Collect all mutants even if some are missed
matrix:
shard: [0, 1, 2, 3, 4, 5, 6, 7]
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@master
- uses: Swatinem/rust-cache@v2
- uses: taiki-e/install-action@v2
name: Install cargo-mutants using install-action
with:
tool: cargo-mutants
# Set an appropriate timeout for your tree here.
# The denominator of the shard count must be the number of shards.
- name: Mutants
run: |
cargo mutants --no-shuffle -vV --shard ${{ matrix.shard }}/8 --baseline=skip --timeout 300 --in-place
- name: Archive mutants.out
uses: actions/upload-artifact@v4
if: always()
with:
path: mutants.out
name: mutants-shard${{matrix.shard}}.out
4 changes: 0 additions & 4 deletions src/build_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

//! A directory containing mutated source to run cargo builds and tests.
use std::convert::TryInto;

use camino::{Utf8Path, Utf8PathBuf};
use tempfile::TempDir;
use tracing::info;

Expand Down Expand Up @@ -76,7 +73,6 @@ impl BuildDir {
#[cfg(test)]
mod test {
use super::*;
use crate::Result;

#[test]
fn build_dir_copy_from() {
Expand Down
6 changes: 0 additions & 6 deletions src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,12 @@

//! Run Cargo as a subprocess, including timeouts and propagating signals.
use std::env;
use std::time::{Duration, Instant};

use anyhow::Result;
use camino::Utf8Path;
use itertools::Itertools;
use nextest_metadata::NextestExitCode;
use tracing::{debug, debug_span, warn};

use crate::options::TestTool;
use crate::outcome::PhaseResult;
use crate::package::Package;
use crate::process::{Process, ProcessStatus};
Expand Down Expand Up @@ -160,8 +156,6 @@ mod test {
use pretty_assertions::assert_eq;
use rusty_fork::rusty_fork_test;

use crate::{Options, Phase};

use super::*;

#[test]
Expand Down
1 change: 0 additions & 1 deletion src/copy_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

//! Copy a source tree, with some exclusions, to a new temporary directory.
use std::convert::TryInto;
use std::fs::FileType;

use anyhow::Context;
Expand Down
4 changes: 2 additions & 2 deletions src/fnvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,10 @@ fn known_map(path: &Path) -> Option<(&Ident, &Type, &Type)> {
{
// TODO: Skip lifetime args.
// TODO: Return the path with args stripped out.
if let Some((GenericArgument::Type(key_type), GenericArgument::Type(value_type))) =
if let Some((GenericArgument::Type(ref key_type), GenericArgument::Type(ref value_type))) =
args.iter().collect_tuple()
{
return Some((&last.ident, &key_type, &value_type));
return Some((&last.ident, key_type, value_type));
}
}
None
Expand Down
Loading

0 comments on commit 2172d76

Please sign in to comment.