Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

polkadot-service: Fix flaky tests #6376

Merged
merged 1 commit into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 0 additions & 26 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
1 change: 0 additions & 1 deletion polkadot/node/service/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
59 changes: 23 additions & 36 deletions polkadot/node/service/src/workers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);

Expand All @@ -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)
}

Expand Down Expand Up @@ -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";

Expand All @@ -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: $#\"
Expand All @@ -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");
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -422,7 +411,6 @@ echo {}
}

#[test]
#[serial]
fn workers_version_mismatch_throws_error() {
let bad_version = "v9.9.9.9";

Expand Down Expand Up @@ -474,7 +462,6 @@ echo {}
}

#[test]
#[serial]
fn should_find_valid_workers() {
// Test bin location.
with_temp_dir_structure(|tempdir, _| {
Expand Down
Loading