Improve presentation of integration test results

.github/workflows/integration_tests.yml

@@ -52,4 +52,14 @@ jobs:
         run: curl -L -o /tmp/libkrunfw-5.0.0-x86_64.tgz https://github.com/containers/libkrunfw/releases/download/v5.0.0/libkrunfw-5.0.0-x86_64.tgz && mkdir tmp && tar xf /tmp/libkrunfw-5.0.0-x86_64.tgz -C tmp && sudo mv tmp/lib64/* /lib/x86_64-linux-gnu
 
       - name: Integration tests
-        run: RUST_LOG=trace KRUN_ENOMEM_WORKAROUND=1 KRUN_NO_UNSHARE=1 make test
+        run: KRUN_ENOMEM_WORKAROUND=1 KRUN_NO_UNSHARE=1 KRUN_TEST_BASE_DIR=/tmp/libkrun-tests make test TEST_FLAGS="--keep-all --github-summary"
+
+      - name: Upload test logs
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: test-logs
+          path: |
+            /tmp/libkrun-tests/
+            !/tmp/libkrun-tests/**/guest-agent
+          if-no-files-found: ignore

Makefile

@@ -181,5 +181,8 @@ test-prefix/lib64/libkrun.pc: $(LIBRARY_RELEASE_$(OS))
 
 test-prefix: test-prefix/lib64/libkrun.pc
 
+TEST ?= all
+TEST_FLAGS ?=
+
 test: test-prefix
-	cd tests; LD_LIBRARY_PATH="$$(realpath ../test-prefix/lib64/)" PKG_CONFIG_PATH="$$(realpath ../test-prefix/lib64/pkgconfig/)" ./run.sh
+	cd tests; RUST_LOG=trace LD_LIBRARY_PATH="$$(realpath ../test-prefix/lib64/)" PKG_CONFIG_PATH="$$(realpath ../test-prefix/lib64/pkgconfig/)" ./run.sh test --test-case "$(TEST)" $(TEST_FLAGS)

tests/run.sh

@@ -1,4 +1,4 @@
-#/bin/sh
+#!/bin/sh
 
 # This script has to be run with the working directory being "test"
 # This runs the tests on the libkrun instance found by pkg-config.
@@ -16,11 +16,19 @@ cargo build -p runner
 
 export KRUN_TEST_GUEST_AGENT_PATH="target/$GUEST_TARGET_ARCH/debug/guest-agent"
 
+# Build runner args: pass through all arguments
+RUNNER_ARGS="$*"
+
+# Add --base-dir if KRUN_TEST_BASE_DIR is set
+if [ -n "${KRUN_TEST_BASE_DIR}" ]; then
+	RUNNER_ARGS="${RUNNER_ARGS} --base-dir ${KRUN_TEST_BASE_DIR}"
+fi
+
 if [ -z "${KRUN_NO_UNSHARE}" ] && which unshare 2>&1 >/dev/null; then
-	unshare --user --map-root-user --net -- /bin/sh -c "ifconfig lo 127.0.0.1 && exec target/debug/runner $@"
+	unshare --user --map-root-user --net -- /bin/sh -c "ifconfig lo 127.0.0.1 && exec target/debug/runner ${RUNNER_ARGS}"
 else
 	echo "WARNING: Running tests without a network namespace."
 	echo "Tests may fail if the required network ports are already in use."
 	echo
-	target/debug/runner $@
+	target/debug/runner ${RUNNER_ARGS}
 fi

tests/runner/src/main.rs

@@ -1,13 +1,21 @@
 use anyhow::Context;
 use clap::Parser;
 use nix::sys::resource::{getrlimit, setrlimit, Resource};
+use std::env;
+use std::fs::{self, File};
+use std::io::Write;
 use std::panic::catch_unwind;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
-use std::{env, mem};
 use tempdir::TempDir;
 use test_cases::{test_cases, Test, TestCase, TestSetup};
 
