diff --git a/.cargo/mutants.toml b/.cargo/mutants.toml index 31bc768a..12ff5751 100644 --- a/.cargo/mutants.toml +++ b/.cargo/mutants.toml @@ -2,3 +2,4 @@ error_values = ["::anyhow::anyhow!(\"mutated!\")"] exclude_globs = ["src/console.rs"] +profile = "mutants" # Build without debug symbols diff --git a/Cargo.toml b/Cargo.toml index d54accb9..4e7380b1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,7 +42,7 @@ indoc = "2.0.0" itertools = "0.12" jobserver = "0.1" mutants = "0.0.3" -num_cpus ="1.16" +num_cpus = "1.16" patch = "0.7" path-slash = "0.2" quote = "1.0.35" diff --git a/NEWS.md b/NEWS.md index 7cb5fdb1..2362d8ec 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,10 @@ ## Unreleased +- 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. + +- New: `--profile` option allows selecting a Cargo profile. In particular, it's recommended that you can use `--profile=mutants` and configure a custom profile in your `Cargo.toml` to optimize the build for mutants, by turning off debug symbols. + - New: `--iterate` option skips mutants that were previously caught or unviable. - New: cargo-mutants starts a GNU jobserver, shared across all children, so that running multiple `--jobs` does not spawn an excessive number of compiler processes. The jobserver is on by default and can be turned off with `--jobserver false`. diff --git a/book/src/performance.md b/book/src/performance.md index 2982a29e..550b15d3 100644 --- a/book/src/performance.md +++ b/book/src/performance.md @@ -16,13 +16,33 @@ test binary. If you're using doctests only as testable documentation and not to assert correctness of the code, you can skip them with `cargo mutants -- --all-targets`. -## Optimized builds +## Choosing a cargo profile -On _some but not all_ projects, cargo-mutants can be faster if you use `-C --release`, which will make the build slower but may make the tests faster. Typically this will help on projects with very long CPU-intensive test suites. +[Cargo profiles](https://doc.rust-lang.org/cargo/reference/profiles.html) provide a way to configure compiler settings including several that influence build and runtime performance. -cargo-mutants now shows the breakdown of build versus test time which may help you work out if this will help: if the tests are much slower than the build it's worth trying more more compiler optimizations. +By default, cargo-mutants will use the default profile selected for `cargo test`, which is also called `test`. This includes debug symbols but disables optimization. + +You can select a different profile using the `--profile` option or the `profile` configuration key. + +You may wish to define a `mutants` profile in `Cargo.toml`, such as: + +```toml +[profile.mutants] +inherits = "test" +debug = "none" +``` + +and then configure this as the default in `.cargo/mutants.toml`: -On projects like this you might also choose just to turn up optimization for all debug builds in [`.cargo/config.toml`](https://doc.rust-lang.org/cargo/reference/config.html). +```toml +profile = "mutants" +``` + +Turning off debug symbols will make the builds faster, at the expense of possibly giving less useful output when a test fails. In general, since mutants are expected to cause tests to fail, debug symbols may not be worth cost. + +If your project's tests take a long time to run then it may be worth experimenting with increasing the `opt` level or other optimization parameters in the profile, to trade off longer builds for faster test runs. + +cargo-mutants now shows the breakdown of build versus test time which may help you work out if this will help: if the tests are much slower than the build it's worth trying more more compiler optimizations. ## Ramdisks diff --git a/src/cargo.rs b/src/cargo.rs index a0c3e704..b86ec01f 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -2,6 +2,7 @@ //! Run Cargo as a subprocess, including timeouts and propagating signals. +use std::iter::once; use std::time::{Duration, Instant}; use itertools::Itertools; @@ -27,14 +28,17 @@ pub fn run_cargo( let _span = debug_span!("run", ?phase).entered(); let start = Instant::now(); let argv = cargo_argv(build_dir.path(), packages, phase, options); - let env = vec![ - ("CARGO_ENCODED_RUSTFLAGS".to_owned(), rustflags(options)), + let mut env = vec![ // The tests might use Insta , and we don't want it to write // updates to the source tree, and we *certainly* don't want it to write // updates and then let the test pass. ("INSTA_UPDATE".to_owned(), "no".to_owned()), ("INSTA_FORCE_PASS".to_owned(), "0".to_owned()), ]; + if let Some(encoded_rustflags) = encoded_rustflags(options) { + debug!(?encoded_rustflags); + env.push(("CARGO_ENCODED_RUSTFLAGS".to_owned(), encoded_rustflags)); + } let process_status = Process::run( &argv, &env, @@ -111,6 +115,16 @@ fn cargo_argv( cargo_args.push("--tests".to_string()); } } + if let Some(profile) = &options.profile { + match options.test_tool { + TestTool::Cargo => { + cargo_args.push(format!("--profile={profile}")); + } + TestTool::Nextest => { + cargo_args.push(format!("--cargo-profile={profile}")); + } + } + } cargo_args.push("--verbose".to_string()); if let Some([package]) = packages { // Use the unambiguous form for this case; it works better when the same @@ -151,34 +165,33 @@ fn cargo_argv( /// /// See /// -fn rustflags(options: &Options) -> String { - let mut rustflags: Vec = if let Some(rustflags) = env::var_os("CARGO_ENCODED_RUSTFLAGS") - { - rustflags - .to_str() - .expect("CARGO_ENCODED_RUSTFLAGS is not valid UTF-8") - .split('\x1f') - .map(|s| s.to_owned()) - .collect() - } else if let Some(rustflags) = env::var_os("RUSTFLAGS") { - rustflags - .to_str() - .expect("RUSTFLAGS is not valid UTF-8") - .split(' ') - .map(|s| s.to_owned()) - .collect() +fn encoded_rustflags(options: &Options) -> Option { + let cap_lints_arg = "--cap-lints=warn"; + let separator = "\x1f"; + if !options.cap_lints { + None + } else if let Ok(encoded) = env::var("CARGO_ENCODED_RUSTFLAGS") { + if encoded.is_empty() { + Some(cap_lints_arg.to_owned()) + } else { + Some(encoded + separator + cap_lints_arg) + } + } else if let Ok(rustflags) = env::var("RUSTFLAGS") { + if rustflags.is_empty() { + Some(cap_lints_arg.to_owned()) + } else { + Some( + rustflags + .split(' ') + .filter(|s| !s.is_empty()) + .chain(once("--cap-lints=warn")) + .collect::>() + .join(separator), + ) + } } else { - // TODO: We could read the config files, but working out the right target and config seems complicated - // given the information available here. - // TODO: All matching target..rustflags and target..rustflags config entries joined together. - // TODO: build.rustflags config value. - Vec::new() - }; - if options.cap_lints { - rustflags.push("--cap-lints=warn".to_owned()); + Some(cap_lints_arg.to_owned()) } - // debug!("adjusted rustflags: {:?}", rustflags); - rustflags.join("\x1f") } #[cfg(test)] @@ -352,16 +365,78 @@ mod test { ); } + #[test] + fn profile_arg_passed_to_cargo() { + let args = Args::try_parse_from(["mutants", "--profile", "mutants"].as_slice()).unwrap(); + let options = Options::from_args(&args).unwrap(); + let build_dir = Utf8Path::new("/tmp/buildXYZ"); + assert_eq!( + cargo_argv(build_dir, None, Phase::Check, &options)[1..], + [ + "check", + "--tests", + "--profile=mutants", + "--verbose", + "--workspace", + ] + ); + } + + #[test] + fn nextest_gets_special_cargo_profile_option() { + let args = Args::try_parse_from( + ["mutants", "--test-tool=nextest", "--profile", "mutants"].as_slice(), + ) + .unwrap(); + let options = Options::from_args(&args).unwrap(); + let build_dir = Utf8Path::new("/tmp/buildXYZ"); + assert_eq!( + cargo_argv(build_dir, None, Phase::Build, &options)[1..], + [ + "nextest", + "run", + "--no-run", + "--cargo-profile=mutants", + "--verbose", + "--workspace", + ] + ); + } + rusty_fork_test! { #[test] - fn rustflags_with_no_environment_variables() { + fn rustflags_without_cap_lints_and_no_environment_variables() { + env::remove_var("RUSTFLAGS"); + env::remove_var("CARGO_ENCODED_RUSTFLAGS"); + assert_eq!( + encoded_rustflags(&Options { + ..Default::default() + }), + None + ); + } + #[test] + fn rustflags_with_cap_lints_and_no_environment_variables() { env::remove_var("RUSTFLAGS"); env::remove_var("CARGO_ENCODED_RUSTFLAGS"); assert_eq!( - rustflags(&Options { + encoded_rustflags(&Options { cap_lints: true, ..Default::default() }), + Some("--cap-lints=warn".into()) + ); + } + + // Don't generate an empty argument if the encoded rustflags is empty. + #[test] + fn rustflags_with_empty_encoded_rustflags() { + env::set_var("CARGO_ENCODED_RUSTFLAGS", ""); + assert_eq!( + encoded_rustflags(&Options { + cap_lints: true, + ..Default::default() + }).unwrap(), "--cap-lints=warn" ); } @@ -374,17 +449,17 @@ mod test { cap_lints: true, ..Default::default() }; - assert_eq!(rustflags(&options), "--something\x1f--else\x1f--cap-lints=warn"); + assert_eq!(encoded_rustflags(&options).unwrap(), "--something\x1f--else\x1f--cap-lints=warn"); } #[test] fn rustflags_added_to_existing_rustflags() { env::set_var("RUSTFLAGS", "-Dwarnings"); env::remove_var("CARGO_ENCODED_RUSTFLAGS"); - assert_eq!(rustflags(&Options { + assert_eq!(encoded_rustflags(&Options { cap_lints: true, ..Default::default() - }), "-Dwarnings\x1f--cap-lints=warn"); + }).unwrap(), "-Dwarnings\x1f--cap-lints=warn"); } } } diff --git a/src/config.rs b/src/config.rs index 5209f3b5..205041de 100644 --- a/src/config.rs +++ b/src/config.rs @@ -45,6 +45,8 @@ pub struct Config { pub additional_cargo_test_args: Vec, /// Minimum test timeout, in seconds, as a floor on the autoset value. pub minimum_test_timeout: Option, + /// Cargo profile. + pub profile: Option, /// Choice of test tool: cargo or nextest. pub test_tool: Option, /// Timeout multiplier, relative to the baseline 'cargo test'. diff --git a/src/main.rs b/src/main.rs index 4cbc0efb..f0843c93 100644 --- a/src/main.rs +++ b/src/main.rs @@ -296,6 +296,10 @@ pub struct Args { #[arg(long, help_heading = "Execution")] no_shuffle: bool, + /// Build with this cargo profile. + #[arg(long, help_heading = "Build")] + profile: Option, + /// Run only one shard of all generated mutants: specify as e.g. 1/4. #[arg(long, help_heading = "Execution")] shard: Option, diff --git a/src/options.rs b/src/options.rs index 11d32871..eca47ad3 100644 --- a/src/options.rs +++ b/src/options.rs @@ -85,6 +85,9 @@ pub struct Options { /// interesting results. pub shuffle: bool, + /// Cargo profile. + pub profile: Option, + /// Additional arguments for every cargo invocation. pub additional_cargo_args: Vec, @@ -205,6 +208,10 @@ impl Options { &config.additional_cargo_test_args, ), baseline: args.baseline, + build_timeout: args.build_timeout.map(Duration::from_secs_f64), + build_timeout_multiplier: args + .build_timeout_multiplier + .or(config.build_timeout_multiplier), cap_lints: args.cap_lints.unwrap_or(config.cap_lints), check_only: args.check, colors: args.colors, @@ -228,16 +235,13 @@ impl Options { output_in_dir: args.output.clone(), print_caught: args.caught, print_unviable: args.unviable, + profile: args.profile.as_ref().or(config.profile.as_ref()).cloned(), shuffle: !args.no_shuffle, show_line_col: args.line_col, show_times: !args.no_times, show_all_logs: args.all_logs, test_timeout: args.timeout.map(Duration::from_secs_f64), test_timeout_multiplier: args.timeout_multiplier.or(config.timeout_multiplier), - build_timeout: args.build_timeout.map(Duration::from_secs_f64), - build_timeout_multiplier: args - .build_timeout_multiplier - .or(config.build_timeout_multiplier), test_tool: args.test_tool.or(config.test_tool).unwrap_or_default(), }; options.error_values.iter().for_each(|e| { @@ -520,5 +524,28 @@ mod test { let options = Options::new(&args, &Config::default()).unwrap(); assert_eq!(options.colors.forced_value(), None); } + + #[test] + fn profile_option_from_args() { + let args = Args::parse_from(["mutants", "--profile=mutants"]); + let options = Options::new(&args, &Config::default()).unwrap(); + assert_eq!(options.profile.unwrap(), "mutants"); + } + + + #[test] + fn profile_from_config() { + let args = Args::parse_from(["mutants", "-j3"]); + let config = indoc! { r#" + profile = "mutants" + timeout_multiplier = 1.0 + build_timeout_multiplier = 2.0 + "#}; + let mut config_file = NamedTempFile::new().unwrap(); + config_file.write_all(config.as_bytes()).unwrap(); + let config = Config::read_file(config_file.path()).unwrap(); + let options = Options::new(&args, &config).unwrap(); + assert_eq!(options.profile.unwrap(), "mutants"); + } } } diff --git a/tests/main.rs b/tests/main.rs index e5f21280..c73ce782 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -785,7 +785,7 @@ fn mutants_causing_tests_to_hang_are_stopped_by_manual_timeout() { #[test] fn hang_avoided_by_build_timeout_with_cap_lints() { let tmp_src_dir = copy_of_testdata("hang_when_mutated"); - run() + let out = run() .arg("mutants") .args([ "--build-timeout-multiplier=4", @@ -795,8 +795,12 @@ fn hang_avoided_by_build_timeout_with_cap_lints() { .current_dir(tmp_src_dir.path()) .env_remove("RUST_BACKTRACE") .timeout(OUTER_TIMEOUT) - .assert() - .code(3); // exit_code::TIMEOUT + .assert(); + println!( + "debug log:\n===\n{}\n===", + read_to_string(tmp_src_dir.path().join("mutants.out/debug.log")).unwrap_or_default() + ); + out.code(3); // exit_code::TIMEOUT let timeout_txt = read_to_string(tmp_src_dir.path().join("mutants.out/timeout.txt")) .expect("read timeout.txt"); assert!(