Auto merge of #4592 - ehuss:check_test, r=alexcrichton

Add unit test checking to `cargo check`

This is an extension of PR #4039, fixing #3431, #4003, #4059, #4330.  The fixes for #4059 can potentially be separated into a separate PR, though there may be some overlap.

The general gist of the changes:

- 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 (checks the same targets as `cargo test`).  This affects the `check`, `test`, and `build` commands.
- `--benches` behaves similarly by using the same targets as `cargo bench`.
- Fix erroneously linking tests when run with `--test`.

There is one thing I did not do because I wanted more feedback on what others think the expected behavior should be.  What should the behavior of `--all-targets` be?  This patch does not (yet) make any changes to its behavior.  My initial thinking is that it should *add* a target of `--lib --bins --profile test`, but that essentially means the lib and bin targets will be checked twice (and thus any errors/warnings outside of `#[cfg(test)]` will appear twice, which may be confusing, and generally take twice as long to run).  I can add that, but I think it would require adding a new `All` variant to `CompileFilter` so that the code in `generate_targets` can detect this scenario.  I wanted feedback before making a more extensive change like that.  The downside of not adding it is that `--all-targets` will ignore unit tests (if you don't specify `--profile test`).

Summary of the profiles used with this patch:

Command                         | Lib               | Bin foo     | Test t1 | Example e1 | Bench b1 |
-------                         | ---               | -------     | ------- | ---------- | -------- |
`check`                         | check             | check       | -       | -          | -        |
`check --profile test`          | check_test†       | check_test† | -       | -          | -        |
`check --lib`                   | check             | -           | -       | -          | -        |
`check --lib --profile test`    | check_test†       | -           | -       | -          | -        |
`check --bin foo`               | check             | check       | -       | -          | -        |
`check -–bin foo –profile test` | check_test†       | check_test† | -       | -          | -        |
`check --bins`                  | check             | check       | -       | -          | -        |
`check --test t1`               | check             | check       | check_test   | -          | -        |
`check --tests`                 | check, check_test†  | check, check_test† | check_test | check†, check_test†  | -    |
`check --all-targets`           | check, check_test†  | check, check_test†  | check_test   | check, check_test† | check_test    |

† = different behavior from today

Author: bors

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<String>,
+    flag_profile: Option<String>,
     #[serde(rename = "flag_Z")]
     flag_z: Vec<String>,
 }
@@ -106,6 +111,16 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
                                     &options.flag_exclude,
                                     &options.flag_package)?;
 
+    let test = 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,7 +129,7 @@ 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,

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();

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,

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(),
             };
 

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,

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 {
@@ -606,11 +606,28 @@ 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 if mode == CompileMode::Build {
+        test
+    } else {
+        profile
+    };
+
+    let bench_profile = if profile.check {
+        &profiles.check_test
+    } else if mode == CompileMode::Build {
+        &profiles.bench
+    } else {
+        profile
+    };
+
     let targets = match *filter {
         CompileFilter::Default { required_features_filterable } => {
             let deps = if release {
@@ -634,15 +651,26 @@ 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)?);
+            // If --tests was specified, add all targets that would be
+            // generated by `cargo test`.
+            let test_filter = match tests {
+                FilterRule::All => Target::tested,
+                FilterRule::Just(_) => Target::is_test
+            };
             targets.append(&mut propose_indicated_targets(
-                pkg, tests, "test", Target::is_test, test)?);
+                pkg, tests, "test", test_filter, test_profile)?);
+            // If --benches was specified, add all targets that would be
+            // generated by `cargo bench`.
+            let bench_filter = match benches {
+                FilterRule::All => Target::benched,
+                FilterRule::Just(_) => Target::is_bench
+            };
             targets.append(&mut propose_indicated_targets(
-                pkg, benches, "bench", Target::is_bench, &profiles.bench)?);
+                pkg, benches, "bench", bench_filter, bench_profile)?);
             targets
         }
     };

src/cargo/ops/cargo_rustc/context.rs

@@ -876,7 +876,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                 Unit {
                     pkg: unit.pkg,
                     target: t,
-                    profile: self.lib_profile(),
+                    profile: self.lib_or_check_profile(unit, t),
                     kind: unit.kind.for_target(t),
                 }
             }));

src/cargo/util/toml/mod.rs

@@ -1059,6 +1059,8 @@ fn build_profiles(profiles: &Option<TomlProfiles>) -> 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

tests/bench.rs

@@ -81,6 +81,7 @@ fn bench_bench_implicit() {
                .with_stderr(format!("\
 [COMPILING] foo v0.0.1 ({dir})
 [FINISHED] release [optimized] target(s) in [..]
+[RUNNING] target[/]release[/]deps[/]foo-[..][EXE]
 [RUNNING] target[/]release[/]deps[/]mybench-[..][EXE]
 ", dir = p.url()))
                .with_stdout_contains("test run2 ... bench: [..]"));
@@ -1323,7 +1324,7 @@ fn bench_virtual_manifest_all_implied() {
         .build();
 
     // The order in which foo and bar are built is not guaranteed
-    
+
     assert_that(p.cargo("bench"),
                 execs().with_status(0)
                        .with_stderr_contains("\

tests/build.rs

@@ -3887,3 +3887,59 @@ fn uplift_dsym_of_bin_on_mac() {
     assert_that(&p.bin("c.dSYM"), is_not(existing_dir()));
     assert_that(&p.bin("d.dSYM"), is_not(existing_dir()));
 }
+
+// Make sure that `cargo build` chooses the correct profile for building
+// targets based on filters (assuming --profile is not specified).
+#[test]
+fn build_filter_infer_profile() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("src/lib.rs", "")
+        .file("src/main.rs", "fn main() {}")
+        .file("tests/t1.rs", "")
+        .file("benches/b1.rs", "")
+        .file("examples/ex1.rs", "fn main() {}")
+        .build();
+
+    assert_that(p.cargo("build").arg("-v"),
+        execs().with_status(0)
+        .with_stderr_contains("\
+            [RUNNING] `rustc --crate-name foo src[/]lib.rs --crate-type lib \
+            --emit=dep-info,link[..]")
+        .with_stderr_contains("\
+            [RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \
+            --emit=dep-info,link[..]")
+        );
+
+    p.root().join("target").rm_rf();
+    assert_that(p.cargo("build").arg("-v").arg("--test=t1"),
+        execs().with_status(0)
+        .with_stderr_contains("\
+            [RUNNING] `rustc --crate-name foo src[/]lib.rs --crate-type lib \
+            --emit=dep-info,link[..]")
+        .with_stderr_contains("\
+            [RUNNING] `rustc --crate-name t1 tests[/]t1.rs --emit=dep-info,link[..]")
+        .with_stderr_contains("\
+            [RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \
+            --emit=dep-info,link[..]")
+        );
+
+    p.root().join("target").rm_rf();
+    assert_that(p.cargo("build").arg("-v").arg("--bench=b1"),
+        execs().with_status(0)
+        .with_stderr_contains("\
+            [RUNNING] `rustc --crate-name foo src[/]lib.rs --crate-type lib \
+            --emit=dep-info,link[..]")
+        .with_stderr_contains("\
+            [RUNNING] `rustc --crate-name b1 benches[/]b1.rs --emit=dep-info,link \
+            -C opt-level=3[..]")
+        .with_stderr_contains("\
+            [RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \
+            --emit=dep-info,link[..]")
+        );
+}

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()}
 }