From e7af2923ffe95b9bb5ff296b6c558db10ad923be Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 11 Jan 2025 11:21:31 -0800 Subject: [PATCH 1/2] fix(inoculate): `--nocapture` disabling output parsing (#511) Currently, the `--nocapture` flag to `cargo inoculate test` incorrectly disables the processing of test output, so we can't actually detect that tests are running. This is wrong. This commit makes it just print each line of output to stdout, but still try to parse them. --- inoculate/src/qemu.rs | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/inoculate/src/qemu.rs b/inoculate/src/qemu.rs index 565e6fff..610c14ad 100644 --- a/inoculate/src/qemu.rs +++ b/inoculate/src/qemu.rs @@ -78,12 +78,7 @@ struct TestResults { impl Cmd { fn should_capture(&self) -> bool { match self { - Cmd::Test { - nocapture: true, .. - } => { - tracing::debug!("running tests with `--nocapture`, will not capture."); - false - } + Cmd::Test { .. } => true, Cmd::Run { serial: true, .. } => { tracing::debug!("running normally with `--serial`, will not capture"); false @@ -184,6 +179,7 @@ impl Cmd { } Cmd::Test { + nocapture, qemu_settings, timeout_secs, .. @@ -208,11 +204,11 @@ impl Cmd { tracing::info!(qemu.test_args = ?TEST_ARGS, "using test mode qemu args"); qemu.args(TEST_ARGS); + let nocapture = *nocapture; let mut child = self.spawn_qemu(&mut qemu, paths.kernel_bin())?; - let stdout = child - .stdout - .take() - .map(|stdout| std::thread::spawn(move || TestResults::watch_tests(stdout))); + let stdout = child.stdout.take().map(|stdout| { + std::thread::spawn(move || TestResults::watch_tests(stdout, nocapture)) + }); let res = match child .wait_timeout(*timeout_secs) @@ -298,7 +294,7 @@ fn parse_secs(s: &str) -> Result { } impl TestResults { - fn watch_tests(output: impl std::io::Read) -> Result { + fn watch_tests(output: impl std::io::Read, nocapture: bool) -> Result { use std::io::{BufRead, BufReader}; let mut results = Self { tests: 0, @@ -363,8 +359,11 @@ impl TestResults { .note(format!("line: {line:?}")); } } - - curr_output.push(line); + if nocapture { + println!("{line}") + } else { + curr_output.push(line); + } } match curr_outcome { @@ -385,7 +384,9 @@ impl TestResults { } None => { tracing::info!("qemu exited unexpectedly! wtf!"); - curr_output.push("".to_string()); + if !nocapture { + curr_output.push("".to_string()); + } eprintln!(" {}", "exit!".style(red)); results.failed.insert(test.to_static(), curr_output); break; From a1249f45395544f4e7349f0b7ca63ae6022b6db6 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 11 Jan 2025 11:24:29 -0800 Subject: [PATCH 2/2] fix(inoculate): bigger test run timeout (#511) Currently, the inoculate test command sets a timeout of 60 seconds, and abruptly kills QEMU if it takes longer than that. This is no longer nearly long enough to run our tests. This commit increases the timeout to 1200 seconds (20 minutes). This is a long time, but our tests take a while to run, and recall that this timeout applies to booting the kernel *and running all the tests*, rather than being enforced on a per-test basis. We should probably make the timeout much smarter than just "kill the QEMU process after 20 minutes", and instead do it only if we don't see any interesting test output, or something. But, this is better than just always failing test runs because we're stupid about timeouts, I guess. --- inoculate/src/qemu.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/inoculate/src/qemu.rs b/inoculate/src/qemu.rs index 610c14ad..0fe9ca70 100644 --- a/inoculate/src/qemu.rs +++ b/inoculate/src/qemu.rs @@ -31,9 +31,15 @@ pub enum Cmd { Test { /// Timeout for failing test run, in seconds. /// - /// If a test run doesn't complete before this timeout has elapsed, it's - /// considered to have failed. - #[clap(long, value_parser = parse_secs, default_value = "60")] + /// If a test run doesn't complete before this timeout has elapsed, the + /// QEMU VM is killed and the test run is assumed to have failed. + /// + /// Note that this timeout applies to the entire test run, including + /// booting the kernel and running the whole kernel test suite, rather + /// than to each individual test. + /// + /// By default, this is 20 minutes (1200 seconds). + #[clap(long, value_parser = parse_secs, default_value = "1200")] timeout_secs: Duration, /// Disables capturing test serial output.