Skip to content

Commit 46924af

Browse files
Auto merge of #147674 - GuillaumeGomez:fix-compile_fail, r=<try>
[rustdoc] Correctly handle `should_panic` doctest attribute and fix `--no-run` test flag on the 2024 edition try-job: test-various
2 parents 6380899 + 9cacdda commit 46924af

File tree

18 files changed

+657
-283
lines changed

18 files changed

+657
-283
lines changed

library/std/src/error.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ use crate::fmt::{self, Write};
123123
/// the `Debug` output means `Report` is an ideal starting place for formatting errors returned
124124
/// from `main`.
125125
///
126-
/// ```should_panic
126+
/// ```
127127
/// #![feature(error_reporter)]
128128
/// use std::error::Report;
129129
/// # use std::error::Error;
@@ -154,10 +154,14 @@ use crate::fmt::{self, Write};
154154
/// # Err(SuperError { source: SuperErrorSideKick })
155155
/// # }
156156
///
157-
/// fn main() -> Result<(), Report<SuperError>> {
157+
/// fn run() -> Result<(), Report<SuperError>> {
158158
/// get_super_error()?;
159159
/// Ok(())
160160
/// }
161+
///
162+
/// fn main() {
163+
/// assert!(run().is_err());
164+
/// }
161165
/// ```
162166
///
163167
/// This example produces the following output:
@@ -170,7 +174,7 @@ use crate::fmt::{self, Write};
170174
/// output format. If you want to make sure your `Report`s are pretty printed and include backtrace
171175
/// you will need to manually convert and enable those flags.
172176
///
173-
/// ```should_panic
177+
/// ```
174178
/// #![feature(error_reporter)]
175179
/// use std::error::Report;
176180
/// # use std::error::Error;
@@ -201,12 +205,16 @@ use crate::fmt::{self, Write};
201205
/// # Err(SuperError { source: SuperErrorSideKick })
202206
/// # }
203207
///
204-
/// fn main() -> Result<(), Report<SuperError>> {
208+
/// fn run() -> Result<(), Report<SuperError>> {
205209
/// get_super_error()
206210
/// .map_err(Report::from)
207211
/// .map_err(|r| r.pretty(true).show_backtrace(true))?;
208212
/// Ok(())
209213
/// }
214+
///
215+
/// fn main() {
216+
/// assert!(run().is_err());
217+
/// }
210218
/// ```
211219
///
212220
/// This example produces the following output:

