From b724d46f2cd5eed6e4dbcfb700aa982c87720093 Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Sun, 11 Jan 2026 00:32:03 +0000 Subject: [PATCH] deb: fix concurrent testing (by making them serial) Co-authored-by: Valentin Haudiquet Co-committed-by: Valentin Haudiquet --- .github/workflows/ci.yml | 2 +- Cargo.toml | 3 ++- src/context/local.rs | 31 +++++++++++++++++++++++++++-- src/context/unshare.rs | 42 ++++++++++++++++++++++++++-------------- src/deb/mod.rs | 10 ++++++++++ 5 files changed, 70 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ef400d1..b58d103 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,7 @@ name: CI on: push: - branches: [ "main" ] + branches: [ "main", "ci-test" ] pull_request: branches: [ "main" ] diff --git a/Cargo.toml b/Cargo.toml index 13c6044..4cb9855 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,8 +27,9 @@ xz2 = "0.1" serde_json = "1.0.145" directories = "6.0.0" ssh2 = "0.9.5" -tempfile = "3.10.1" gpgme = "0.11" [dev-dependencies] test-log = "0.2.19" +serial_test = "3.3.1" +tempfile = "3.10.1" diff --git a/src/context/local.rs b/src/context/local.rs index 7d391cc..b20f008 100644 --- a/src/context/local.rs +++ b/src/context/local.rs @@ -4,6 +4,7 @@ use super::api::ContextDriver; use std::io; use std::path::{Path, PathBuf}; use std::process::Command; +use std::time::SystemTime; pub struct LocalDriver; @@ -20,8 +21,34 @@ impl ContextDriver for LocalDriver { } fn create_temp_dir(&self) -> io::Result { - let temp_dir = tempfile::Builder::new().prefix("pkh-").tempdir()?; - Ok(temp_dir.keep().to_string_lossy().to_string()) + // Generate a unique temporary directory name with random string + let base_timestamp = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs(); + + let mut attempt = 0; + loop { + let work_dir_name = if attempt == 0 { + format!("pkh-{}", base_timestamp) + } else { + format!("pkh-{}-{}", base_timestamp, attempt) + }; + + let temp_dir_path = std::env::temp_dir().join(&work_dir_name); + + // Check if directory already exists + if temp_dir_path.exists() { + attempt += 1; + continue; + } + + // Create the directory + std::fs::create_dir_all(&temp_dir_path)?; + + // Return the path as a string + return Ok(temp_dir_path.to_string_lossy().to_string()); + } } fn retrieve_path(&self, src: &Path, dest: &Path) -> io::Result<()> { diff --git a/src/context/unshare.rs b/src/context/unshare.rs index 8e2a319..a1a5a95 100644 --- a/src/context/unshare.rs +++ b/src/context/unshare.rs @@ -112,27 +112,41 @@ impl ContextDriver for UnshareDriver { } fn create_temp_dir(&self) -> io::Result { - // Create a temporary directory inside the chroot - let timestamp = std::time::SystemTime::now() + // Create a temporary directory inside the chroot with unique naming + let base_timestamp = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap() .as_secs(); - let work_dir_name = format!("pkh-build-{}", timestamp); - let work_dir_inside_chroot = format!("/tmp/{}", work_dir_name); + let mut attempt = 0; + loop { + let work_dir_name = if attempt == 0 { + format!("pkh-build-{}", base_timestamp) + } else { + format!("pkh-build-{}-{}", base_timestamp, attempt) + }; - // Create the directory on the host filesystem - let host_path = Path::new(&self.path).join("tmp").join(&work_dir_name); - std::fs::create_dir_all(&host_path)?; + let work_dir_inside_chroot = format!("/tmp/{}", work_dir_name); + let host_path = Path::new(&self.path).join("tmp").join(&work_dir_name); - debug!( - "Created work directory: {} (host: {})", - work_dir_inside_chroot, - host_path.display() - ); + // Check if directory already exists + if host_path.exists() { + attempt += 1; + continue; + } - // Return the path as it appears inside the chroot - Ok(work_dir_inside_chroot) + // Create the directory on the host filesystem + std::fs::create_dir_all(&host_path)?; + + debug!( + "Created work directory: {} (host: {})", + work_dir_inside_chroot, + host_path.display() + ); + + // Return the path as it appears inside the chroot + return Ok(work_dir_inside_chroot); + } } fn copy_path(&self, src: &Path, dest: &Path) -> io::Result<()> { diff --git a/src/deb/mod.rs b/src/deb/mod.rs index 9659a9c..aed63d5 100644 --- a/src/deb/mod.rs +++ b/src/deb/mod.rs @@ -105,6 +105,7 @@ fn find_dsc_file( #[cfg(test)] mod tests { + use serial_test::serial; async fn test_build_end_to_end( package: &str, series: &str, @@ -159,8 +160,16 @@ mod tests { ); } + // Tests below will be marked 'serial' + // As builds are using ephemeral contexts, tests running on the same + // process could use the ephemeral context of another thread and + // interfere with each other. + // FIXME: This is not ideal. In the future, we might want to + // either explicitely pass context (instead of shared state) or + // fork for building? #[tokio::test] #[test_log::test] + #[serial] async fn test_deb_hello_ubuntu_end_to_end() { test_build_end_to_end("hello", "noble", None, None, false).await; } @@ -168,6 +177,7 @@ mod tests { #[tokio::test] #[test_log::test] #[cfg(target_arch = "x86_64")] + #[serial] async fn test_deb_hello_ubuntu_cross_end_to_end() { test_build_end_to_end("hello", "noble", None, Some("riscv64"), true).await; }