Auto merge of rust-lang#6824 - ehuss:improve-test-rerun-hint, r=alexc…

…richton

Improve error message to rerun a test in a workspace.

In a non-virtual workspace, if you run `cargo test --all` and something fails, it tells you to rerun `--test foo`, but if `foo` is not the default, then it won't work without a `-p` flag.  This tries to be a little more careful about when `-p` is needed in the hint.

src/bin/cargo/commands/test.rs

@@ -150,7 +150,10 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
     match err {
         None => Ok(()),
         Some(err) => Err(match err.exit.as_ref().and_then(|e| e.code()) {
-            Some(i) => CliError::new(failure::format_err!("{}", err.hint(&ws)), i),
+            Some(i) => CliError::new(
+                failure::format_err!("{}", err.hint(&ws, &ops.compile_opts)),
+                i,
+            ),
             None => CliError::new(err.into(), 101),
         }),
     }

src/cargo/ops/cargo_compile.rs

@@ -182,6 +182,17 @@ impl Packages {
         };
         Ok(packages)
     }
+
+    /// Returns whether or not the user needs to pass a `-p` flag to target a
+    /// specific package in the workspace.
+    pub fn needs_spec_flag(&self, ws: &Workspace<'_>) -> bool {
+        match self {
+            Packages::Default => ws.default_members().count() > 1,
+            Packages::All => ws.members().count() > 1,
+            Packages::Packages(_) => true,
+            Packages::OptOut(_) => true,
+        }
+    }
 }
 
 #[derive(Debug, PartialEq, Eq)]

src/cargo/util/errors.rs

@@ -10,6 +10,7 @@ use failure::{Context, Error, Fail};
 use log::trace;
 
 use crate::core::{TargetKind, Workspace};
+use crate::ops::CompileOptions;
 
 pub type CargoResult<T> = failure::Fallible<T>; // Alex's body isn't quite ready to give up "Result"
 
@@ -188,14 +189,14 @@ impl CargoTestError {
         }
     }
 
-    pub fn hint(&self, ws: &Workspace<'_>) -> String {
+    pub fn hint(&self, ws: &Workspace<'_>, opts: &CompileOptions<'_>) -> String {
         match self.test {
             Test::UnitTest {
                 ref kind,
                 ref name,
                 ref pkg_name,
             } => {
-                let pkg_info = if ws.members().count() > 1 && ws.is_virtual() {
+                let pkg_info = if opts.spec.needs_spec_flag(ws) {
                     format!("-p {} ", pkg_name)
                 } else {
                     String::new()

tests/testsuite/test.rs

@@ -3270,7 +3270,7 @@ fn test_hint_not_masked_by_doctest() {
 }
 
 #[test]
-fn test_hint_workspace() {
+fn test_hint_workspace_virtual() {
     let p = project()
         .file(
             "Cargo.toml",
@@ -3289,6 +3289,40 @@ fn test_hint_workspace() {
         .with_stderr_contains("[ERROR] test failed, to rerun pass '-p b --lib'")
         .with_status(101)
         .run();
+    p.cargo("test")
+        .cwd("b")
+        .with_stderr_contains("[ERROR] test failed, to rerun pass '--lib'")
+        .with_status(101)
+        .run();
+}
+
+#[test]
+fn test_hint_workspace_nonvirtual() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+
+            [workspace]
+            members = ["a"]
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .file("a/Cargo.toml", &basic_manifest("a", "0.1.0"))
+        .file("a/src/lib.rs", "#[test] fn t1() {assert!(false)}")
+        .build();
+
+    p.cargo("test --all")
+        .with_stderr_contains("[ERROR] test failed, to rerun pass '-p a --lib'")
+        .with_status(101)
+        .run();
+    p.cargo("test -p a")
+        .with_stderr_contains("[ERROR] test failed, to rerun pass '-p a --lib'")
+        .with_status(101)
+        .run();
 }
 
 #[test]