From 3a1cad6f2851401542951bc78f6ed78dcb978edd Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 16 Sep 2018 19:41:57 -0700 Subject: [PATCH] --all-targets fixes - Fix: `cargo test --all-targets` was running lib tests three times. - `--all-targets` help strings were wrong or misleading. - Minor cleanup to add `Proposal` type to maybe make the code more readable. --- src/bin/cargo/commands/bench.rs | 3 +- src/bin/cargo/commands/build.rs | 2 +- src/bin/cargo/commands/check.rs | 2 +- src/bin/cargo/commands/fix.rs | 2 +- src/bin/cargo/commands/rustc.rs | 2 +- src/bin/cargo/commands/rustdoc.rs | 2 +- src/bin/cargo/commands/test.rs | 3 +- src/cargo/ops/cargo_compile.rs | 81 +++++++++++++++++++++---------- src/cargo/ops/cargo_test.rs | 17 +------ tests/testsuite/test.rs | 14 ++++++ 10 files changed, 77 insertions(+), 51 deletions(-) diff --git a/src/bin/cargo/commands/bench.rs b/src/bin/cargo/commands/bench.rs index ea12a45754a..b688e36a5b6 100644 --- a/src/bin/cargo/commands/bench.rs +++ b/src/bin/cargo/commands/bench.rs @@ -26,7 +26,7 @@ pub fn cli() -> App { "Benchmark all tests", "Benchmark only the specified bench target", "Benchmark all benches", - "Benchmark all targets (default)", + "Benchmark all targets", ) .arg(opt("no-run", "Compile, but don't run benchmarks")) .arg_package_spec( @@ -78,7 +78,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let ops = TestOptions { no_run: args.is_present("no-run"), no_fail_fast: args.is_present("no-fail-fast"), - only_doc: false, compile_opts, }; diff --git a/src/bin/cargo/commands/build.rs b/src/bin/cargo/commands/build.rs index f1c8b256b2e..9a3e27213a5 100644 --- a/src/bin/cargo/commands/build.rs +++ b/src/bin/cargo/commands/build.rs @@ -22,7 +22,7 @@ pub fn cli() -> App { "Build all tests", "Build only the specified bench target", "Build all benches", - "Build all targets (lib and bin targets by default)", + "Build all targets", ) .arg_release("Build artifacts in release mode, with optimizations") .arg_features() diff --git a/src/bin/cargo/commands/check.rs b/src/bin/cargo/commands/check.rs index 72cebfda8f4..c9bbac1e74a 100644 --- a/src/bin/cargo/commands/check.rs +++ b/src/bin/cargo/commands/check.rs @@ -21,7 +21,7 @@ pub fn cli() -> App { "Check all tests", "Check only the specified bench target", "Check all benches", - "Check all targets (lib and bin targets by default)", + "Check all targets", ) .arg_release("Check artifacts in release mode, with optimizations") .arg(opt("profile", "Profile to build the selected target for").value_name("PROFILE")) diff --git a/src/bin/cargo/commands/fix.rs b/src/bin/cargo/commands/fix.rs index f88e52794fa..b98968b5406 100644 --- a/src/bin/cargo/commands/fix.rs +++ b/src/bin/cargo/commands/fix.rs @@ -21,7 +21,7 @@ pub fn cli() -> App { "Fix all tests", "Fix only the specified bench target", "Fix all benches", - "Fix all targets (lib and bin targets by default)", + "Fix all targets (default)", ) .arg_release("Fix artifacts in release mode, with optimizations") .arg(opt("profile", "Profile to build the selected target for").value_name("PROFILE")) diff --git a/src/bin/cargo/commands/rustc.rs b/src/bin/cargo/commands/rustc.rs index a5ca4deff02..dd2f1aa2cc0 100644 --- a/src/bin/cargo/commands/rustc.rs +++ b/src/bin/cargo/commands/rustc.rs @@ -19,7 +19,7 @@ pub fn cli() -> App { "Build all tests", "Build only the specified bench target", "Build all benches", - "Build all targets (lib and bin targets by default)", + "Build all targets", ) .arg_release("Build artifacts in release mode, with optimizations") .arg(opt("profile", "Profile to build the selected target for").value_name("PROFILE")) diff --git a/src/bin/cargo/commands/rustdoc.rs b/src/bin/cargo/commands/rustdoc.rs index 9dda84f2e73..8593bd2d0f3 100644 --- a/src/bin/cargo/commands/rustdoc.rs +++ b/src/bin/cargo/commands/rustdoc.rs @@ -23,7 +23,7 @@ pub fn cli() -> App { "Build all tests", "Build only the specified bench target", "Build all benches", - "Build all targets (default)", + "Build all targets", ) .arg_release("Build artifacts in release mode, with optimizations") .arg_features() diff --git a/src/bin/cargo/commands/test.rs b/src/bin/cargo/commands/test.rs index d6df9484d4a..0e9b577b594 100644 --- a/src/bin/cargo/commands/test.rs +++ b/src/bin/cargo/commands/test.rs @@ -27,7 +27,7 @@ pub fn cli() -> App { "Test all tests", "Test only the specified bench target", "Test all benches", - "Test all targets (default)", + "Test all targets", ) .arg(opt("doc", "Test only this library's documentation")) .arg(opt("no-run", "Compile, but don't run tests")) @@ -116,7 +116,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let ops = ops::TestOptions { no_run: args.is_present("no-run"), no_fail_fast: args.is_present("no-fail-fast"), - only_doc: doc, compile_opts, }; diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 58d1771042f..0c353d9bc8f 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -444,6 +444,22 @@ impl CompileFilter { } } +/// A proposed target. +/// +/// Proposed targets are later filtered into actual Units based on whether or +/// not the target requires its features to be present. +#[derive(Debug)] +struct Proposal<'a> { + pkg: &'a Package, + target: &'a Target, + /// Indicates whether or not all required features *must* be present. If + /// false, and the features are not available, then it will be silently + /// skipped. Generally, targets specified by name (`--bin foo`) are + /// required, all others can be silently skipped if features are missing. + requires_features: bool, + mode: CompileMode, +} + /// Generates all the base targets for the packages the user has requested to /// compile. Dependencies for these targets are computed later in /// `unit_dependencies`. @@ -531,13 +547,8 @@ fn generate_targets<'a>( } }; - // Create a list of proposed targets. The `bool` value indicates - // whether or not all required features *must* be present. If false, - // and the features are not available, then it will be silently - // skipped. Generally, targets specified by name (`--bin foo`) are - // required, all others can be silently skipped if features are - // missing. - let mut proposals: Vec<(&Package, &Target, bool, CompileMode)> = Vec::new(); + // Create a list of proposed targets. + let mut proposals: Vec = Vec::new(); match *filter { CompileFilter::Default { @@ -545,22 +556,24 @@ fn generate_targets<'a>( } => { for pkg in packages { let default = filter_default_targets(pkg.targets(), build_config.mode); - proposals.extend(default.into_iter().map(|target| { - ( - *pkg, - target, - !required_features_filterable, - build_config.mode, - ) + proposals.extend(default.into_iter().map(|target| Proposal { + pkg, + target, + requires_features: !required_features_filterable, + mode: build_config.mode, })); if build_config.mode == CompileMode::Test { - // Include doctest for lib. if let Some(t) = pkg .targets() .iter() .find(|t| t.is_lib() && t.doctested() && t.doctestable()) { - proposals.push((pkg, t, false, CompileMode::Doctest)); + proposals.push(Proposal { + pkg, + target: t, + requires_features: false, + mode: CompileMode::Doctest, + }); } } } @@ -586,7 +599,12 @@ fn generate_targets<'a>( pkg.name() ))?; } else { - libs.push((*pkg, target, false, build_config.mode)); + libs.push(Proposal { + pkg, + target, + requires_features: false, + mode: build_config.mode, + }); } } } @@ -600,6 +618,7 @@ fn generate_targets<'a>( } proposals.extend(libs); } + // If --tests was specified, add all targets that would be // generated by `cargo test`. let test_filter = match *tests { @@ -657,8 +676,8 @@ fn generate_targets<'a>( // Only include targets that are libraries or have all required // features available. let mut features_map = HashMap::new(); - let mut units = Vec::new(); - for (pkg, target, required, mode) in proposals { + let mut units = HashSet::new(); + for Proposal { pkg, target, requires_features, mode} in proposals { let unavailable_features = match target.required_features() { Some(rf) => { let features = features_map @@ -670,8 +689,8 @@ fn generate_targets<'a>( }; if target.is_lib() || unavailable_features.is_empty() { let unit = new_unit(pkg, target, mode); - units.push(unit); - } else if required { + units.insert(unit); + } else if requires_features { let required_features = target.required_features().unwrap(); let quoted_required_features: Vec = required_features .iter() @@ -688,7 +707,7 @@ fn generate_targets<'a>( } // else, silently skip target. } - Ok(units) + Ok(units.into_iter().collect()) } fn resolve_all_features( @@ -746,14 +765,19 @@ fn list_rule_targets<'a>( target_desc: &'static str, is_expected_kind: fn(&Target) -> bool, mode: CompileMode, -) -> CargoResult> { +) -> CargoResult>> { let mut result = Vec::new(); match *rule { FilterRule::All => { for pkg in packages { for target in pkg.targets() { if is_expected_kind(target) { - result.push((*pkg, target, false, mode)); + result.push(Proposal { + pkg, + target, + requires_features: false, + mode, + }); } } } @@ -780,12 +804,17 @@ fn find_named_targets<'a>( target_desc: &'static str, is_expected_kind: fn(&Target) -> bool, mode: CompileMode, -) -> CargoResult> { +) -> CargoResult>> { let mut result = Vec::new(); for pkg in packages { for target in pkg.targets() { if target.name() == target_name && is_expected_kind(target) { - result.push((*pkg, target, true, mode)); + result.push(Proposal { + pkg, + target, + requires_features: true, + mode, + }); } } } diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index f1db552506e..79271e5f506 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -10,7 +10,6 @@ pub struct TestOptions<'a> { pub compile_opts: ops::CompileOptions<'a>, pub no_run: bool, pub no_fail_fast: bool, - pub only_doc: bool, } pub fn run_tests( @@ -23,27 +22,13 @@ pub fn run_tests( if options.no_run { return Ok(None); } - let (test, mut errors) = if options.only_doc { - assert!(options.compile_opts.filter.is_specific()); - run_doc_tests(options, test_args, &compilation)? - } else { - run_unit_tests(options, test_args, &compilation)? - }; + let (test, mut errors) = run_unit_tests(options, test_args, &compilation)?; // If we have an error and want to fail fast, return if !errors.is_empty() && !options.no_fail_fast { return Ok(Some(CargoTestError::new(test, errors))); } - // If a specific test was requested or we're not running any tests at all, - // don't run any doc tests. - if options.compile_opts.filter.is_specific() { - match errors.len() { - 0 => return Ok(None), - _ => return Ok(Some(CargoTestError::new(test, errors))), - } - } - let (doctest, docerrors) = run_doc_tests(options, test_args, &compilation)?; let test = if docerrors.is_empty() { test } else { doctest }; errors.extend(docerrors); diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index 5e026f9cc2e..3de5262801b 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -3168,3 +3168,17 @@ test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out .with_stderr("[ERROR] Can't mix --doc with other target selecting options\n") .run(); } + +#[test] +fn test_all_targets_lib() { + let p = project().file("src/lib.rs", "").build(); + + p.cargo("test --all-targets") + .with_stderr( + "\ +[COMPILING] foo [..] +[FINISHED] dev [..] +[RUNNING] [..]foo[..] +", + ).run(); +}