library/test/src/lib.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ pub mod test {
4545
pub use crate::cli::{TestOpts, parse_opts};
4646
pub use crate::helpers::metrics::{Metric, MetricMap};
4747
pub use crate::options::{Options, RunIgnored, RunStrategy, ShouldPanic};
48-
pub use crate::test_result::{TestResult, TrFailed, TrFailedMsg, TrIgnored, TrOk};
48+
pub use crate::test_result::{
49+
RustdocResult, TestResult, TrFailed, TrFailedMsg, TrIgnored, TrOk, get_rustdoc_result,
50+
};
4951
pub use crate::time::{TestExecTime, TestTimeOptions};
5052
pub use crate::types::{
5153
DynTestFn, DynTestName, StaticBenchFn, StaticTestFn, StaticTestName, TestDesc,
@@ -568,6 +570,10 @@ pub fn convert_benchmarks_to_tests(tests: Vec<TestDescAndFn>) -> Vec<TestDescAnd
568570
.collect()
569571
}
570572

573+
pub fn cannot_handle_should_panic() -> bool {
574+
(cfg!(target_family = "wasm") || cfg!(target_os = "zkvm")) && !cfg!(target_os = "emscripten")
575+
}
576+
571577
pub fn run_test(
572578
opts: &TestOpts,
573579
force_ignore: bool,
@@ -579,9 +585,8 @@ pub fn run_test(
579585
let TestDescAndFn { desc, testfn } = test;
580586

581587
// Emscripten can catch panics but other wasm targets cannot
582-
let ignore_because_no_process_support = desc.should_panic != ShouldPanic::No
583-
&& (cfg!(target_family = "wasm") || cfg!(target_os = "zkvm"))
584-
&& !cfg!(target_os = "emscripten");
588+
let ignore_because_no_process_support =
589+
desc.should_panic != ShouldPanic::No && cannot_handle_should_panic();
585590

586591
if force_ignore || desc.ignore || ignore_because_no_process_support {
587592
let message = CompletedTest::new(id, desc, TrIgnored, None, Vec::new());

library/test/src/test_result.rs

Lines changed: 109 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use std::any::Any;
22
#[cfg(unix)]
33
use std::os::unix::process::ExitStatusExt;
4-
use std::process::ExitStatus;
4+
use std::process::{ExitStatus, Output};
5+
use std::{fmt, io};
56

67
pub use self::TestResult::*;
78
use super::bench::BenchSamples;
@@ -103,15 +104,14 @@ pub(crate) fn calc_result(
103104
result
104105
}
105106

106-
/// Creates a `TestResult` depending on the exit code of test subprocess.
107-
pub(crate) fn get_result_from_exit_code(
108-
desc: &TestDesc,
107+
/// Creates a `TestResult` depending on the exit code of test subprocess
108+
pub(crate) fn get_result_from_exit_code_inner(
109109
status: ExitStatus,
110-
time_opts: Option<&time::TestTimeOptions>,
111-
exec_time: Option<&time::TestExecTime>,
110+
success_error_code: i32,
112111
) -> TestResult {
113-
let result = match status.code() {
114-
Some(TR_OK) => TestResult::TrOk,
112+
match status.code() {
113+
Some(error_code) if error_code == success_error_code => TestResult::TrOk,
114+
Some(crate::ERROR_EXIT_CODE) => TestResult::TrFailed,
115115
#[cfg(windows)]
116116
Some(STATUS_FAIL_FAST_EXCEPTION) => TestResult::TrFailed,
117117
#[cfg(unix)]
@@ -131,7 +131,17 @@ pub(crate) fn get_result_from_exit_code(
131131
Some(code) => TestResult::TrFailedMsg(format!("got unexpected return code {code}")),
132132
#[cfg(not(any(windows, unix)))]
133133
Some(_) => TestResult::TrFailed,
134-
};
134+
}
135+
}
136+
137+
/// Creates a `TestResult` depending on the exit code of test subprocess and on its runtime.
138+
pub(crate) fn get_result_from_exit_code(
139+
desc: &TestDesc,
140+
status: ExitStatus,
141+
time_opts: Option<&time::TestTimeOptions>,
142+
exec_time: Option<&time::TestExecTime>,
143+
) -> TestResult {
144+
let result = get_result_from_exit_code_inner(status, TR_OK);
135145

136146
// If test is already failed (or allowed to fail), do not change the result.
137147
if result != TestResult::TrOk {
@@ -147,3 +157,93 @@ pub(crate) fn get_result_from_exit_code(
147157

148158
result
149159
}
160+
161+
#[derive(Debug)]
162+
pub enum RustdocResult {
163+
/// The test failed to compile.
164+
CompileError,
165+
/// The test is marked `compile_fail` but compiled successfully.
166+
UnexpectedCompilePass,
167+
/// The test failed to compile (as expected) but the compiler output did not contain all
168+
/// expected error codes.
169+
MissingErrorCodes(Vec<String>),
170+
/// The test binary was unable to be executed.
171+
ExecutionError(io::Error),
172+
/// The test binary exited with a non-zero exit code.
173+
///
174+
/// This typically means an assertion in the test failed or another form of panic occurred.
175+
ExecutionFailure(Output),
176+
/// The test is marked `should_panic` but the test binary executed successfully.
177+
NoPanic(Option<String>),
178+
}
179+
180+
impl fmt::Display for RustdocResult {
181+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
182+
match self {
183+
Self::CompileError => {
184+
write!(f, "Couldn't compile the test.")
185+
}
186+
Self::UnexpectedCompilePass => {
187+
write!(f, "Test compiled successfully, but it's marked `compile_fail`.")
188+
}
189+
Self::NoPanic(msg) => {
190+
write!(f, "Test didn't panic, but it's marked `should_panic`")?;
191+
if let Some(msg) = msg {
192+
write!(f, " ({msg})")?;
193+
}
194+
f.write_str(".")
195+
}
196+
Self::MissingErrorCodes(codes) => {
197+
write!(f, "Some expected error codes were not found: {codes:?}")
198+
}
199+
Self::ExecutionError(err) => {
200+
write!(f, "Couldn't run the test: {err}")?;
201+
if err.kind() == io::ErrorKind::PermissionDenied {
202+
f.write_str(" - maybe your tempdir is mounted with noexec?")?;
203+
}
204+
Ok(())
205+
}
206+
Self::ExecutionFailure(out) => {
207+
writeln!(f, "Test executable failed ({reason}).", reason = out.status)?;
208+
209+
// FIXME(#12309): An unfortunate side-effect of capturing the test
210+
// executable's output is that the relative ordering between the test's
211+
// stdout and stderr is lost. However, this is better than the
212+
// alternative: if the test executable inherited the parent's I/O
213+
// handles the output wouldn't be captured at all, even on success.
214+
//
215+
// The ordering could be preserved if the test process' stderr was
216+
// redirected to stdout, but that functionality does not exist in the
217+
// standard library, so it may not be portable enough.
218+
let stdout = str::from_utf8(&out.stdout).unwrap_or_default();
219+
let stderr = str::from_utf8(&out.stderr).unwrap_or_default();
220+
221+
if !stdout.is_empty() || !stderr.is_empty() {
222+
writeln!(f)?;
223+
224+
if !stdout.is_empty() {
225+
writeln!(f, "stdout:\n{stdout}")?;
226+
}
227+
228+
if !stderr.is_empty() {
229+
writeln!(f, "stderr:\n{stderr}")?;
230+
}
231+
}
232+
Ok(())
233+
}
234+
}
235+
}
236+
}
237+
238+
pub fn get_rustdoc_result(output: Output, should_panic: bool) -> Result<(), RustdocResult> {
239+
let result = get_result_from_exit_code_inner(output.status, 0);
240+
match (result, should_panic) {
241+
(TestResult::TrFailed, true) | (TestResult::TrOk, false) => Ok(()),
242+
(TestResult::TrOk, true) => Err(RustdocResult::NoPanic(None)),
243+
(TestResult::TrFailedMsg(msg), true) => Err(RustdocResult::NoPanic(Some(msg))),
244+
(TestResult::TrFailedMsg(_) | TestResult::TrFailed, false) => {
245+
Err(RustdocResult::ExecutionFailure(output))
246+
}
247+
_ => unreachable!("unexpected status for rustdoc test output"),
248+
}
249+
}

0 commit comments

Comments
 (0)