From f07dcd13a0a1eb9596f8184b21860e732404d3b4 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 19 Jan 2022 02:22:58 +0800 Subject: [PATCH 1/8] test: compile all bins when `TESTNAME` arg exists This behavior is not correct should be fixed. Update this test only for pointing out the current behavior. --- tests/testsuite/test.rs | 59 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index eb66bc77c00..1679b60d4cf 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -1606,32 +1606,77 @@ fn test_run_implicit_example_target() { #[cargo_test] fn test_filtered_excludes_compiling_examples() { let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [[bin]] + name = "mybin" + test = false + "#, + ) .file( "src/lib.rs", - "#[cfg(test)] mod tests { #[test] fn foo() { assert!(true); } }", + "#[cfg(test)] mod tests { #[test] fn test_in_lib() { } }", + ) + .file( + "src/bin/mybin.rs", + "#[test] fn test_in_bin() { } + fn main() { panic!(\"Don't execute me!\"); }", + ) + .file("tests/mytest.rs", "#[test] fn test_in_test() { }") + .file( + "benches/mybench.rs", + "#[test] fn test_in_bench() { assert!(false) }", + ) + .file( + "examples/myexm1.rs", + "#[test] fn test_in_exm() { assert!(false) } + fn main() { panic!(\"Don't execute me!\"); }", ) - .file("examples/ex1.rs", "fn main() {}") .build(); - p.cargo("test -v foo") + p.cargo("test -v test_in_") .with_stdout( " running 1 test -test tests::foo ... ok +test tests::test_in_lib ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..] + + +running 1 test +test test_in_bin ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..] + + +running 1 test +test test_in_test ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..] ", ) - .with_stderr( + .with_stderr_unordered( "\ [COMPILING] foo v0.0.1 ([CWD]) +[RUNNING] `rustc --crate-name foo src/lib.rs [..] --crate-type lib [..]` [RUNNING] `rustc --crate-name foo src/lib.rs [..] --test [..]` +[RUNNING] `rustc --crate-name mybin src/bin/mybin.rs [..] --crate-type bin [..]` +[RUNNING] `rustc --crate-name mybin src/bin/mybin.rs [..] --test [..]` +[RUNNING] `rustc --crate-name mytest tests/mytest.rs [..] --test [..]` [FINISHED] test [unoptimized + debuginfo] target(s) in [..] -[RUNNING] `[CWD]/target/debug/deps/foo-[..] foo` +[RUNNING] `[CWD]/target/debug/deps/foo-[..] test_in_` +[RUNNING] `[CWD]/target/debug/deps/mybin-[..] test_in_` +[RUNNING] `[CWD]/target/debug/deps/mytest-[..] test_in_` ", ) - .with_stderr_does_not_contain("[RUNNING][..]rustc[..]ex1[..]") + .with_stderr_does_not_contain("[RUNNING][..]rustc[..]myexm1[..]") .run(); } From 0a7a7bd5ef8a9fcb9511fd576d4d9b37b9e80533 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 19 Jan 2022 02:42:08 +0800 Subject: [PATCH 2/8] do not compile test for bins flagged as `test = false` Binaries without `test = false` are included by default, so no need to explicitly compile all binaries. --- src/bin/cargo/commands/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/cargo/commands/test.rs b/src/bin/cargo/commands/test.rs index 25b949dcfb9..c03bc915f6f 100644 --- a/src/bin/cargo/commands/test.rs +++ b/src/bin/cargo/commands/test.rs @@ -109,7 +109,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { if let CompileFilter::Default { .. } = compile_opts.filter { compile_opts.filter = ops::CompileFilter::new( LibRule::Default, // compile the library, so the unit tests can be run filtered - FilterRule::All, // compile the binaries, so the unit tests in binaries can be run filtered + FilterRule::none(), // binaries without `test = false` are included by default FilterRule::All, // compile the tests, so the integration tests can be run filtered FilterRule::none(), // specify --examples to unit test binaries filtered FilterRule::none(), // specify --benches to unit test benchmarks filtered From bee3534f27300198f87acbd3982512f9e05f7408 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 19 Jan 2022 02:45:05 +0800 Subject: [PATCH 3/8] test: do not compile test for bins flagged as `test = false` --- tests/testsuite/test.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index 1679b60d4cf..4b2f4b1b299 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -1649,12 +1649,6 @@ test tests::test_in_lib ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..] -running 1 test -test test_in_bin ... ok - -test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..] - - running 1 test test test_in_test ... ok @@ -1668,15 +1662,14 @@ test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..] [RUNNING] `rustc --crate-name foo src/lib.rs [..] --crate-type lib [..]` [RUNNING] `rustc --crate-name foo src/lib.rs [..] --test [..]` [RUNNING] `rustc --crate-name mybin src/bin/mybin.rs [..] --crate-type bin [..]` -[RUNNING] `rustc --crate-name mybin src/bin/mybin.rs [..] --test [..]` [RUNNING] `rustc --crate-name mytest tests/mytest.rs [..] --test [..]` [FINISHED] test [unoptimized + debuginfo] target(s) in [..] [RUNNING] `[CWD]/target/debug/deps/foo-[..] test_in_` -[RUNNING] `[CWD]/target/debug/deps/mybin-[..] test_in_` [RUNNING] `[CWD]/target/debug/deps/mytest-[..] test_in_` ", ) .with_stderr_does_not_contain("[RUNNING][..]rustc[..]myexm1[..]") + .with_stderr_does_not_contain("[RUNNING][..]deps/mybin-[..] test_in_") .run(); } From 9138c35d8cbd035fddd3a231424f390a83cfaa8a Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 20 Jan 2022 11:30:55 +0800 Subject: [PATCH 4/8] Extract logic of all test target filter to `all_test_targets` --- src/bin/cargo/commands/test.rs | 16 ++++++---------- src/cargo/ops/cargo_compile.rs | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/bin/cargo/commands/test.rs b/src/bin/cargo/commands/test.rs index c03bc915f6f..cb35ab5aeeb 100644 --- a/src/bin/cargo/commands/test.rs +++ b/src/bin/cargo/commands/test.rs @@ -105,16 +105,12 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { FilterRule::none(), FilterRule::none(), ); - } else if test_name.is_some() { - if let CompileFilter::Default { .. } = compile_opts.filter { - compile_opts.filter = ops::CompileFilter::new( - LibRule::Default, // compile the library, so the unit tests can be run filtered - FilterRule::none(), // binaries without `test = false` are included by default - FilterRule::All, // compile the tests, so the integration tests can be run filtered - FilterRule::none(), // specify --examples to unit test binaries filtered - FilterRule::none(), // specify --benches to unit test benchmarks filtered - ); // also, specify --doc to run doc tests filtered - } + } else if test_name.is_some() && !compile_opts.filter.is_specific() { + // If arg `TESTNAME` is provided, assumed that the user knows what + // exactly they wants to test, so we use `all_test_targets` to + // avoid compiling unnecessary targets such as examples, which are + // included by the logic of default target filter. + compile_opts.filter = ops::CompileFilter::all_test_targets(); } let ops = ops::TestOptions { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index a21cf4f429b..11ff38b6a1b 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -828,6 +828,25 @@ impl CompileFilter { } } + /// Constructs a filter that includes all test targets. + /// + /// Being different from the behavior of [`CompileFilter::Default`], this + /// function only recongnizes test targets, which means cargo might compile + /// all targets with `tested` flag on, whereas [`CompileFilter::Default`] + /// may include additional example targets to ensure they can be compiled. + /// + /// Note that the actual behavior is subject to `filter_default_targets` + /// and `generate_targets` though. + pub fn all_test_targets() -> Self { + Self::Only { + all_targets: false, + lib: LibRule::Default, + bins: FilterRule::none(), + examples: FilterRule::none(), + tests: FilterRule::All, + benches: FilterRule::none(), + } + } pub fn need_dev_deps(&self, mode: CompileMode) -> bool { match mode { CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true, From 5aa985c42a7a746fb954bb2ba0e659f58ee29fdb Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 20 Jan 2022 11:33:11 +0800 Subject: [PATCH 5/8] Extract logic of lib target only filter to `lib_only` --- src/bin/cargo/commands/test.rs | 26 ++++++++------------------ src/cargo/ops/cargo_compile.rs | 13 +++++++++++++ 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/bin/cargo/commands/test.rs b/src/bin/cargo/commands/test.rs index cb35ab5aeeb..a02bdb57443 100644 --- a/src/bin/cargo/commands/test.rs +++ b/src/bin/cargo/commands/test.rs @@ -1,6 +1,6 @@ use crate::command_prelude::*; use anyhow::Error; -use cargo::ops::{self, CompileFilter, FilterRule, LibRule}; +use cargo::ops; pub fn cli() -> App { subcommand("test") @@ -77,7 +77,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { // `TESTNAME` is actually an argument of the test binary, but it's // important, so we explicitly mention it and reconfigure. - let test_name: Option<&str> = args.value_of("TESTNAME"); + let test_name = args.value_of("TESTNAME"); let test_args = args.value_of("TESTNAME").into_iter(); let test_args = test_args.chain(args.values_of("args").unwrap_or_default()); let test_args = test_args.collect::>(); @@ -85,26 +85,16 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let no_run = args.is_present("no-run"); let doc = args.is_present("doc"); if doc { - if let CompileFilter::Only { .. } = compile_opts.filter { - return Err(CliError::new( - anyhow::format_err!("Can't mix --doc with other target selecting options"), - 101, - )); + if compile_opts.filter.is_specific() { + return Err( + anyhow::format_err!("Can't mix --doc with other target selecting options").into(), + ); } if no_run { - return Err(CliError::new( - anyhow::format_err!("Can't skip running doc tests with --no-run"), - 101, - )); + return Err(anyhow::format_err!("Can't skip running doc tests with --no-run").into()); } compile_opts.build_config.mode = CompileMode::Doctest; - compile_opts.filter = ops::CompileFilter::new( - LibRule::True, - FilterRule::none(), - FilterRule::none(), - FilterRule::none(), - FilterRule::none(), - ); + compile_opts.filter = ops::CompileFilter::lib_only(); } else if test_name.is_some() && !compile_opts.filter.is_specific() { // If arg `TESTNAME` is provided, assumed that the user knows what // exactly they wants to test, so we use `all_test_targets` to diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 11ff38b6a1b..699a6be3960 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -847,6 +847,19 @@ impl CompileFilter { benches: FilterRule::none(), } } + + /// Constructs a filter that includes lib target only. + pub fn lib_only() -> Self { + Self::Only { + all_targets: false, + lib: LibRule::True, + bins: FilterRule::none(), + examples: FilterRule::none(), + tests: FilterRule::none(), + benches: FilterRule::none(), + } + } + pub fn need_dev_deps(&self, mode: CompileMode) -> bool { match mode { CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true, From 0a73f8b9fdda85e51276e97bde01649fe0ea2b4c Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 20 Jan 2022 12:34:57 +0800 Subject: [PATCH 6/8] Extract logic of single bin target filter to `single_bin` --- src/bin/cargo/commands/run.rs | 15 ++------------- src/cargo/ops/cargo_compile.rs | 12 ++++++++++++ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/bin/cargo/commands/run.rs b/src/bin/cargo/commands/run.rs index d90399dd8a8..57b69dbf293 100644 --- a/src/bin/cargo/commands/run.rs +++ b/src/bin/cargo/commands/run.rs @@ -62,19 +62,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { .iter() .filter_map(|pkg| pkg.manifest().default_run()) .collect(); - if default_runs.len() == 1 { - compile_opts.filter = CompileFilter::from_raw_arguments( - false, - vec![default_runs[0].to_owned()], - false, - vec![], - false, - vec![], - false, - vec![], - false, - false, - ); + if let [bin] = &default_runs[..] { + compile_opts.filter = CompileFilter::single_bin(bin.to_string()); } else { // ops::run will take care of errors if len pkgs != 1. compile_opts.filter = CompileFilter::Default { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 699a6be3960..7dfcda26898 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -860,6 +860,18 @@ impl CompileFilter { } } + /// Constructs a filter that includes the given binary. No more. No less. + pub fn single_bin(bin: String) -> Self { + Self::Only { + all_targets: false, + lib: LibRule::False, + bins: FilterRule::new(vec![bin], false), + examples: FilterRule::none(), + tests: FilterRule::none(), + benches: FilterRule::none(), + } + } + pub fn need_dev_deps(&self, mode: CompileMode) -> bool { match mode { CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true, From ecbaec1a9f73fc4698bdab924e68f6570c74daa8 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 20 Jan 2022 11:41:12 +0800 Subject: [PATCH 7/8] Use `new_all_targets` in cargo-fix when no target selection exists --- src/bin/cargo/commands/fix.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/bin/cargo/commands/fix.rs b/src/bin/cargo/commands/fix.rs index e366f7f892b..c8e36c58062 100644 --- a/src/bin/cargo/commands/fix.rs +++ b/src/bin/cargo/commands/fix.rs @@ -1,6 +1,6 @@ use crate::command_prelude::*; -use cargo::ops::{self, CompileFilter, FilterRule, LibRule}; +use cargo::ops; pub fn cli() -> App { subcommand("fix") @@ -76,15 +76,9 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let mut opts = args.compile_options(config, mode, Some(&ws), ProfileChecking::LegacyTestOnly)?; - if let CompileFilter::Default { .. } = opts.filter { - opts.filter = CompileFilter::Only { - all_targets: true, - lib: LibRule::Default, - bins: FilterRule::All, - examples: FilterRule::All, - benches: FilterRule::All, - tests: FilterRule::All, - } + if !opts.filter.is_specific() { + // cargo fix with no target selection implies `--all-targets`. + opts.filter = ops::CompileFilter::new_all_targets(); } ops::fix( From 2f5b3033f209a4ccc416d41664f5e120d893518e Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 20 Jan 2022 12:52:17 +0800 Subject: [PATCH 8/8] Update doc for CompileFilter --- src/cargo/ops/cargo_compile.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 7dfcda26898..c34276a5fc1 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -759,7 +759,7 @@ impl FilterRule { } impl CompileFilter { - /// Construct a CompileFilter from raw command line arguments. + /// Constructs a filter from raw command line arguments. pub fn from_raw_arguments( lib_only: bool, bins: Vec, @@ -788,7 +788,7 @@ impl CompileFilter { CompileFilter::new(rule_lib, rule_bins, rule_tsts, rule_exms, rule_bens) } - /// Construct a CompileFilter from underlying primitives. + /// Constructs a filter from underlying primitives. pub fn new( rule_lib: LibRule, rule_bins: FilterRule, @@ -817,6 +817,7 @@ impl CompileFilter { } } + /// Constructs a filter that includes all targets. pub fn new_all_targets() -> CompileFilter { CompileFilter::Only { all_targets: true, @@ -872,6 +873,7 @@ impl CompileFilter { } } + /// Indicates if Cargo needs to build any dev dependency. pub fn need_dev_deps(&self, mode: CompileMode) -> bool { match mode { CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true, @@ -892,8 +894,8 @@ impl CompileFilter { } } - // this selects targets for "cargo run". for logic to select targets for - // other subcommands, see generate_targets and filter_default_targets + /// Selects targets for "cargo run". for logic to select targets for other + /// subcommands, see `generate_targets` and `filter_default_targets`. pub fn target_run(&self, target: &Target) -> bool { match *self { CompileFilter::Default { .. } => true,