Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runner improvements and signal bugfixes #1972

Merged
merged 4 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ cp_r = "0.5.2"
crossterm = { version = "0.28.1", features = ["event-stream"] }
dialoguer = "0.11.0"
debug-ignore = "1.0.5"
derive-where = "1.2.7"
duct = "0.13.7"
dunce = "1.0.5"
enable-ansi-support = "0.2.1"
Expand Down
1 change: 1 addition & 0 deletions integration-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ nextest-metadata.workspace = true
pathdiff.workspace = true
regex.workspace = true
target-spec.workspace = true
tokio.workspace = true

# These platforms are supported by num_threads.
# https://docs.rs/num_threads/0.1.7/src/num_threads/lib.rs.html#5-8
Expand Down
1 change: 1 addition & 0 deletions integration-tests/tests/integration/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use std::{borrow::Cow, fs::File, io::Write};
use target_spec::Platform;

mod fixtures;
mod stuck_signal;
mod temp_project;

use crate::temp_project::{create_uds, UdsStatus};
Expand Down
17 changes: 17 additions & 0 deletions integration-tests/tests/integration/stuck_signal.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) The nextest Contributors
// SPDX-License-Identifier: MIT OR Apache-2.0

//! A test that gets stuck in a signal handler.
//!
//! Meant mostly for debugging. We should also likely have a fixture which does
//! this, but it's hard to do that without pulling in extra dependencies.

#[ignore]
#[tokio::test]
async fn test_stuck_signal() {
// This test installs a signal handler that gets stuck. Feel free to tweak
// the loop as needed.
loop {
tokio::signal::ctrl_c().await.expect("received signal");
}
}
1 change: 1 addition & 0 deletions nextest-runner/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ cfg-if.workspace = true
chrono.workspace = true
crossterm.workspace = true
debug-ignore.workspace = true
derive-where.workspace = true
duct.workspace = true
future-queue.workspace = true
futures.workspace = true
Expand Down
8 changes: 7 additions & 1 deletion nextest-runner/src/list/test_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
use std::{
collections::{BTreeMap, BTreeSet},
ffi::{OsStr, OsString},
io,
fmt, io,
path::PathBuf,
sync::{Arc, OnceLock},
};
Expand Down Expand Up @@ -1070,6 +1070,12 @@
pub test_name: &'a str,
}

impl fmt::Display for TestInstanceId<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{} {}", self.binary_id, self.test_name)
}

Check warning on line 1076 in nextest-runner/src/list/test_list.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/list/test_list.rs#L1074-L1076

Added lines #L1074 - L1076 were not covered by tests
}

/// Context required for test execution.
#[derive(Clone, Debug)]
pub struct TestExecuteContext<'a> {
Expand Down
2 changes: 1 addition & 1 deletion nextest-runner/src/reporter/aggregator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@
//
// testsuite.add_testcase(testcase);
}
TestEventKind::RunBeginCancel { .. } => {}
TestEventKind::RunBeginCancel { .. } | TestEventKind::RunBeginKill { .. } => {}

Check warning on line 205 in nextest-runner/src/reporter/aggregator.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/reporter/aggregator.rs#L205

Added line #L205 was not covered by tests
TestEventKind::RunFinished {
run_id,
start_time,
Expand Down
53 changes: 47 additions & 6 deletions nextest-runner/src/reporter/displayer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,9 @@
// continue to output to it.
self.hidden_run_paused = false;
}
TestEventKind::RunBeginCancel { .. } => {
progress_bar.set_prefix(progress_bar_cancel_prefix(styles));
TestEventKind::RunBeginCancel { reason, .. }
| TestEventKind::RunBeginKill { reason, .. } => {
progress_bar.set_prefix(progress_bar_cancel_prefix(*reason, styles));

Check warning on line 465 in nextest-runner/src/reporter/displayer.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/reporter/displayer.rs#L463-L465

Added lines #L463 - L465 were not covered by tests
}
_ => {}
}
Expand Down Expand Up @@ -540,17 +541,25 @@
}
}

fn progress_bar_cancel_prefix(styles: &Styles) -> String {
format!("{:>12}", "Cancelling".style(styles.fail))
fn progress_bar_cancel_prefix(reason: CancelReason, styles: &Styles) -> String {
let status = match reason {
CancelReason::SetupScriptFailure
| CancelReason::TestFailure
| CancelReason::ReportError
| CancelReason::Signal
| CancelReason::Interrupt => "Cancelling",
CancelReason::SecondSignal => "Killing",

Check warning on line 551 in nextest-runner/src/reporter/displayer.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/reporter/displayer.rs#L551

Added line #L551 was not covered by tests
};
format!("{:>12}", status.style(styles.fail))
}

fn progress_bar_prefix(
run_stats: &RunStats,
cancel_reason: Option<CancelReason>,
styles: &Styles,
) -> String {
if cancel_reason.is_some() {
return progress_bar_cancel_prefix(styles);
if let Some(reason) = cancel_reason {
return progress_bar_cancel_prefix(reason, styles);
}

let style = if run_stats.has_failures() {
Expand Down Expand Up @@ -972,6 +981,38 @@
}
writeln!(writer)?;
}
TestEventKind::RunBeginKill {
setup_scripts_running,
running,
reason,
} => {
self.cancel_status = self.cancel_status.max(Some(*reason));

write!(
writer,
"{:>12} due to {}",
"Killing".style(self.styles.fail),
reason.to_static_str().style(self.styles.fail)
)?;

Check warning on line 996 in nextest-runner/src/reporter/displayer.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/reporter/displayer.rs#L985-L996

Added lines #L985 - L996 were not covered by tests

// At the moment, we can have either setup scripts or tests running, but not both.
if *setup_scripts_running > 0 {
let s = plural::setup_scripts_str(*setup_scripts_running);
write!(
writer,
": {} {s} still running",
setup_scripts_running.style(self.styles.count),
)?;
} else if *running > 0 {
let tests_str = plural::tests_str(*running);
write!(
writer,
": {} {tests_str} still running",
running.style(self.styles.count),
)?;
}
writeln!(writer)?;

Check warning on line 1014 in nextest-runner/src/reporter/displayer.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/reporter/displayer.rs#L999-L1014

Added lines #L999 - L1014 were not covered by tests
}
TestEventKind::RunPaused {
setup_scripts_running,
running,
Expand Down
16 changes: 16 additions & 0 deletions nextest-runner/src/reporter/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,18 @@
reason: CancelReason,
},

/// A forcible kill was requested due to receiving a signal.
RunBeginKill {
/// The number of setup scripts still running.
setup_scripts_running: usize,

/// The number of tests still running.
running: usize,

/// The reason this run was killed.
reason: CancelReason,
},

/// A SIGTSTP event was received and the run was paused.
RunPaused {
/// The number of setup scripts running.
Expand Down Expand Up @@ -777,6 +789,9 @@

/// An interrupt (on Unix, Ctrl-C) was received.
Interrupt,

/// A second signal was received, and the run is being forcibly killed.
SecondSignal,
}

impl CancelReason {
Expand All @@ -787,6 +802,7 @@
CancelReason::ReportError => "reporting error",
CancelReason::Signal => "signal",
CancelReason::Interrupt => "interrupt",
CancelReason::SecondSignal => "second signal",

Check warning on line 805 in nextest-runner/src/reporter/events.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/reporter/events.rs#L805

Added line #L805 was not covered by tests
}
}
}
Expand Down
Loading
Loading