+struct TestResult {
+    name: String,
+    passed: bool,
+    log_path: PathBuf,
+}
+
 fn get_test(name: &str) -> anyhow::Result<Box<dyn Test>> {
     let tests = test_cases();
     tests
@@ -30,20 +38,34 @@ fn start_vm(test_setup: TestSetup) -> anyhow::Result<()> {
     Ok(())
 }
 
-fn run_single_test(test_case: &str) -> anyhow::Result<bool> {
+fn run_single_test(
+    test_case: &str,
+    base_dir: &Path,
+    keep_all: bool,
+    max_name_len: usize,
+) -> anyhow::Result<TestResult> {
     let executable = env::current_exe().context("Failed to detect current executable")?;
-    let tmp_dir =
-        TempDir::new(&format!("krun-test-{test_case}")).context("Failed to create tmp dir")?;
+    let test_dir = base_dir.join(test_case);
+    fs::create_dir(&test_dir).context("Failed to create test directory")?;
+
+    let log_path = test_dir.join("log.txt");
+    let log_file = File::create(&log_path).context("Failed to create log file")?;
+
+    eprint!(
+        "[{test_case}] {:.<width$} ",
+        "",
+        width = max_name_len - test_case.len() + 3
+    );
 
     let child = Command::new(&executable)
         .arg("start-vm")
         .arg("--test-case")
         .arg(test_case)
         .arg("--tmp-dir")
-        .arg(tmp_dir.path())
+        .arg(&test_dir)
         .stdin(Stdio::piped())
         .stdout(Stdio::piped())
-        .stderr(Stdio::piped())
+        .stderr(log_file)
         .spawn()
         .context("Failed to start subprocess for test")?;
 
@@ -53,40 +75,122 @@ fn run_single_test(test_case: &str) -> anyhow::Result<bool> {
         test.check(child);
     });
 
-    match result {
-        Ok(()) => {
-            println!("[{test_case}]: OK");
-            Ok(true)
-        }
-        Err(_e) => {
-            println!("[{test_case}]: FAIL (dir {:?} kept)", tmp_dir.path());
-            mem::forget(tmp_dir);
-            Ok(false)
+    let passed = result.is_ok();
+    if passed {
+        eprintln!("OK");
+        if !keep_all {
+            let _ = fs::remove_dir_all(&test_dir);
         }
+    } else {
+        eprintln!("FAIL");
     }
+
+    Ok(TestResult {
+        name: test_case.to_string(),
+        passed,
+        log_path,
+    })
 }
 
-fn run_tests(test_case: &str) -> anyhow::Result<()> {
-    let mut num_tests = 1;
-    let mut num_ok: usize = 0;
+fn write_github_summary(
+    results: &[TestResult],
+    num_ok: usize,
+    num_tests: usize,
+) -> anyhow::Result<()> {
+    let summary_path = env::var("GITHUB_STEP_SUMMARY")
+        .context("GITHUB_STEP_SUMMARY environment variable not set")?;
+
+    let mut file = fs::OpenOptions::new()
+        .create(true)
+        .append(true)
+        .open(&summary_path)
+        .context("Failed to open GITHUB_STEP_SUMMARY")?;
+
+    let all_passed = num_ok == num_tests;
+    let status = if all_passed { "✅" } else { "❌" };
+
+    writeln!(
+        file,
+        "## {status} Integration Tests ({num_ok}/{num_tests} passed)\n"
+    )?;
+
+    for result in results {
+        let icon = if result.passed { "✅" } else { "❌" };
+        let log_content = fs::read_to_string(&result.log_path).unwrap_or_default();
+
+        writeln!(file, "<details>")?;
+        writeln!(file, "<summary>{icon} {}</summary>\n", result.name)?;
+        writeln!(file, "```")?;
+        // Limit log size to avoid huge summaries (2 MiB limit)
+        const MAX_LOG_SIZE: usize = 2 * 1024 * 1024;
+        let truncated = if log_content.len() > MAX_LOG_SIZE {
+            format!(
+                "... (truncated, showing last 1 MiB) ...\n{}",
+                &log_content[log_content.len() - MAX_LOG_SIZE..]
+            )
+        } else {
+            log_content
+        };
+        writeln!(file, "{truncated}")?;
+        writeln!(file, "```")?;
+        writeln!(file, "</details>\n")?;
+    }
+
+    Ok(())
+}
+
+fn run_tests(
+    test_case: &str,
+    base_dir: Option<PathBuf>,
+    keep_all: bool,
+    github_summary: bool,
+) -> anyhow::Result<()> {
+    // Create the base directory - either use provided path or create a temp one
+    let base_dir = match base_dir {
+        Some(path) => {
+            fs::create_dir_all(&path).context("Failed to create base directory")?;
+            path
+        }
+        None => TempDir::new("libkrun-tests")
+            .context("Failed to create temp base directory")?
+            .into_path(),
+    };
+
+    let mut results: Vec<TestResult> = Vec::new();
 
     if test_case == "all" {
-        let test_cases = test_cases();
-        num_tests = test_cases.len();
+        let all_tests = test_cases();
+        let max_name_len = all_tests.iter().map(|t| t.name.len()).max().unwrap_or(0);
 
-        for TestCase { name, test: _ } in test_cases {
-            num_ok += run_single_test(name).context(name)? as usize;
+        for TestCase { name, test: _ } in all_tests {
+            results.push(run_single_test(name, &base_dir, keep_all, max_name_len).context(name)?);
         }
     } else {
-        num_ok += run_single_test(test_case).context(test_case.to_string())? as usize;
+        let max_name_len = test_case.len();
+        results.push(
+            run_single_test(test_case, &base_dir, keep_all, max_name_len)
+                .context(test_case.to_string())?,
+        );
+    }
+
+    let num_tests = results.len();
+    let num_ok = results.iter().filter(|r| r.passed).count();
+
+    // Write GitHub Actions summary if requested
+    if github_summary {
+        write_github_summary(&results, num_ok, num_tests)?;
     }
 
     let num_failures = num_tests - num_ok;
     if num_failures > 0 {
+        eprintln!("(See test artifacts at: {})", base_dir.display());
         println!("\nFAIL (PASSED {num_ok}/{num_tests})");
         anyhow::bail!("")
     } else {
-        println!("\nOK (PASSED {num_ok}/{num_tests})");
+        if keep_all {
+            eprintln!("(See test artifacts at: {})", base_dir.display());
+        }
+        eprintln!("\nOK ({num_ok}/{num_tests} passed)");
     }
 
     Ok(())
@@ -98,6 +202,15 @@ enum CliCommand {
         /// Specify which test to run or "all"
         #[arg(long, default_value = "all")]
         test_case: String,
+        /// Base directory for test artifacts
+        #[arg(long)]
+        base_dir: Option<PathBuf>,
+        /// Keep all test artifacts even on success
+        #[arg(long)]
+        keep_all: bool,
+        /// Write test results to GitHub Actions job summary ($GITHUB_STEP_SUMMARY)
+        #[arg(long)]
+        github_summary: bool,
     },
     StartVm {
         #[arg(long)]
@@ -111,6 +224,9 @@ impl Default for CliCommand {
     fn default() -> Self {
         Self::Test {
             test_case: "all".to_string(),
+            base_dir: None,
+            keep_all: false,
+            github_summary: false,
         }
     }
 }
@@ -127,6 +243,11 @@ fn main() -> anyhow::Result<()> {
 
     match command {
         CliCommand::StartVm { test_case, tmp_dir } => start_vm(TestSetup { test_case, tmp_dir }),
-        CliCommand::Test { test_case } => run_tests(&test_case),
+        CliCommand::Test {
+            test_case,
+            base_dir,
+            keep_all,
+            github_summary,
+        } => run_tests(&test_case, base_dir, keep_all, github_summary),
     }
 }

tests/test_cases/src/lib.rs

@@ -78,11 +78,6 @@ pub trait Test {
     /// Checks the output of the (host) process which started the VM
     fn check(self: Box<Self>, child: Child) {
         let output = child.wait_with_output().unwrap();
-        let err = String::from_utf8(output.stderr).unwrap();
-        if !err.is_empty() {
-            eprintln!("{}", err);
-        }
-
         assert_eq!(String::from_utf8(output.stdout).unwrap(), "OK\n");
     }
 }

tests/test_cases/src/test_multiport_console.rs

@@ -50,7 +50,7 @@ mod host {
     impl Test for TestMultiportConsole {
         fn start_vm(self: Box<Self>, test_setup: TestSetup) -> anyhow::Result<()> {
             unsafe {
-                krun_call!(krun_set_log_level(KRUN_LOG_LEVEL_WARN))?;
+                krun_call!(krun_set_log_level(KRUN_LOG_LEVEL_TRACE))?;
                 let ctx = krun_call_u32!(krun_create_ctx())?;
 
                 krun_call!(krun_disable_implicit_console(ctx))?;

tests/test_cases/src/test_tsi_tcp_guest_connect.rs

@@ -30,7 +30,7 @@ mod host {
             let listener = self.tcp_tester.create_server_socket();
             thread::spawn(move || self.tcp_tester.run_server(listener));
             unsafe {
-                krun_call!(krun_set_log_level(KRUN_LOG_LEVEL_WARN))?;
+                krun_call!(krun_set_log_level(KRUN_LOG_LEVEL_TRACE))?;
                 let ctx = krun_call_u32!(krun_create_ctx())?;
                 krun_call!(krun_set_vm_config(ctx, 1, 512))?;
                 setup_fs_and_enter(ctx, test_setup)?;

tests/test_cases/src/test_tsi_tcp_guest_listen.rs

@@ -34,7 +34,7 @@ mod host {
                     self.tcp_tester.run_client();
                 });
 
-                krun_call!(krun_set_log_level(KRUN_LOG_LEVEL_WARN))?;
+                krun_call!(krun_set_log_level(KRUN_LOG_LEVEL_TRACE))?;
                 let ctx = krun_call_u32!(krun_create_ctx())?;
                 let port_mapping = format!("{PORT}:{PORT}");
                 let port_mapping = CString::new(port_mapping).unwrap();

tests/test_cases/src/test_vm_config.rs

@@ -17,7 +17,7 @@ mod host {
     impl Test for TestVmConfig {
         fn start_vm(self: Box<Self>, test_setup: TestSetup) -> anyhow::Result<()> {
             unsafe {
-                krun_call!(krun_set_log_level(KRUN_LOG_LEVEL_WARN))?;
+                krun_call!(krun_set_log_level(KRUN_LOG_LEVEL_TRACE))?;
                 let ctx = krun_call_u32!(krun_create_ctx())?;
                 krun_call!(krun_set_vm_config(ctx, self.num_cpus, self.ram_mib))?;
                 setup_fs_and_enter(ctx, test_setup)?;

tests/test_cases/src/test_vsock_guest_connect.rs

@@ -63,7 +63,7 @@ mod host {
 
             thread::spawn(move || server(listener));
             unsafe {
-                krun_call!(krun_set_log_level(KRUN_LOG_LEVEL_WARN))?;
+                krun_call!(krun_set_log_level(KRUN_LOG_LEVEL_TRACE))?;
                 let ctx = krun_call_u32!(krun_create_ctx())?;
                 krun_call!(krun_add_vsock_port(
                     ctx,