From 235712f5998d223895282e5e52b228808ac02f13 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sat, 7 Oct 2017 09:28:34 -0700 Subject: [PATCH] Add unit test checking to `cargo check` - Add `--profile test` flag to `cargo check` that will enable checking of unit tests. - `--tests` will implicitly enable checking of unit tests within the lib. - Don't implicitly compile binaries when using just `--test` filters. - Fix erroneously linking tests when run with `--test`. Fixes #3431, #4003, #4059, #4330. --- src/bin/check.rs | 24 +++- src/bin/rustc.rs | 2 +- src/cargo/core/manifest.rs | 9 ++ src/cargo/core/workspace.rs | 1 + src/cargo/ops/cargo_clean.rs | 4 +- src/cargo/ops/cargo_compile.rs | 20 ++- src/cargo/ops/cargo_rustc/context.rs | 2 +- src/cargo/util/toml/mod.rs | 2 + tests/cargotest/install.rs | 2 +- tests/check.rs | 195 ++++++++++++++++++++++++++- 10 files changed, 238 insertions(+), 23 deletions(-) diff --git a/src/bin/check.rs b/src/bin/check.rs index 98220413076..c1b963c373c 100644 --- a/src/bin/check.rs +++ b/src/bin/check.rs @@ -2,7 +2,7 @@ use std::env; use cargo::core::Workspace; use cargo::ops::{self, CompileOptions, MessageFormat, Packages}; -use cargo::util::{CliResult, Config}; +use cargo::util::{CliResult, CliError, Config}; use cargo::util::important_paths::find_root_manifest_for_wd; pub const USAGE: &'static str = " @@ -28,6 +28,7 @@ Options: --benches Check all benches --all-targets Check all targets (lib and bin targets by default) --release Check artifacts in release mode, with optimizations + --profile PROFILE Profile to build the selected target for --features FEATURES Space-separated list of features to also check --all-features Check all available features --no-default-features Do not check the `default` feature @@ -53,6 +54,9 @@ Note that `--exclude` has to be specified in conjunction with the `--all` flag. Compilation can be configured via the use of profiles which are configured in the manifest. The default profile for this command is `dev`, but passing the --release flag will use the `release` profile instead. + +The `--profile test` flag can be used to check unit tests with the +`#[cfg(test)]` attribute. "; #[derive(Deserialize)] @@ -83,6 +87,7 @@ pub struct Options { flag_frozen: bool, flag_all: bool, flag_exclude: Vec, + flag_profile: Option, #[serde(rename = "flag_Z")] flag_z: Vec, } @@ -106,6 +111,17 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { &options.flag_exclude, &options.flag_package)?; + let test = options.flag_tests || + match options.flag_profile.as_ref().map(|t| &t[..]) { + Some("test") => true, + None => false, + Some(profile) => { + let err = format!("unknown profile: `{}`, only `test` is currently supported", + profile).into(); + return Err(CliError::new(err, 101)) + } + }; + let opts = CompileOptions { config: config, jobs: options.flag_jobs, @@ -114,10 +130,10 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { all_features: options.flag_all_features, no_default_features: options.flag_no_default_features, spec: spec, - mode: ops::CompileMode::Check, + mode: ops::CompileMode::Check{test:test}, release: options.flag_release, - filter: ops::CompileFilter::new(options.flag_lib, - &options.flag_bin, options.flag_bins, + filter: ops::CompileFilter::new(options.flag_lib || options.flag_tests, + &options.flag_bin, options.flag_bins || options.flag_tests, &options.flag_test, options.flag_tests, &options.flag_example, options.flag_examples, &options.flag_bench, options.flag_benches, diff --git a/src/bin/rustc.rs b/src/bin/rustc.rs index e6f5dc540f6..747f2ead958 100644 --- a/src/bin/rustc.rs +++ b/src/bin/rustc.rs @@ -102,7 +102,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { Some("dev") | None => CompileMode::Build, Some("test") => CompileMode::Test, Some("bench") => CompileMode::Bench, - Some("check") => CompileMode::Check, + Some("check") => CompileMode::Check {test: false}, Some(mode) => { let err = format!("unknown profile: `{}`, use dev, test, or bench", mode).into(); diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index d8c3710ed43..53c70baf917 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -189,6 +189,7 @@ pub struct Profiles { pub doc: Profile, pub custom_build: Profile, pub check: Profile, + pub check_test: Profile, pub doctest: Profile, } @@ -661,6 +662,14 @@ impl Profile { } } + pub fn default_check_test() -> Profile { + Profile { + check: true, + test: true, + ..Profile::default_dev() + } + } + pub fn default_doctest() -> Profile { Profile { doc: true, diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 58b1412696e..d63ded77140 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -545,6 +545,7 @@ impl<'cfg> Workspace<'cfg> { doc: Profile::default_doc(), custom_build: Profile::default_custom_build(), check: Profile::default_check(), + check_test: Profile::default_check_test(), doctest: Profile::default_doctest(), }; diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 9b266b0bbf8..bc25a888ef2 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -54,10 +54,10 @@ pub fn clean(ws: &Workspace, opts: &CleanOptions) -> CargoResult<()> { let Profiles { ref release, ref dev, ref test, ref bench, ref doc, ref custom_build, ref test_deps, ref bench_deps, ref check, - ref doctest, + ref check_test, ref doctest, } = *profiles; let profiles = [release, dev, test, bench, doc, custom_build, - test_deps, bench_deps, check, doctest]; + test_deps, bench_deps, check, check_test, doctest]; for profile in profiles.iter() { units.push(Unit { pkg, diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 685911203de..d6678ac6b72 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -92,7 +92,7 @@ impl<'a> CompileOptions<'a> { pub enum CompileMode { Test, Build, - Check, + Check { test: bool }, Bench, Doc { deps: bool }, Doctest, @@ -478,7 +478,7 @@ fn generate_auto_targets<'a>(mode: CompileMode, targets: &'a [Target], } base } - CompileMode::Build | CompileMode::Check => { + CompileMode::Build | CompileMode::Check{..} => { targets.iter().filter(|t| { t.is_bin() || t.is_lib() }).map(|t| BuildProposal { @@ -603,11 +603,18 @@ fn generate_targets<'a>(pkg: &'a Package, CompileMode::Test => test, CompileMode::Bench => &profiles.bench, CompileMode::Build => build, - CompileMode::Check => &profiles.check, + CompileMode::Check {test: false} => &profiles.check, + CompileMode::Check {test: true} => &profiles.check_test, CompileMode::Doc { .. } => &profiles.doc, CompileMode::Doctest => &profiles.doctest, }; + let test_profile = if profile.check { + &profiles.check_test + } else { + profile + }; + let targets = match *filter { CompileFilter::Default { required_features_filterable } => { let deps = if release { @@ -631,15 +638,14 @@ fn generate_targets<'a>(pkg: &'a Package, bail!("no library targets found") } } - targets.append(&mut propose_indicated_targets( pkg, bins, "bin", Target::is_bin, profile)?); targets.append(&mut propose_indicated_targets( - pkg, examples, "example", Target::is_example, build)?); + pkg, examples, "example", Target::is_example, profile)?); targets.append(&mut propose_indicated_targets( - pkg, tests, "test", Target::is_test, test)?); + pkg, tests, "test", Target::is_test, test_profile)?); targets.append(&mut propose_indicated_targets( - pkg, benches, "bench", Target::is_bench, &profiles.bench)?); + pkg, benches, "bench", Target::is_bench, test_profile)?); targets } }; diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 369c9d7c533..334af3eaa88 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -862,7 +862,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { ret.extend(self.maybe_lib(unit)); // Integration tests/benchmarks require binaries to be built - if unit.profile.test && + if unit.profile.test && !unit.profile.check && (unit.target.is_test() || unit.target.is_bench()) { ret.extend(unit.pkg.targets().iter().filter(|t| { let no_required_features = Vec::new(); diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 32122444d2e..ef5c4c13c4c 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1041,6 +1041,8 @@ fn build_profiles(profiles: &Option) -> Profiles { custom_build: Profile::default_custom_build(), check: merge(Profile::default_check(), profiles.and_then(|p| p.dev.as_ref())), + check_test: merge(Profile::default_check_test(), + profiles.and_then(|p| p.dev.as_ref())), doctest: Profile::default_doctest(), }; // The test/bench targets cannot have panic=abort because they'll all get diff --git a/tests/cargotest/install.rs b/tests/cargotest/install.rs index 12a396972e5..e768e040e2b 100644 --- a/tests/cargotest/install.rs +++ b/tests/cargotest/install.rs @@ -12,7 +12,7 @@ pub fn cargo_home() -> PathBuf { pub struct InstalledExe(pub &'static str); -fn exe(name: &str) -> String { +pub fn exe(name: &str) -> String { if cfg!(windows) {format!("{}.exe", name)} else {name.to_string()} } diff --git a/tests/check.rs b/tests/check.rs index 37f2ea8281a..2e547bd2149 100644 --- a/tests/check.rs +++ b/tests/check.rs @@ -1,10 +1,21 @@ extern crate cargotest; extern crate hamcrest; +extern crate glob; use cargotest::is_nightly; use cargotest::support::{execs, project}; use cargotest::support::registry::Package; -use hamcrest::assert_that; +use hamcrest::prelude::*; +use cargotest::support::paths::CargoPathExt; +use cargotest::install::exe; +use glob::glob; + +const SIMPLE_MANIFEST: &str = r#" +[package] +name = "foo" +version = "0.0.1" +authors = [] +"#; #[test] fn check_success() { @@ -437,12 +448,7 @@ fn check_virtual_all_implied() { #[test] fn check_all_targets() { let foo = project("foo") - .file("Cargo.toml", r#" - [package] - name = "foo" - version = "0.0.1" - authors = [] - "#) + .file("Cargo.toml", SIMPLE_MANIFEST) .file("src/main.rs", "fn main() {}") .file("src/lib.rs", "pub fn smth() {}") .file("examples/example1.rs", "fn main() {}") @@ -459,3 +465,178 @@ fn check_all_targets() { .with_stderr_contains("[..] --crate-name bench3 benches[/]bench3.rs [..]") ); } + +#[test] +fn check_unit_test_profile() { + let foo = project("foo") + .file("Cargo.toml", SIMPLE_MANIFEST) + .file("src/lib.rs", r#" + #[cfg(test)] + mod tests { + #[test] + fn it_works() { + badtext + } + } + "#); + + foo.build(); + assert_that(foo.cargo("check"), + execs().with_status(0)); + assert_that(foo.cargo("check").arg("--profile").arg("test"), + execs().with_status(101) + .with_stderr_contains("[..]badtext[..]")); +} + +#[test] +fn check_unit_test_all_tests() { + // Lib unit. + let p = project("foo") + .file("Cargo.toml", SIMPLE_MANIFEST) + .file("src/lib.rs", r#" + fn unused_normal_lib() {} + #[cfg(test)] + mod tests { + fn unused_unit_lib() {} + } + "#) + .file("src/main.rs", r#" + fn main() {} + fn unused_normal_bin() {} + #[cfg(test)] + mod tests { + fn unused_unit_bin() {} + } + "#) + .file("tests/t1.rs", r#" + fn unused_normal_t1() {} + #[cfg(test)] + mod tests { + fn unused_unit_t1() {} + } + "#) + .file("examples/ex1.rs", r#" + fn main() {} + fn unused_normal_ex1() {} + #[cfg(test)] + mod tests { + fn unused_unit_ex1() {} + } + "#) + .file("benches/b1.rs", r#" + fn unused_normal_b1() {} + #[cfg(test)] + mod tests { + fn unused_unit_b1() {} + } + "#); + + p.build(); + assert_that(p.cargo("check"), + execs().with_status(0) + .with_stderr_contains("[..]unused_normal_lib[..]") + .with_stderr_contains("[..]unused_normal_bin[..]") + .with_stderr_does_not_contain("unused_noraml_t1") + .with_stderr_does_not_contain("unused_noraml_ex1") + .with_stderr_does_not_contain("unused_noraml_b1") + .with_stderr_does_not_contain("unused_unit_")); + p.root().join("target").rm_rf(); + assert_that(p.cargo("check").arg("--tests").arg("-v"), + execs().with_status(0) + .with_stderr_contains("[..]unused_unit_lib[..]") + .with_stderr_contains("[..]unused_unit_bin[..]") + .with_stderr_contains("[..]unused_unit_t1[..]") + .with_stderr_does_not_contain("unused_normal_ex1") + .with_stderr_does_not_contain("unused_normal_b1") + .with_stderr_does_not_contain("unused_unit_ex1") + .with_stderr_does_not_contain("unused_unit_b1")); + p.root().join("target").rm_rf(); + assert_that(p.cargo("check").arg("--all-targets").arg("-v"), + execs().with_status(0) + .with_stderr_contains("[..]unused_normal_lib[..]") + .with_stderr_contains("[..]unused_normal_bin[..]") + .with_stderr_contains("[..]unused_normal_t1[..]") + .with_stderr_contains("[..]unused_normal_ex1[..]") + .with_stderr_contains("[..]unused_normal_b1[..]") + .with_stderr_contains("[..]unused_unit_b1[..]") + .with_stderr_contains("[..]unused_unit_t1[..]") + .with_stderr_does_not_contain("unused_unit_lib") + .with_stderr_does_not_contain("unused_unit_bin") + .with_stderr_does_not_contain("unused_unit_ex1")); +} + +#[test] +fn check_artifacts() +{ + // Verify which artifacts are created when running check (#4059). + let p = project("foo") + .file("Cargo.toml", SIMPLE_MANIFEST) + .file("src/lib.rs", "") + .file("src/main.rs", "fn main() {}") + .file("tests/t1.rs", "") + .file("examples/ex1.rs", "fn main() {}") + .file("benches/b1.rs", ""); + p.build(); + assert_that(p.cargo("check"), execs().with_status(0)); + assert_that(&p.root().join("target/debug/libfoo.rmeta"), + existing_file()); + assert_that(&p.root().join("target/debug/libfoo.rlib"), + is_not(existing_file())); + assert_that(&p.root().join("target/debug").join(exe("foo")), + is_not(existing_file())); + + p.root().join("target").rm_rf(); + assert_that(p.cargo("check").arg("--lib"), execs().with_status(0)); + assert_that(&p.root().join("target/debug/libfoo.rmeta"), + existing_file()); + assert_that(&p.root().join("target/debug/libfoo.rlib"), + is_not(existing_file())); + assert_that(&p.root().join("target/debug").join(exe("foo")), + is_not(existing_file())); + + p.root().join("target").rm_rf(); + assert_that(p.cargo("check").arg("--bin").arg("foo"), + execs().with_status(0)); + assert_that(&p.root().join("target/debug/libfoo.rmeta"), + existing_file()); + assert_that(&p.root().join("target/debug/libfoo.rlib"), + is_not(existing_file())); + assert_that(&p.root().join("target/debug").join(exe("foo")), + is_not(existing_file())); + + p.root().join("target").rm_rf(); + assert_that(p.cargo("check").arg("--test").arg("t1"), + execs().with_status(0)); + assert_that(&p.root().join("target/debug/libfoo.rmeta"), + existing_file()); + assert_that(&p.root().join("target/debug/libfoo.rlib"), + is_not(existing_file())); + assert_that(&p.root().join("target/debug").join(exe("foo")), + is_not(existing_file())); + assert_that(glob(&p.root().join("target/debug/t1-*").to_str().unwrap()) + .unwrap().count(), + is(equal_to(0))); + + p.root().join("target").rm_rf(); + assert_that(p.cargo("check").arg("--example").arg("ex1"), + execs().with_status(0)); + assert_that(&p.root().join("target/debug/libfoo.rmeta"), + existing_file()); + assert_that(&p.root().join("target/debug/libfoo.rlib"), + is_not(existing_file())); + assert_that(&p.root().join("target/debug/examples").join(exe("ex1")), + is_not(existing_file())); + + p.root().join("target").rm_rf(); + assert_that(p.cargo("check").arg("--bench").arg("b1"), + execs().with_status(0)); + assert_that(&p.root().join("target/debug/libfoo.rmeta"), + existing_file()); + assert_that(&p.root().join("target/debug/libfoo.rlib"), + is_not(existing_file())); + assert_that(&p.root().join("target/debug").join(exe("foo")), + is_not(existing_file())); + assert_that(glob(&p.root().join("target/debug/b1-*").to_str().unwrap()) + .unwrap().count(), + is(equal_to(0))); +}