From 938c2c48449a29d6d51f68b61759fd94fef0c779 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 31 Oct 2025 11:22:43 -0400 Subject: [PATCH] tests: Add parameterized integration test infrastructure This introduces a matrix-based testing approach for integration tests that need to run against multiple container images. Instead of hardcoding specific images or manually creating variants for each distribution, tests can now register as parameterized tests and automatically run once per image in BCVK_ALL_IMAGES. This enables systematic cross-distribution testing without code duplication and makes it easy to add or change the test matrix by updating environment variables rather than modifying test code. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters --- Justfile | 16 ++-- crates/integration-tests/src/lib.rs | 86 +++++++++++++++++++ crates/integration-tests/src/main.rs | 79 +++++++++++------ .../src/tests/run_ephemeral_ssh.rs | 44 +++++----- crates/integration-tests/src/tests/to_disk.rs | 10 +-- 5 files changed, 173 insertions(+), 62 deletions(-) diff --git a/Justfile b/Justfile index 2ee76d4c4..739d86399 100644 --- a/Justfile +++ b/Justfile @@ -1,3 +1,6 @@ +PRIMARY_IMAGE := "quay.io/centos-bootc/centos-bootc:stream10" +ALL_BASE_IMAGES := "quay.io/fedora/fedora-bootc:42 quay.io/centos-bootc/centos-bootc:stream9 quay.io/centos-bootc/centos-bootc:stream10" + # Build the native binary build: make @@ -17,17 +20,20 @@ unit *ARGS: fi pull-test-images: - podman pull -q quay.io/fedora/fedora-bootc:42 quay.io/centos-bootc/centos-bootc:stream9 quay.io/centos-bootc/centos-bootc:stream10 >/dev/null + podman pull -q {{ALL_BASE_IMAGES}} >/dev/null # Run integration tests (auto-detects nextest, with cleanup) test-integration *ARGS: build pull-test-images #!/usr/bin/env bash set -euo pipefail export BCVK_PATH=$(pwd)/target/release/bcvk - + export BCVK_PRIMARY_IMAGE={{ PRIMARY_IMAGE }} + # Note: BCVK_ALL_IMAGES is quoted to preserve the space-separated list + export BCVK_ALL_IMAGES="{{ ALL_BASE_IMAGES }}" + # Clean up any leftover containers before starting cargo run --release --bin test-cleanup -p integration-tests 2>/dev/null || true - + # Run the tests if command -v cargo-nextest &> /dev/null; then cargo nextest run --release -P integration -p integration-tests {{ ARGS }} @@ -36,10 +42,10 @@ test-integration *ARGS: build pull-test-images cargo test --release -p integration-tests -- {{ ARGS }} TEST_EXIT_CODE=$? fi - + # Clean up containers after tests complete cargo run --release --bin test-cleanup -p integration-tests 2>/dev/null || true - + exit $TEST_EXIT_CODE # Clean up integration test containers diff --git a/crates/integration-tests/src/lib.rs b/crates/integration-tests/src/lib.rs index 62571cafa..86502f6de 100644 --- a/crates/integration-tests/src/lib.rs +++ b/crates/integration-tests/src/lib.rs @@ -17,6 +17,9 @@ pub const LIBVIRT_INTEGRATION_TEST_LABEL: &str = "bcvk-integration"; /// A test function that returns a Result pub type TestFn = fn() -> color_eyre::Result<()>; +/// A parameterized test function that takes an image parameter +pub type ParameterizedTestFn = fn(&str) -> color_eyre::Result<()>; + /// Metadata for a registered integration test #[derive(Debug)] pub struct IntegrationTest { @@ -33,6 +36,89 @@ impl IntegrationTest { } } +/// Metadata for a parameterized integration test that runs once per image +#[derive(Debug)] +pub struct ParameterizedIntegrationTest { + /// Base name of the integration test (will be suffixed with image identifier) + pub name: &'static str, + /// Parameterized test function to execute + pub f: ParameterizedTestFn, +} + +impl ParameterizedIntegrationTest { + /// Create a new parameterized integration test with the given name and function + pub const fn new(name: &'static str, f: ParameterizedTestFn) -> Self { + Self { name, f } + } +} + /// Distributed slice holding all registered integration tests #[distributed_slice] pub static INTEGRATION_TESTS: [IntegrationTest]; + +/// Distributed slice holding all registered parameterized integration tests +#[distributed_slice] +pub static PARAMETERIZED_INTEGRATION_TESTS: [ParameterizedIntegrationTest]; + +/// Create a test suffix from an image name by replacing invalid characters with underscores +/// +/// Replaces all non-alphanumeric characters with `_` to create a predictable, filesystem-safe +/// test name suffix. +/// +/// Examples: +/// - "quay.io/fedora/fedora-bootc:42" -> "quay_io_fedora_fedora_bootc_42" +/// - "quay.io/centos-bootc/centos-bootc:stream10" -> "quay_io_centos_bootc_centos_bootc_stream10" +/// - "quay.io/image@sha256:abc123" -> "quay_io_image_sha256_abc123" +pub fn image_to_test_suffix(image: &str) -> String { + image.replace(|c: char| !c.is_alphanumeric(), "_") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_image_to_test_suffix_basic() { + assert_eq!( + image_to_test_suffix("quay.io/fedora/fedora-bootc:42"), + "quay_io_fedora_fedora_bootc_42" + ); + } + + #[test] + fn test_image_to_test_suffix_stream() { + assert_eq!( + image_to_test_suffix("quay.io/centos-bootc/centos-bootc:stream10"), + "quay_io_centos_bootc_centos_bootc_stream10" + ); + } + + #[test] + fn test_image_to_test_suffix_digest() { + assert_eq!( + image_to_test_suffix("quay.io/image@sha256:abc123"), + "quay_io_image_sha256_abc123" + ); + } + + #[test] + fn test_image_to_test_suffix_complex() { + assert_eq!( + image_to_test_suffix("registry.example.com:5000/my-org/my-image:v1.2.3"), + "registry_example_com_5000_my_org_my_image_v1_2_3" + ); + } + + #[test] + fn test_image_to_test_suffix_only_alphanumeric() { + assert_eq!(image_to_test_suffix("simpleimage"), "simpleimage"); + } + + #[test] + fn test_image_to_test_suffix_special_chars() { + assert_eq!( + image_to_test_suffix("image/with@special:chars-here.now"), + "image_with_special_chars_here_now" + ); + } +} diff --git a/crates/integration-tests/src/main.rs b/crates/integration-tests/src/main.rs index 1d27d4d2f..04ee10c60 100644 --- a/crates/integration-tests/src/main.rs +++ b/crates/integration-tests/src/main.rs @@ -11,7 +11,8 @@ use xshell::{cmd, Shell}; // Re-export constants from lib for internal use pub(crate) use integration_tests::{ - IntegrationTest, INTEGRATION_TESTS, INTEGRATION_TEST_LABEL, LIBVIRT_INTEGRATION_TEST_LABEL, + image_to_test_suffix, IntegrationTest, ParameterizedIntegrationTest, INTEGRATION_TESTS, + INTEGRATION_TEST_LABEL, LIBVIRT_INTEGRATION_TEST_LABEL, PARAMETERIZED_INTEGRATION_TESTS, }; use linkme::distributed_slice; @@ -43,30 +44,38 @@ pub(crate) fn get_bck_command() -> Result { return Ok("bcvk".to_owned()); } -/// Get the default bootc image to use for tests +/// Get the primary bootc image to use for tests /// -/// Checks BCVK_TEST_IMAGE environment variable first, then falls back to default. -/// This allows easily overriding the base image for all integration tests. -/// -/// Default images: -/// - Primary: quay.io/fedora/fedora-bootc:42 (Fedora 42 with latest features) -/// - Alternative: quay.io/centos-bootc/centos-bootc:stream9 (CentOS Stream 9 for compatibility testing) +/// Checks BCVK_PRIMARY_IMAGE environment variable first, then falls back to BCVK_TEST_IMAGE +/// for backwards compatibility, then to a hardcoded default. pub(crate) fn get_test_image() -> String { - std::env::var("BCVK_TEST_IMAGE") - .unwrap_or_else(|_| "quay.io/fedora/fedora-bootc:42".to_string()) + std::env::var("BCVK_PRIMARY_IMAGE") + .or_else(|_| std::env::var("BCVK_TEST_IMAGE")) + .unwrap_or_else(|_| "quay.io/centos-bootc/centos-bootc:stream10".to_string()) } -/// Get an alternative bootc image for cross-platform testing +/// Get all test images for matrix testing +/// +/// Parses BCVK_ALL_IMAGES environment variable, which should be a whitespace-separated +/// list of container images (spaces, tabs, and newlines are all acceptable separators). +/// Falls back to a single-element vec containing the primary image if not set or empty. /// -/// Returns a different image from the primary test image to test compatibility. -/// If BCVK_TEST_IMAGE is set to Fedora, returns CentOS Stream 9. -/// If BCVK_TEST_IMAGE is set to CentOS, returns Fedora. -pub(crate) fn get_alternative_test_image() -> String { - let primary = get_test_image(); - if primary.contains("centos") { - "quay.io/fedora/fedora-bootc:42".to_string() +/// Example: `export BCVK_ALL_IMAGES="quay.io/fedora/fedora-bootc:42 quay.io/centos-bootc/centos-bootc:stream9"` +pub(crate) fn get_all_test_images() -> Vec { + if let Ok(all_images) = std::env::var("BCVK_ALL_IMAGES") { + let images: Vec = all_images + .split_whitespace() + .map(|s| s.to_string()) + .collect(); + + if images.is_empty() { + eprintln!("Warning: BCVK_ALL_IMAGES is set but empty, falling back to primary image"); + vec![get_test_image()] + } else { + images + } } else { - "quay.io/centos-bootc/centos-bootc:stream9".to_string() + vec![get_test_image()] } } @@ -183,15 +192,29 @@ fn test_images_list() -> Result<()> { fn main() { let args = Arguments::from_args(); - // Collect tests from the distributed slice - let tests: Vec = INTEGRATION_TESTS - .iter() - .map(|test| { - let name = test.name; - let f = test.f; - Trial::test(name, move || f().map_err(|e| format!("{:?}", e).into())) - }) - .collect(); + let mut tests: Vec = Vec::new(); + + // Collect regular tests from the distributed slice + tests.extend(INTEGRATION_TESTS.iter().map(|test| { + let name = test.name; + let f = test.f; + Trial::test(name, move || f().map_err(|e| format!("{:?}", e).into())) + })); + + // Collect parameterized tests and generate variants for each image + let all_images = get_all_test_images(); + for param_test in PARAMETERIZED_INTEGRATION_TESTS.iter() { + for image in &all_images { + let image = image.clone(); + let test_suffix = image_to_test_suffix(&image); + let test_name = format!("{}_{}", param_test.name, test_suffix); + let f = param_test.f; + + tests.push(Trial::test(test_name, move || { + f(&image).map_err(|e| format!("{:?}", e).into()) + })); + } + } // Run the tests and exit with the result libtest_mimic::run(&args, tests).exit(); diff --git a/crates/integration-tests/src/tests/run_ephemeral_ssh.rs b/crates/integration-tests/src/tests/run_ephemeral_ssh.rs index 54c72a647..219d6019a 100644 --- a/crates/integration-tests/src/tests/run_ephemeral_ssh.rs +++ b/crates/integration-tests/src/tests/run_ephemeral_ssh.rs @@ -21,8 +21,8 @@ use std::thread; use std::time::Duration; use crate::{ - get_alternative_test_image, get_test_image, run_bcvk, IntegrationTest, INTEGRATION_TESTS, - INTEGRATION_TEST_LABEL, + get_test_image, run_bcvk, IntegrationTest, ParameterizedIntegrationTest, INTEGRATION_TESTS, + INTEGRATION_TEST_LABEL, PARAMETERIZED_INTEGRATION_TESTS, }; #[distributed_slice(INTEGRATION_TESTS)] @@ -144,22 +144,18 @@ fn test_run_ephemeral_ssh_exit_code() -> Result<()> { Ok(()) } -#[distributed_slice(INTEGRATION_TESTS)] -static TEST_RUN_EPHEMERAL_SSH_CROSS_DISTRO_COMPATIBILITY: IntegrationTest = IntegrationTest::new( - "run_ephemeral_ssh_cross_distro_compatibility", - test_run_ephemeral_ssh_cross_distro_compatibility, -); - -/// Test SSH functionality across different bootc images (Fedora and CentOS) -/// This test verifies that our systemd version compatibility fix works correctly -/// with both newer systemd (Fedora) and older systemd (CentOS Stream 9) -fn test_run_ephemeral_ssh_cross_distro_compatibility() -> Result<()> { - test_ssh_with_image(&get_test_image(), "primary")?; - test_ssh_with_image(&get_alternative_test_image(), "alternative")?; - Ok(()) -} +#[distributed_slice(PARAMETERIZED_INTEGRATION_TESTS)] +static TEST_RUN_EPHEMERAL_SSH_CROSS_DISTRO_COMPATIBILITY: ParameterizedIntegrationTest = + ParameterizedIntegrationTest::new( + "run_ephemeral_ssh_cross_distro_compatibility", + test_run_ephemeral_ssh_cross_distro_compatibility, + ); -fn test_ssh_with_image(image: &str, image_type: &str) -> Result<()> { +/// Test SSH functionality across different bootc images +/// This parameterized test runs once per image in BCVK_ALL_IMAGES and verifies +/// that our systemd version compatibility fix works correctly with both newer +/// systemd (Fedora) and older systemd (CentOS Stream 9) +fn test_run_ephemeral_ssh_cross_distro_compatibility(image: &str) -> Result<()> { let output = run_bcvk(&[ "ephemeral", "run-ssh", @@ -173,21 +169,21 @@ fn test_ssh_with_image(image: &str, image_type: &str) -> Result<()> { assert!( output.success(), - "{} image SSH test failed: {}", - image_type, + "SSH test failed for image {}: {}", + image, output.stderr ); assert!( output.stdout.contains("systemd"), - "{} image: systemd version not found. Got: {}", - image_type, + "systemd version not found for image {}. Got: {}", + image, output.stdout ); // Log systemd version for diagnostic purposes if let Some(version_line) = output.stdout.lines().next() { - eprintln!("{} image systemd version: {}", image_type, version_line); + eprintln!("Image {} systemd version: {}", image, version_line); let version_parts: Vec<&str> = version_line.split_whitespace().collect(); if version_parts.len() >= 2 { @@ -195,12 +191,12 @@ fn test_ssh_with_image(image: &str, image_type: &str) -> Result<()> { if version_num >= 254 { eprintln!( "✓ {} supports vmm.notify_socket (version {})", - image_type, version_num + image, version_num ); } else { eprintln!( "✓ {} falls back to SSH polling (version {} < 254)", - image_type, version_num + image, version_num ); } } diff --git a/crates/integration-tests/src/tests/to_disk.rs b/crates/integration-tests/src/tests/to_disk.rs index a8a2dc019..5081647ac 100644 --- a/crates/integration-tests/src/tests/to_disk.rs +++ b/crates/integration-tests/src/tests/to_disk.rs @@ -20,7 +20,7 @@ use linkme::distributed_slice; use std::process::Command; use tempfile::TempDir; -use crate::{run_bcvk, IntegrationTest, INTEGRATION_TESTS, INTEGRATION_TEST_LABEL}; +use crate::{get_test_image, run_bcvk, IntegrationTest, INTEGRATION_TESTS, INTEGRATION_TEST_LABEL}; #[distributed_slice(INTEGRATION_TESTS)] static TEST_TO_DISK: IntegrationTest = IntegrationTest::new("to_disk", test_to_disk); @@ -35,7 +35,7 @@ fn test_to_disk() -> Result<()> { "to-disk", "--label", INTEGRATION_TEST_LABEL, - "quay.io/centos-bootc/centos-bootc:stream10", + &get_test_image(), disk_path.as_str(), ])?; @@ -104,7 +104,7 @@ fn test_to_disk_qcow2() -> Result<()> { "--format=qcow2", "--label", INTEGRATION_TEST_LABEL, - "quay.io/centos-bootc/centos-bootc:stream10", + &get_test_image(), disk_path.as_str(), ])?; @@ -162,7 +162,7 @@ fn test_to_disk_caching() -> Result<()> { "to-disk", "--label", INTEGRATION_TEST_LABEL, - "quay.io/centos-bootc/centos-bootc:stream10", + &get_test_image(), disk_path.as_str(), ])?; @@ -189,7 +189,7 @@ fn test_to_disk_caching() -> Result<()> { "to-disk", "--label", INTEGRATION_TEST_LABEL, - "quay.io/centos-bootc/centos-bootc:stream10", + &get_test_image(), disk_path.as_str(), ])?;