Skip to content

Commit

Permalink
Auto merge of rust-lang#120326 - tmandry:abort-in-tests, r=<try>
Browse files Browse the repository at this point in the history
Actually abort in panic-abort-tests

When a test fails in panic=abort, it can be useful to have a debugger or other tooling hook into the `abort()` call for debugging.

There's no reason we couldn't have done this in the first place; using a single exit code for "success" or "failed" was just simpler. Now we are aware of the special exit codes for posix and windows platforms, logging a special error if an unrecognized code is used on those platforms, and falling back to just "failure" on other platforms.

This continues to account for `#[should_panic]` inside the test process itself, so there's no risk of misinterpreting a random call to `abort()` as an expected panic. Any exit code besides `TR_OK` is logged as a test failure.
  • Loading branch information
bors committed Jan 25, 2024
2 parents 7ffc697 + bb332f3 commit 11f3f41
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 41 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,11 @@ jobs:
env:
CODEGEN_BACKENDS: "llvm,cranelift"
os: ubuntu-20.04-16core-64gb
- name: x86_64-msvc
env:
RUST_CONFIGURE_ARGS: "--build=x86_64-pc-windows-msvc --enable-profiler"
SCRIPT: make ci-msvc
os: windows-2019-8core-32gb
timeout-minutes: 600
runs-on: "${{ matrix.os }}"
steps:
Expand Down
20 changes: 0 additions & 20 deletions library/test/src/helpers/exit_code.rs

This file was deleted.

1 change: 0 additions & 1 deletion library/test/src/helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,5 @@
//! but used in `libtest`.

pub mod concurrency;
pub mod exit_code;
pub mod metrics;
pub mod shuffle;
15 changes: 2 additions & 13 deletions library/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ mod tests;
use core::any::Any;
use event::{CompletedTest, TestEvent};
use helpers::concurrency::get_concurrency;
use helpers::exit_code::get_exit_code;
use helpers::shuffle::{get_shuffle_seed, shuffle_tests};
use options::RunStrategy;
use test_result::*;
Expand Down Expand Up @@ -712,17 +711,7 @@ fn spawn_test_subprocess(
formatters::write_stderr_delimiter(&mut test_output, &desc.name);
test_output.extend_from_slice(&stderr);

let result = match (|| -> Result<TestResult, String> {
let exit_code = get_exit_code(status)?;
Ok(get_result_from_exit_code(&desc, exit_code, &time_opts, &exec_time))
})() {
Ok(r) => r,
Err(e) => {
write!(&mut test_output, "Unexpected error: {e}").unwrap();
TrFailed
}
};

let result = get_result_from_exit_code(&desc, status, &time_opts, &exec_time);
(result, test_output, exec_time)
})();

Expand Down Expand Up @@ -751,7 +740,7 @@ fn run_test_in_spawned_subprocess(desc: TestDesc, runnable_test: RunnableTest) -
if let TrOk = test_result {
process::exit(test_result::TR_OK);
} else {
process::exit(test_result::TR_FAILED);
process::abort();
}
});
let record_result2 = record_result.clone();
Expand Down
37 changes: 30 additions & 7 deletions library/test/src/test_result.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use std::any::Any;
use std::process::ExitStatus;

#[cfg(unix)]
use std::os::unix::process::ExitStatusExt;

use super::bench::BenchSamples;
use super::options::ShouldPanic;
Expand All @@ -7,11 +11,16 @@ use super::types::TestDesc;

pub use self::TestResult::*;

// Return codes for secondary process.
// Return code for secondary process.
// Start somewhere other than 0 so we know the return code means what we think
// it means.
pub const TR_OK: i32 = 50;
pub const TR_FAILED: i32 = 51;

#[cfg(unix)]
const SIGABRT: i32 = 6;

#[cfg(windows)]
const EXIT_ABORTED: i32 = 3;

#[derive(Debug, Clone, PartialEq)]
pub enum TestResult {
Expand Down Expand Up @@ -81,14 +90,28 @@ pub fn calc_result<'a>(
/// Creates a `TestResult` depending on the exit code of test subprocess.
pub fn get_result_from_exit_code(
desc: &TestDesc,
code: i32,
status: ExitStatus,
time_opts: &Option<time::TestTimeOptions>,
exec_time: &Option<time::TestExecTime>,
) -> TestResult {
let result = match code {
TR_OK => TestResult::TrOk,
TR_FAILED => TestResult::TrFailed,
_ => TestResult::TrFailedMsg(format!("got unexpected return code {code}")),
let result = match status.code() {
Some(TR_OK) => TestResult::TrOk,
#[cfg(windows)]
Some(EXIT_ABORTED) => TestResult::TrFailed,
#[cfg(unix)]
None => match status.signal() {
Some(SIGABRT) => TestResult::TrFailed,
Some(signal) => {
TestResult::TrFailedMsg(format!("child process exited with signal {signal}"))
}
None => unreachable!("status.code() returned None but status.signal() was None"),
},
#[cfg(not(unix))]
None => TestResult::TrFailedMsg(format!("unknown return code")),
#[cfg(any(windows, unix))]
Some(code) => TestResult::TrFailedMsg(format!("got unexpected return code {code}")),
#[cfg(not(any(windows, unix)))]
Some(_) => TestResult::TrFailed,
};

// If test is already failed (or allowed to fail), do not change the result.
Expand Down
5 changes: 5 additions & 0 deletions src/ci/github-actions/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,11 @@ jobs:
env:
CODEGEN_BACKENDS: llvm,cranelift
<<: *job-linux-16c
- name: x86_64-msvc
env:
RUST_CONFIGURE_ARGS: --build=x86_64-pc-windows-msvc --enable-profiler
SCRIPT: make ci-msvc
<<: *job-windows-8c


master:
Expand Down

0 comments on commit 11f3f41

Please sign in to comment.