Skip to content

Commit

Permalink
--all-targets fixes
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
ehuss committed Sep 17, 2018
1 parent b7ef8d5 commit 3a1cad6
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 51 deletions.
3 changes: 1 addition & 2 deletions src/bin/cargo/commands/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
};

Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 1 addition & 2 deletions src/bin/cargo/commands/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down Expand Up @@ -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,
};

Expand Down
81 changes: 55 additions & 26 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -531,36 +547,33 @@ 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<Proposal> = Vec::new();

match *filter {
CompileFilter::Default {
required_features_filterable,
} => {
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,
});
}
}
}
Expand All @@ -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,
});
}
}
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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<String> = required_features
.iter()
Expand All @@ -688,7 +707,7 @@ fn generate_targets<'a>(
}
// else, silently skip target.
}
Ok(units)
Ok(units.into_iter().collect())
}

fn resolve_all_features(
Expand Down Expand Up @@ -746,14 +765,19 @@ fn list_rule_targets<'a>(
target_desc: &'static str,
is_expected_kind: fn(&Target) -> bool,
mode: CompileMode,
) -> CargoResult<Vec<(&'a Package, &'a Target, bool, CompileMode)>> {
) -> CargoResult<Vec<Proposal<'a>>> {
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,
});
}
}
}
Expand All @@ -780,12 +804,17 @@ fn find_named_targets<'a>(
target_desc: &'static str,
is_expected_kind: fn(&Target) -> bool,
mode: CompileMode,
) -> CargoResult<Vec<(&'a Package, &'a Target, bool, CompileMode)>> {
) -> CargoResult<Vec<Proposal<'a>>> {
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,
});
}
}
}
Expand Down
17 changes: 1 addition & 16 deletions src/cargo/ops/cargo_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
Expand Down
14 changes: 14 additions & 0 deletions tests/testsuite/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

0 comments on commit 3a1cad6

Please sign in to comment.