Rollup merge of rust-lang#60549 - euclio:doctest-panic-messages, r=GuillaumeGomez

do not print panic message on doctest failures

This PR cleans up rustdoc test output by silently unwinding on failure instead of using `panic!`. It also improves the clarity and consistency of the output on test failure, and adds test cases for failure modes that were previously untested.

Author: Centril

src/librustdoc/test.rs

@@ -17,7 +17,7 @@ use std::io::prelude::*;
 use std::io;
 use std::panic::{self, AssertUnwindSafe};
 use std::path::PathBuf;
-use std::process::Command;
+use std::process::{self, Command};
 use std::str;
 use std::sync::{Arc, Mutex};
 use syntax::symbol::sym;
@@ -160,13 +160,45 @@ fn scrape_test_config(krate: &::rustc::hir::Crate) -> TestOptions {
     opts
 }
 
-fn run_test(test: &str, cratename: &str, filename: &FileName, line: usize,
-            cfgs: Vec<String>, libs: Vec<SearchPath>,
-            cg: CodegenOptions, externs: Externs,
-            should_panic: bool, no_run: bool, as_test_harness: bool,
-            compile_fail: bool, mut error_codes: Vec<String>, opts: &TestOptions,
-            maybe_sysroot: Option<PathBuf>, linker: Option<PathBuf>, edition: Edition,
-            persist_doctests: Option<PathBuf>) {
+/// Documentation test failure modes.
+enum TestFailure {
+    /// The test failed to compile.
+    CompileError,
+    /// The test is marked `compile_fail` but compiled successfully.
+    UnexpectedCompilePass,
+    /// The test failed to compile (as expected) but the compiler output did not contain all
+    /// expected error codes.
+    MissingErrorCodes(Vec<String>),
+    /// The test binary was unable to be executed.
+    ExecutionError(io::Error),
+    /// The test binary exited with a non-zero exit code.
+    ///
+    /// This typically means an assertion in the test failed or another form of panic occurred.
+    ExecutionFailure(process::Output),
+    /// The test is marked `should_panic` but the test binary executed successfully.
+    UnexpectedRunPass,
+}
+
+fn run_test(
+    test: &str,
+    cratename: &str,
+    filename: &FileName,
+    line: usize,
+    cfgs: Vec<String>,
+    libs: Vec<SearchPath>,
+    cg: CodegenOptions,
+    externs: Externs,
+    should_panic: bool,
+    no_run: bool,
+    as_test_harness: bool,
+    compile_fail: bool,
+    mut error_codes: Vec<String>,
+    opts: &TestOptions,
+    maybe_sysroot: Option<PathBuf>,
+    linker: Option<PathBuf>,
+    edition: Edition,
+    persist_doctests: Option<PathBuf>,
+) -> Result<(), TestFailure> {
     let (test, line_offset) = match panic::catch_unwind(|| {
         make_test(test, Some(cratename), as_test_harness, opts, edition)
     }) {
@@ -307,44 +339,43 @@ fn run_test(test: &str, cratename: &str, filename: &FileName, line: usize,
 
     match (compile_result, compile_fail) {
         (Ok(()), true) => {
-            panic!("test compiled while it wasn't supposed to")
+            return Err(TestFailure::UnexpectedCompilePass);
         }
         (Ok(()), false) => {}
         (Err(_), true) => {
-            if error_codes.len() > 0 {
+            if !error_codes.is_empty() {
                 let out = String::from_utf8(data.lock().unwrap().to_vec()).unwrap();
                 error_codes.retain(|err| !out.contains(err));
+
+                if !error_codes.is_empty() {
+                    return Err(TestFailure::MissingErrorCodes(error_codes));
+                }
             }
         }
         (Err(_), false) => {
-            panic!("couldn't compile the test")
+            return Err(TestFailure::CompileError);
         }
     }
 
-    if error_codes.len() > 0 {
-        panic!("Some expected error codes were not found: {:?}", error_codes);
+    if no_run {
+        return Ok(());
     }
 
-    if no_run { return }
-
     // Run the code!
     let mut cmd = Command::new(output_file);
 
     match cmd.output() {
-        Err(e) => panic!("couldn't run the test: {}{}", e,
-                        if e.kind() == io::ErrorKind::PermissionDenied {
-                            " - maybe your tempdir is mounted with noexec?"
-                        } else { "" }),
+        Err(e) => return Err(TestFailure::ExecutionError(e)),
         Ok(out) => {
             if should_panic && out.status.success() {
-                panic!("test executable succeeded when it should have failed");
+                return Err(TestFailure::UnexpectedRunPass);
             } else if !should_panic && !out.status.success() {
-                panic!("test executable failed:\n{}\n{}\n",
-                       str::from_utf8(&out.stdout).unwrap_or(""),
-                       str::from_utf8(&out.stderr).unwrap_or(""));
+                return Err(TestFailure::ExecutionFailure(out));
             }
         }
     }
+
+    Ok(())
 }
 
 /// Transforms a test into code that can be compiled into a Rust binary, and returns the number of
@@ -711,7 +742,7 @@ impl Tester for Collector {
                 allow_fail: config.allow_fail,
             },
             testfn: testing::DynTestFn(box move || {
-                run_test(
+                let res = run_test(
                     &test,
                     &cratename,
                     &filename,
@@ -730,7 +761,65 @@ impl Tester for Collector {
                     linker,
                     edition,
                     persist_doctests
-                )
+                );
+
+                if let Err(err) = res {
+                    match err {
+                        TestFailure::CompileError => {
+                            eprint!("Couldn't compile the test.");
+                        }
+                        TestFailure::UnexpectedCompilePass => {
+                            eprint!("Test compiled successfully, but it's marked `compile_fail`.");
+                        }
+                        TestFailure::UnexpectedRunPass => {
+                            eprint!("Test executable succeeded, but it's marked `should_panic`.");
+                        }
+                        TestFailure::MissingErrorCodes(codes) => {
+                            eprint!("Some expected error codes were not found: {:?}", codes);
+                        }
+                        TestFailure::ExecutionError(err) => {
+                            eprint!("Couldn't run the test: {}", err);
+                            if err.kind() == io::ErrorKind::PermissionDenied {
+                                eprint!(" - maybe your tempdir is mounted with noexec?");
+                            }
+                        }
+                        TestFailure::ExecutionFailure(out) => {
+                            let reason = if let Some(code) = out.status.code() {
+                                format!("exit code {}", code)
+                            } else {
+                                String::from("terminated by signal")
+                            };
+
+                            eprintln!("Test executable failed ({}).", reason);
+
+                            // FIXME(#12309): An unfortunate side-effect of capturing the test
+                            // executable's output is that the relative ordering between the test's
+                            // stdout and stderr is lost. However, this is better than the
+                            // alternative: if the test executable inherited the parent's I/O
+                            // handles the output wouldn't be captured at all, even on success.
+                            //
+                            // The ordering could be preserved if the test process' stderr was
+                            // redirected to stdout, but that functionality does not exist in the
+                            // standard library, so it may not be portable enough.
+                            let stdout = str::from_utf8(&out.stdout).unwrap_or_default();
+                            let stderr = str::from_utf8(&out.stderr).unwrap_or_default();
+
+                            if !stdout.is_empty() || !stderr.is_empty() {
+                                eprintln!();
+
+                                if !stdout.is_empty() {
+                                    eprintln!("stdout:\n{}", stdout);
+                                }
+
+                                if !stderr.is_empty() {
+                                    eprintln!("stderr:\n{}", stderr);
+                                }
+                            }
+                        }
+                    }
+
+                    panic::resume_unwind(box ());
+                }
             }),
         });
     }

src/test/rustdoc-ui/failed-doctest-compile-fail.rs

@@ -0,0 +1,11 @@
+// FIXME: if/when the output of the test harness can be tested on its own, this test should be
+// adapted to use that, and that normalize line can go away
+
+// compile-flags:--test
+// normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR"
+// failure-status: 101
+
+/// ```compile_fail
+/// println!("Hello");
+/// ```
+pub struct Foo;

src/test/rustdoc-ui/failed-doctest-compile-fail.stdout

@@ -0,0 +1,14 @@
+
+running 1 test
+test $DIR/failed-doctest-compile-fail.rs - Foo (line 8) ... FAILED
+
+failures:
+
+---- $DIR/failed-doctest-compile-fail.rs - Foo (line 8) stdout ----
+Test compiled successfully, but it's marked `compile_fail`.
+
+failures:
+    $DIR/failed-doctest-compile-fail.rs - Foo (line 8)
+
+test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
+

src/test/rustdoc-ui/failed-doctest-missing-codes.rs

@@ -0,0 +1,11 @@
+// FIXME: if/when the output of the test harness can be tested on its own, this test should be
+// adapted to use that, and that normalize line can go away
+
+// compile-flags:--test
+// normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR"
+// failure-status: 101
+
+/// ```compile_fail,E0004
+/// let x: () = 5i32;
+/// ```
+pub struct Foo;

src/test/rustdoc-ui/failed-doctest-missing-codes.stdout

@@ -0,0 +1,26 @@
+
+running 1 test
+test $DIR/failed-doctest-missing-codes.rs - Foo (line 8) ... FAILED
+
+failures:
+
+---- $DIR/failed-doctest-missing-codes.rs - Foo (line 8) stdout ----
+error[E0308]: mismatched types
+ --> $DIR/failed-doctest-missing-codes.rs:9:13
+  |
+3 | let x: () = 5i32;
+  |             ^^^^ expected (), found i32
+  |
+  = note: expected type `()`
+             found type `i32`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0308`.
+Some expected error codes were not found: ["E0004"]
+
+failures:
+    $DIR/failed-doctest-missing-codes.rs - Foo (line 8)
+
+test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
+

src/test/rustdoc-ui/failed-doctest-output.rs

@@ -5,10 +5,13 @@
 // compile-flags:--test
 // normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR"
 // failure-status: 101
-// rustc-env:RUST_BACKTRACE=0
 
 // doctest fails at runtime
 /// ```
+/// println!("stdout 1");
+/// eprintln!("stderr 1");
+/// println!("stdout 2");
+/// eprintln!("stderr 2");
 /// panic!("oh no");
 /// ```
 pub struct SomeStruct;

src/test/rustdoc-ui/failed-doctest-output.stdout

@@ -1,35 +1,39 @@
 
 running 2 tests
-test $DIR/failed-doctest-output.rs - OtherStruct (line 17) ... FAILED
-test $DIR/failed-doctest-output.rs - SomeStruct (line 11) ... FAILED
+test $DIR/failed-doctest-output.rs - OtherStruct (line 20) ... FAILED
+test $DIR/failed-doctest-output.rs - SomeStruct (line 10) ... FAILED
 
 failures:
 
----- $DIR/failed-doctest-output.rs - OtherStruct (line 17) stdout ----
+---- $DIR/failed-doctest-output.rs - OtherStruct (line 20) stdout ----
 error[E0425]: cannot find value `no` in this scope
- --> $DIR/failed-doctest-output.rs:18:1
+ --> $DIR/failed-doctest-output.rs:21:1
   |
 3 | no
   | ^^ not found in this scope
 
 error: aborting due to previous error
 
 For more information about this error, try `rustc --explain E0425`.
-thread '$DIR/failed-doctest-output.rs - OtherStruct (line 17)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:320:13
+Couldn't compile the test.
+---- $DIR/failed-doctest-output.rs - SomeStruct (line 10) stdout ----
+Test executable failed (exit code 101).
+
+stdout:
+stdout 1
+stdout 2
+
+stderr:
+stderr 1
+stderr 2
+thread 'main' panicked at 'oh no', $DIR/failed-doctest-output.rs:7:1
 note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
 
----- $DIR/failed-doctest-output.rs - SomeStruct (line 11) stdout ----
-thread '$DIR/failed-doctest-output.rs - SomeStruct (line 11)' panicked at 'test executable failed:
-
-thread 'main' panicked at 'oh no', $DIR/failed-doctest-output.rs:3:1
-note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
-
-', src/librustdoc/test.rs:342:17
 
 
 failures:
-    $DIR/failed-doctest-output.rs - OtherStruct (line 17)
-    $DIR/failed-doctest-output.rs - SomeStruct (line 11)
+    $DIR/failed-doctest-output.rs - OtherStruct (line 20)
+    $DIR/failed-doctest-output.rs - SomeStruct (line 10)
 
 test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out
 

src/test/rustdoc-ui/failed-doctest-should-panic.rs

@@ -0,0 +1,11 @@
+// FIXME: if/when the output of the test harness can be tested on its own, this test should be
+// adapted to use that, and that normalize line can go away
+
+// compile-flags:--test
+// normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR"
+// failure-status: 101
+
+/// ```should_panic
+/// println!("Hello, world!");
+/// ```
+pub struct Foo;

src/test/rustdoc-ui/failed-doctest-should-panic.stdout

@@ -0,0 +1,14 @@
+
+running 1 test
+test $DIR/failed-doctest-should-panic.rs - Foo (line 8) ... FAILED
+
+failures:
+
+---- $DIR/failed-doctest-should-panic.rs - Foo (line 8) stdout ----
+Test executable succeeded, but it's marked `should_panic`.
+
+failures:
+    $DIR/failed-doctest-should-panic.rs - Foo (line 8)
+
+test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
+

src/test/rustdoc-ui/unparseable-doc-test.stdout

@@ -13,9 +13,7 @@ error: unterminated double quote string
 
 error: aborting due to previous error
 
-thread '$DIR/unparseable-doc-test.rs - foo (line 6)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:320:13
-note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
-
+Couldn't compile the test.
 
 failures:
     $DIR/unparseable-doc-test.rs - foo (line 6)