polkadot-service: Fix flaky tests (#6376)

The tests used the same paths. When run on CI, each test is run in its
own process and thus, this "serial_test" crate wasn't used. The tests
are now using their own thread local tempdir, which ensures that the
tests are working when running in parallel in the same program or when
being run individually.

Cargo.toml

@@ -1210,7 +1210,6 @@ serde-big-array = { version = "0.3.2" }
 serde_derive = { version = "1.0.117" }
 serde_json = { version = "1.0.132", default-features = false }
 serde_yaml = { version = "0.9" }
-serial_test = { version = "2.0.0" }
 sha1 = { version = "0.10.6" }
 sha2 = { version = "0.10.7", default-features = false }
 sha3 = { version = "0.10.0", default-features = false }

polkadot/node/service/Cargo.toml

@@ -140,7 +140,6 @@ polkadot-node-subsystem-test-helpers = { workspace = true }
 polkadot-primitives-test-helpers = { workspace = true }
 sp-tracing = { workspace = true }
 assert_matches = { workspace = true }
-serial_test = { workspace = true }
 tempfile = { workspace = true }
 
 [features]

polkadot/node/service/src/workers.rs

@@ -21,19 +21,20 @@ use is_executable::IsExecutable;
 use std::path::PathBuf;
 
 #[cfg(test)]
-use std::sync::{Mutex, OnceLock};
+thread_local! {
+	static TMP_DIR: std::cell::RefCell<Option<tempfile::TempDir>> = std::cell::RefCell::new(None);
+}
 
 /// Override the workers polkadot binary directory path, used for testing.
 #[cfg(test)]
-fn workers_exe_path_override() -> &'static Mutex<Option<PathBuf>> {
-	static OVERRIDE: OnceLock<Mutex<Option<PathBuf>>> = OnceLock::new();
-	OVERRIDE.get_or_init(|| Mutex::new(None))
+fn workers_exe_path_override() -> Option<PathBuf> {
+	TMP_DIR.with_borrow(|t| t.as_ref().map(|t| t.path().join("usr/bin")))
 }
+
 /// Override the workers lib directory path, used for testing.
 #[cfg(test)]
-fn workers_lib_path_override() -> &'static Mutex<Option<PathBuf>> {
-	static OVERRIDE: OnceLock<Mutex<Option<PathBuf>>> = OnceLock::new();
-	OVERRIDE.get_or_init(|| Mutex::new(None))
+fn workers_lib_path_override() -> Option<PathBuf> {
+	TMP_DIR.with_borrow(|t| t.as_ref().map(|t| t.path().join("usr/lib/polkadot")))
 }
 
 /// Determines the final set of paths to use for the PVF workers.
@@ -147,12 +148,9 @@ fn list_workers_paths(
 
 	// Consider the /usr/lib/polkadot/ directory.
 	{
-		#[allow(unused_mut)]
-		let mut lib_path = PathBuf::from("/usr/lib/polkadot");
+		let lib_path = PathBuf::from("/usr/lib/polkadot");
 		#[cfg(test)]
-		if let Some(ref path_override) = *workers_lib_path_override().lock().unwrap() {
-			lib_path = path_override.clone();
-		}
+		let lib_path = if let Some(o) = workers_lib_path_override() { o } else { lib_path };
 
 		let (prep_worker, exec_worker) = build_worker_paths(lib_path, workers_names);
 
@@ -175,9 +173,10 @@ fn get_exe_path() -> Result<PathBuf, Error> {
 	let mut exe_path = std::env::current_exe()?;
 	let _ = exe_path.pop(); // executable file will always have a parent directory.
 	#[cfg(test)]
-	if let Some(ref path_override) = *workers_exe_path_override().lock().unwrap() {
-		exe_path = path_override.clone();
+	if let Some(o) = workers_exe_path_override() {
+		exe_path = o;
 	}
+
 	Ok(exe_path)
 }
 
@@ -205,8 +204,7 @@ mod tests {
 	use super::*;
 
 	use assert_matches::assert_matches;
-	use serial_test::serial;
-	use std::{env::temp_dir, fs, os::unix::fs::PermissionsExt, path::Path};
+	use std::{fs, os::unix::fs::PermissionsExt, path::Path};
 
 	const TEST_NODE_VERSION: &'static str = "v0.1.2";
 
@@ -228,7 +226,7 @@ mod tests {
 
 	fn get_program(version: &str) -> String {
 		format!(
-			"#!/bin/bash
+			"#!/usr/bin/env bash
 
 if [[ $# -ne 1 ]] ; then
     echo \"unexpected number of arguments: $#\"
@@ -253,27 +251,21 @@ echo {}
 	) -> Result<(), Box<dyn std::error::Error>> {
 		// Set up <tmp>/usr/lib/polkadot and <tmp>/usr/bin, both empty.
 
-		let tempdir = temp_dir();
-		let lib_path = tempdir.join("usr/lib/polkadot");
-		let _ = fs::remove_dir_all(&lib_path);
-		fs::create_dir_all(&lib_path)?;
-		*workers_lib_path_override().lock()? = Some(lib_path);
+		let tempdir = tempfile::tempdir().unwrap();
+		let tmp_dir = tempdir.path().to_path_buf();
+		TMP_DIR.with_borrow_mut(|t| *t = Some(tempdir));
 
-		let exe_path = tempdir.join("usr/bin");
-		let _ = fs::remove_dir_all(&exe_path);
-		fs::create_dir_all(&exe_path)?;
-		*workers_exe_path_override().lock()? = Some(exe_path.clone());
+		fs::create_dir_all(workers_lib_path_override().unwrap()).unwrap();
+		fs::create_dir_all(workers_exe_path_override().unwrap()).unwrap();
 
+		let custom_path = tmp_dir.join("usr/local/bin");
 		// Set up custom path at <tmp>/usr/local/bin.
-		let custom_path = tempdir.join("usr/local/bin");
-		let _ = fs::remove_dir_all(&custom_path);
-		fs::create_dir_all(&custom_path)?;
+		fs::create_dir_all(&custom_path).unwrap();
 
-		f(tempdir, exe_path)
+		f(tmp_dir, workers_exe_path_override().unwrap())
 	}
 
 	#[test]
-	#[serial]
 	fn test_given_worker_path() {
 		with_temp_dir_structure(|tempdir, exe_path| {
 			let given_workers_path = tempdir.join("usr/local/bin");
@@ -318,7 +310,6 @@ echo {}
 	}
 
 	#[test]
-	#[serial]
 	fn missing_workers_paths_throws_error() {
 		with_temp_dir_structure(|tempdir, exe_path| {
 			// Try with both binaries missing.
@@ -368,7 +359,6 @@ echo {}
 	}
 
 	#[test]
-	#[serial]
 	fn should_find_workers_at_all_locations() {
 		with_temp_dir_structure(|tempdir, _| {
 				let prepare_worker_bin_path = tempdir.join("usr/bin/polkadot-prepare-worker");
@@ -394,7 +384,6 @@ echo {}
 	}
 
 	#[test]
-	#[serial]
 	fn should_find_workers_with_custom_names_at_all_locations() {
 		with_temp_dir_structure(|tempdir, _| {
 			let (prep_worker_name, exec_worker_name) = ("test-prepare", "test-execute");
@@ -422,7 +411,6 @@ echo {}
 	}
 
 	#[test]
-	#[serial]
 	fn workers_version_mismatch_throws_error() {
 		let bad_version = "v9.9.9.9";
 
@@ -474,7 +462,6 @@ echo {}
 	}
 
 	#[test]
-	#[serial]
 	fn should_find_valid_workers() {
 		// Test bin location.
 		with_temp_dir_structure(|tempdir, _| {