Skip to content

Commit 89d437e

Browse files
euclioeuclio
committed
do not print panic message on doctest failures
1 parent 37ff5d3 commit 89d437e

10 files changed

+224
-43
lines changed

‎src/librustdoc/test.rs‎

Lines changed: 114 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use std::io::prelude::*;
1717
use std::io;
1818
use std::panic::{self, AssertUnwindSafe};
1919
use std::path::PathBuf;
20-
use std::process::Command;
20+
use std::process::{self, Command};
2121
use std::str;
2222
use std::sync::{Arc, Mutex};
2323
use syntax::symbol::sym;
@@ -160,13 +160,45 @@ fn scrape_test_config(krate: &::rustc::hir::Crate) -> TestOptions {
160160
opts
161161
}
162162

163-
fn run_test(test: &str, cratename: &str, filename: &FileName, line: usize,
164-
cfgs: Vec<String>, libs: Vec<SearchPath>,
165-
cg: CodegenOptions, externs: Externs,
166-
should_panic: bool, no_run: bool, as_test_harness: bool,
167-
compile_fail: bool, mut error_codes: Vec<String>, opts: &TestOptions,
168-
maybe_sysroot: Option<PathBuf>, linker: Option<PathBuf>, edition: Edition,
169-
persist_doctests: Option<PathBuf>) {
163+
/// Documentation test failure modes.
164+
enum TestFailure {
165+
/// The test failed to compile.
166+
CompileError,
167+
/// The test is marked `compile_fail` but compiled successfully.
168+
UnexpectedCompilePass,
169+
/// The test failed to compile (as expected) but the compiler output did not contain all
170+
/// expected error codes.
171+
MissingErrorCodes(Vec<String>),
172+
/// The test binary was unable to be executed.
173+
ExecutionError(io::Error),
174+
/// The test binary exited with a non-zero exit code.
175+
///
176+
/// This typically means an assertion in the test failed or another form of panic occurred.
177+
ExecutionFailure(process::Output),
178+
/// The test is marked `should_panic` but the test binary executed successfully.
179+
UnexpectedRunPass,
180+
}
181+
182+
fn run_test(
183+
test: &str,
184+
cratename: &str,
185+
filename: &FileName,
186+
line: usize,
187+
cfgs: Vec<String>,
188+
libs: Vec<SearchPath>,
189+
cg: CodegenOptions,
190+
externs: Externs,
191+
should_panic: bool,
192+
no_run: bool,
193+
as_test_harness: bool,
194+
compile_fail: bool,
195+
mut error_codes: Vec<String>,
196+
opts: &TestOptions,
197+
maybe_sysroot: Option<PathBuf>,
198+
linker: Option<PathBuf>,
199+
edition: Edition,
200+
persist_doctests: Option<PathBuf>,
201+
) -> Result<(), TestFailure> {
170202
let (test, line_offset) = match panic::catch_unwind(|| {
171203
make_test(test, Some(cratename), as_test_harness, opts, edition)
172204
}) {
@@ -307,44 +339,43 @@ fn run_test(test: &str, cratename: &str, filename: &FileName, line: usize,
307339

308340
match (compile_result, compile_fail) {
309341
(Ok(()), true) => {
310-
panic!("test compiled while it wasn't supposed to")
342+
return Err(TestFailure::UnexpectedCompilePass);
311343
}
312344
(Ok(()), false) => {}
313345
(Err(_), true) => {
314-
if error_codes.len() > 0 {
346+
if !error_codes.is_empty() {
315347
let out = String::from_utf8(data.lock().unwrap().to_vec()).unwrap();
316348
error_codes.retain(|err| !out.contains(err));
349+
350+
if !error_codes.is_empty() {
351+
return Err(TestFailure::MissingErrorCodes(error_codes));
352+
}
317353
}
318354
}
319355
(Err(_), false) => {
320-
panic!("couldn't compile the test")
356+
return Err(TestFailure::CompileError);
321357
}
322358
}
323359

324-
if error_codes.len() > 0 {
325-
panic!("Some expected error codes were not found: {:?}", error_codes);
360+
if no_run {
361+
return Ok(());
326362
}
327363

328-
if no_run { return }
329-
330364
// Run the code!
331365
let mut cmd = Command::new(output_file);
332366

333367
match cmd.output() {
334-
Err(e) => panic!("couldn't run the test: {}{}", e,
335-
if e.kind() == io::ErrorKind::PermissionDenied {
336-
" - maybe your tempdir is mounted with noexec?"
337-
} else { "" }),
368+
Err(e) => return Err(TestFailure::ExecutionError(e)),
338369
Ok(out) => {
339370
if should_panic && out.status.success() {
340-
panic!("test executable succeeded when it should have failed");
371+
return Err(TestFailure::UnexpectedRunPass);
341372
} else if !should_panic && !out.status.success() {
342-
panic!("test executable failed:\n{}\n{}\n",
343-
str::from_utf8(&out.stdout).unwrap_or(""),
344-
str::from_utf8(&out.stderr).unwrap_or(""));
373+
return Err(TestFailure::ExecutionFailure(out));
345374
}
346375
}
347376
}
377+
378+
Ok(())
348379
}
349380

350381
/// 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 {
711742
allow_fail: config.allow_fail,
712743
},
713744
testfn: testing::DynTestFn(box move || {
714-
run_test(
745+
let res = run_test(
715746
&test,
716747
&cratename,
717748
&filename,
@@ -730,7 +761,65 @@ impl Tester for Collector {
730761
linker,
731762
edition,
732763
persist_doctests
733-
)
764+
);
765+
766+
if let Err(err) = res {
767+
match err {
768+
TestFailure::CompileError => {
769+
eprint!("Couldn't compile the test.");
770+
}
771+
TestFailure::UnexpectedCompilePass => {
772+
eprint!("Test compiled successfully, but it's marked `compile_fail`.");
773+
}
774+
TestFailure::UnexpectedRunPass => {
775+
eprint!("Test executable succeeded, but it's marked `should_panic`.");
776+
}
777+
TestFailure::MissingErrorCodes(codes) => {
778+
eprint!("Some expected error codes were not found: {:?}", codes);
779+
}
780+
TestFailure::ExecutionError(err) => {
781+
eprint!("Couldn't run the test: {}", err);
782+
if err.kind() == io::ErrorKind::PermissionDenied {
783+
eprint!(" - maybe your tempdir is mounted with noexec?");
784+
}
785+
}
786+
TestFailure::ExecutionFailure(out) => {
787+
let reason = if let Some(code) = out.status.code() {
788+
format!("exit code {}", code)
789+
} else {
790+
String::from("terminated by signal")
791+
};
792+
793+
eprintln!("Test executable failed ({}).", reason);
794+
795+
// FIXME(#12309): An unfortunate side-effect of capturing the test
796+
// executable's output is that the relative ordering between the test's
797+
// stdout and stderr is lost. However, this is better than the
798+
// alternative: if the test executable inherited the parent's I/O
799+
// handles the output wouldn't be captured at all, even on success.
800+
//
801+
// The ordering could be preserved if the test process' stderr was
802+
// redirected to stdout, but that functionality does not exist in the
803+
// standard library, so it may not be portable enough.
804+
let stdout = str::from_utf8(&out.stdout).unwrap_or_default();
805+
let stderr = str::from_utf8(&out.stderr).unwrap_or_default();
806+
807+
if !stdout.is_empty() || !stderr.is_empty() {
808+
eprintln!();
809+
810+
if !stdout.is_empty() {
811+
eprintln!("stdout:\n{}", stdout);
812+
}
813+
814+
if !stderr.is_empty() {
815+
eprintln!("stderr:\n{}", stderr);
816+
}
817+
}
818+
}
819+
}
820+
821+
panic::resume_unwind(box ());
822+
}
734823
}),
735824
});
736825
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// FIXME: if/when the output of the test harness can be tested on its own, this test should be
2+
// adapted to use that, and that normalize line can go away
3+
4+
// compile-flags:--test
5+
// normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR"
6+
// failure-status: 101
7+
8+
/// ```compile_fail
9+
/// println!("Hello");
10+
/// ```
11+
pub struct Foo;

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
2+
running 1 test
3+
test $DIR/failed-doctest-compile-fail.rs - Foo (line 8) ... FAILED
4+
5+
failures:
6+
7+
---- $DIR/failed-doctest-compile-fail.rs - Foo (line 8) stdout ----
8+
Test compiled successfully, but it's marked `compile_fail`.
9+
10+
failures:
11+
$DIR/failed-doctest-compile-fail.rs - Foo (line 8)
12+
13+
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
14+

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// FIXME: if/when the output of the test harness can be tested on its own, this test should be
2+
// adapted to use that, and that normalize line can go away
3+
4+
// compile-flags:--test
5+
// normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR"
6+
// failure-status: 101
7+
8+
/// ```compile_fail,E0004
9+
/// let x: () = 5i32;
10+
/// ```
11+
pub struct Foo;

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
2+
running 1 test
3+
test $DIR/failed-doctest-missing-codes.rs - Foo (line 8) ... FAILED
4+
5+
failures:
6+
7+
---- $DIR/failed-doctest-missing-codes.rs - Foo (line 8) stdout ----
8+
error[E0308]: mismatched types
9+
--> $DIR/failed-doctest-missing-codes.rs:9:13
10+
|
11+
3 | let x: () = 5i32;
12+
| ^^^^ expected (), found i32
13+
|
14+
= note: expected type `()`
15+
found type `i32`
16+
17+
error: aborting due to previous error
18+
19+
For more information about this error, try `rustc --explain E0308`.
20+
Some expected error codes were not found: ["E0004"]
21+
22+
failures:
23+
$DIR/failed-doctest-missing-codes.rs - Foo (line 8)
24+
25+
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
26+

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@
55
// compile-flags:--test
66
// normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR"
77
// failure-status: 101
8-
// rustc-env:RUST_BACKTRACE=0
98

109
// doctest fails at runtime
1110
/// ```
11+
/// println!("stdout 1");
12+
/// eprintln!("stderr 1");
13+
/// println!("stdout 2");
14+
/// eprintln!("stderr 2");
1215
/// panic!("oh no");
1316
/// ```
1417
pub struct SomeStruct;

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

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,39 @@
11

22
running 2 tests
3-
test $DIR/failed-doctest-output.rs - OtherStruct (line 17) ... FAILED
4-
test $DIR/failed-doctest-output.rs - SomeStruct (line 11) ... FAILED
3+
test $DIR/failed-doctest-output.rs - OtherStruct (line 20) ... FAILED
4+
test $DIR/failed-doctest-output.rs - SomeStruct (line 10) ... FAILED
55

66
failures:
77

8-
---- $DIR/failed-doctest-output.rs - OtherStruct (line 17) stdout ----
8+
---- $DIR/failed-doctest-output.rs - OtherStruct (line 20) stdout ----
99
error[E0425]: cannot find value `no` in this scope
10-
--> $DIR/failed-doctest-output.rs:18:1
10+
--> $DIR/failed-doctest-output.rs:21:1
1111
|
1212
3 | no
1313
| ^^ not found in this scope
1414

1515
error: aborting due to previous error
1616

1717
For more information about this error, try `rustc --explain E0425`.
18-
thread '$DIR/failed-doctest-output.rs - OtherStruct (line 17)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:320:13
18+
Couldn't compile the test.
19+
---- $DIR/failed-doctest-output.rs - SomeStruct (line 10) stdout ----
20+
Test executable failed (exit code 101).
21+
22+
stdout:
23+
stdout 1
24+
stdout 2
25+
26+
stderr:
27+
stderr 1
28+
stderr 2
29+
thread 'main' panicked at 'oh no', $DIR/failed-doctest-output.rs:7:1
1930
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
2031

21-
---- $DIR/failed-doctest-output.rs - SomeStruct (line 11) stdout ----
22-
thread '$DIR/failed-doctest-output.rs - SomeStruct (line 11)' panicked at 'test executable failed:
23-
24-
thread 'main' panicked at 'oh no', $DIR/failed-doctest-output.rs:3:1
25-
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
26-
27-
', src/librustdoc/test.rs:342:17
2832

2933

3034
failures:
31-
$DIR/failed-doctest-output.rs - OtherStruct (line 17)
32-
$DIR/failed-doctest-output.rs - SomeStruct (line 11)
35+
$DIR/failed-doctest-output.rs - OtherStruct (line 20)
36+
$DIR/failed-doctest-output.rs - SomeStruct (line 10)
3337

3438
test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out
3539

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// FIXME: if/when the output of the test harness can be tested on its own, this test should be
2+
// adapted to use that, and that normalize line can go away
3+
4+
// compile-flags:--test
5+
// normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR"
6+
// failure-status: 101
7+
8+
/// ```should_panic
9+
/// println!("Hello, world!");
10+
/// ```
11+
pub struct Foo;

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
2+
running 1 test
3+
test $DIR/failed-doctest-should-panic.rs - Foo (line 8) ... FAILED
4+
5+
failures:
6+
7+
---- $DIR/failed-doctest-should-panic.rs - Foo (line 8) stdout ----
8+
Test executable succeeded, but it's marked `should_panic`.
9+
10+
failures:
11+
$DIR/failed-doctest-should-panic.rs - Foo (line 8)
12+
13+
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
14+

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,7 @@ error: unterminated double quote string
1313

1414
error: aborting due to previous error
1515

16-
thread '$DIR/unparseable-doc-test.rs - foo (line 6)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:320:13
17-
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
18-
16+
Couldn't compile the test.
1917

2018
failures:
2119
$DIR/unparseable-doc-test.rs - foo (line 6)

0 commit comments

Comments
 (0)