From 1ad72bf14ec77fa9199a7e7542f4920f7ea2518a Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Thu, 26 Jan 2023 11:51:04 +0100 Subject: [PATCH 1/3] Print QEMU output directly instead of waiting until it finishes The `output` method captures the output in a `Vec`, so we can only print it when the QEMU command is finished. This is problematic when a test panics inside the bootloader because we never exit QEMU in that case. Thus, we never print the error message, which makes debugging more difficult. This commit fixes this issue by piping the stdout and stderr directly to a background thread, which prints them directly. We also print the full QEMU command and a separator line between test runs now, which should make the output easier to read. --- tests/runner/src/lib.rs | 129 ++++++++++++++++++++++------------------ 1 file changed, 70 insertions(+), 59 deletions(-) diff --git a/tests/runner/src/lib.rs b/tests/runner/src/lib.rs index f9491493..86b43e6e 100644 --- a/tests/runner/src/lib.rs +++ b/tests/runner/src/lib.rs @@ -1,4 +1,4 @@ -use std::{io::Write, path::Path, process::Command}; +use std::{io::Read, path::Path, process::Command}; const QEMU_ARGS: &[&str] = &[ "-device", @@ -9,6 +9,7 @@ const QEMU_ARGS: &[&str] = &[ "none", "--no-reboot", ]; +const SEPARATOR: &str = "\n____________________________________\n"; pub fn run_test_kernel(kernel_binary_path: &str) { run_test_kernel_with_ramdisk(kernel_binary_path, None) @@ -54,73 +55,83 @@ pub fn run_test_kernel_with_ramdisk(kernel_binary_path: &str, ramdisk_path: Opti #[cfg(feature = "uefi")] pub fn run_test_kernel_on_uefi(out_gpt_path: &Path) { - let mut run_cmd = Command::new("qemu-system-x86_64"); - run_cmd - .arg("-drive") - .arg(format!("format=raw,file={}", out_gpt_path.display())); - run_cmd.args(QEMU_ARGS); - run_cmd.arg("-bios").arg(ovmf_prebuilt::ovmf_pure_efi()); - - let child_output = run_cmd.output().unwrap(); - strip_ansi_escapes::Writer::new(std::io::stderr()) - .write_all(&child_output.stderr) - .unwrap(); - strip_ansi_escapes::Writer::new(std::io::stderr()) - .write_all(&child_output.stdout) - .unwrap(); - - match child_output.status.code() { - Some(33) => {} // success - Some(35) => panic!("Test failed"), // success - other => panic!("Test failed with unexpected exit code `{:?}`", other), - } + let ovmf_pure_efi = ovmf_prebuilt::ovmf_pure_efi(); + let args = [ + "-bios", + ovmf_pure_efi.to_str().unwrap(), + "-drive", + &format!("format=raw,file={}", out_gpt_path.display()), + ]; + run_qemu(args); } #[cfg(feature = "bios")] pub fn run_test_kernel_on_bios(out_mbr_path: &Path) { - let mut run_cmd = Command::new("qemu-system-x86_64"); - run_cmd - .arg("-drive") - .arg(format!("format=raw,file={}", out_mbr_path.display())); - run_cmd.args(QEMU_ARGS); - - let child_output = run_cmd.output().unwrap(); - strip_ansi_escapes::Writer::new(std::io::stderr()) - .write_all(&child_output.stderr) - .unwrap(); - strip_ansi_escapes::Writer::new(std::io::stderr()) - .write_all(&child_output.stdout) - .unwrap(); - - match child_output.status.code() { - Some(33) => {} // success - Some(35) => panic!("Test failed"), // success - other => panic!("Test failed with unexpected exit code `{:?}`", other), - } + let args = [ + "-drive", + &(format!("format=raw,file={}", out_mbr_path.display())), + ]; + run_qemu(args); } #[cfg(feature = "uefi")] pub fn run_test_kernel_on_uefi_pxe(out_tftp_path: &Path) { + let ovmf_pure_efi = ovmf_prebuilt::ovmf_pure_efi(); + let args = [ + "-netdev", + &format!( + "user,id=net0,net=192.168.17.0/24,tftp={},bootfile=bootloader,id=net0", + out_tftp_path.display() + ), + "-device", + "virtio-net-pci,netdev=net0", + "-bios", + ovmf_pure_efi.to_str().unwrap(), + ]; + run_qemu(args); +} + +fn run_qemu<'a, A>(args: A) +where + A: IntoIterator, +{ + use std::process::Stdio; + let mut run_cmd = Command::new("qemu-system-x86_64"); - run_cmd.arg("-netdev").arg(format!( - "user,id=net0,net=192.168.17.0/24,tftp={},bootfile=bootloader,id=net0", - out_tftp_path.display() - )); - run_cmd.arg("-device").arg("virtio-net-pci,netdev=net0"); + run_cmd.args(args); run_cmd.args(QEMU_ARGS); - run_cmd.arg("-bios").arg(ovmf_prebuilt::ovmf_pure_efi()); - - let child_output = run_cmd.output().unwrap(); - strip_ansi_escapes::Writer::new(std::io::stderr()) - .write_all(&child_output.stderr) - .unwrap(); - strip_ansi_escapes::Writer::new(std::io::stderr()) - .write_all(&child_output.stdout) - .unwrap(); - - match child_output.status.code() { - Some(33) => {} // success - Some(35) => panic!("Test failed"), - other => panic!("Test failed with unexpected exit code `{:?}`", other), + let run_cmd_str = format!("{run_cmd:?}"); + + run_cmd.stdout(Stdio::piped()); + run_cmd.stderr(Stdio::piped()); + + let mut child = run_cmd.spawn().unwrap(); + + let child_stdout = child.stdout.take().unwrap(); + let mut child_stderr = child.stderr.take().unwrap(); + + let copy_stdout = std::thread::spawn(move || { + let print_cmd = format!("\nRunning {run_cmd_str}\n\n").into_bytes(); + let mut output = print_cmd.chain(child_stdout).chain(SEPARATOR.as_bytes()); + std::io::copy( + &mut output, + &mut strip_ansi_escapes::Writer::new(std::io::stdout()), + ) + }); + let copy_stderr = std::thread::spawn(move || { + std::io::copy( + &mut child_stderr, + &mut strip_ansi_escapes::Writer::new(std::io::stderr()), + ) + }); + + let exit_status = child.wait().unwrap(); + match exit_status.code() { + Some(33) => {} // success + Some(35) => panic!("Test failed"), // success + other => panic!("Test failed with unexpected exit code `{other:?}`"), } + + copy_stdout.join().unwrap().unwrap(); + copy_stderr.join().unwrap().unwrap(); } From 170b7c39115ecaaba34277fe6eb6965bcd545094 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Thu, 26 Jan 2023 11:52:34 +0100 Subject: [PATCH 2/3] Run CI tests single threaded to avoid output mangling This way, the error should always be at/near the bottom, so it's easier to find. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0204477f..703445a5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -65,7 +65,7 @@ jobs: - name: Run api tests run: cargo test -p bootloader_api - name: Run integration tests - run: cargo test + run: cargo test -- --test-threads 1 # test feature gates (only on one OS is enough) - name: Test with only UEFI feature From a51bb151d1ba96a038ff2aabfb1a5d4fec2801b3 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Thu, 26 Jan 2023 11:53:43 +0100 Subject: [PATCH 3/3] Fix extension: `.tftp` instead of `..tftp` The `with_extension` method adds the dot already. --- tests/runner/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/runner/src/lib.rs b/tests/runner/src/lib.rs index 86b43e6e..ad6ac614 100644 --- a/tests/runner/src/lib.rs +++ b/tests/runner/src/lib.rs @@ -31,7 +31,7 @@ pub fn run_test_kernel_with_ramdisk(kernel_binary_path: &str, ramdisk_path: Opti // create a TFTP folder with the kernel executable and UEFI bootloader for // UEFI PXE booting - let tftp_path = kernel_path.with_extension(".tftp"); + let tftp_path = kernel_path.with_extension("tftp"); uefi_builder.create_pxe_tftp_folder(&tftp_path).unwrap(); run_test_kernel_on_uefi(&gpt_path);