From 5586e03d33c03a3c1ddfb23b04a02e6a6b26c2e4 Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 9 Sep 2026 04:13:45 +0000 Subject: [PATCH] Use a static SSH fixture to eliminate executable-file races --- crates/ployz/src/connect/tests.rs | 46 ++++++++----------------------- crates/ployz/tests/fixtures/ssh | 16 +++++++++++ 2 files changed, 28 insertions(+), 34 deletions(-) create mode 100755 crates/ployz/tests/fixtures/ssh diff --git a/crates/ployz/src/connect/tests.rs b/crates/ployz/src/connect/tests.rs index 8c59121235..3959823fd1 100644 --- a/crates/ployz/src/connect/tests.rs +++ b/crates/ployz/src/connect/tests.rs @@ -1,4 +1,4 @@ -use std::{fs, os::unix::fs::PermissionsExt, process::Command as StdCommand, time::Duration}; +use std::{fs, os::unix::fs::symlink, process::Command as StdCommand, time::Duration}; use super::*; use crate::context::SshDestination; @@ -32,11 +32,7 @@ fn setup_retry_classifies_ssh_and_preserves_aggregate_cause() { #[tokio::test] async fn setup_retry_preserves_transient_failures_in_either_connection_order() { - let root = std::env::temp_dir().join(format!("ployz-ssh-retry-{}", std::process::id())); - fs::create_dir_all(&root).unwrap(); - let program = root.join("ssh"); - fs::write(&program, "#!/bin/sh\ncase \"$*\" in *transient*) echo 'Connection refused' >&2;; *) echo 'Permission denied (publickey)' >&2;; esac\nexit 255\n").unwrap(); - fs::set_permissions(&program, fs::Permissions::from_mode(0o700)).unwrap(); + let program = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/ssh"); for (hosts, retryable) in [ (["user@transient", "user@permanent"], true), (["user@permanent", "user@transient"], true), @@ -50,7 +46,7 @@ async fn setup_retry_preserves_transient_failures_in_either_connection_order() { .collect(), }; let error = - match connect_selected_with(selected, Arc::new(SystemConnector::new(&program))).await { + match connect_selected_with(selected, Arc::new(SystemConnector::new(program))).await { Err(error) => error, Ok(_) => panic!("script must reject every connection"), }; @@ -62,7 +58,6 @@ async fn setup_retry_preserves_transient_failures_in_either_connection_order() { ); } } - fs::remove_dir_all(root).unwrap(); } #[tokio::test] @@ -215,15 +210,13 @@ async fn cancelling_ssh_establishment_returns_without_waiting_for_ssh() { let program = root.join("ssh"); let _ = fs::remove_dir_all(&root); fs::create_dir_all(&root).unwrap(); - // Startup deliberately exceeds the old 50 ms cancellation deadline. - fs::write( + symlink( + concat!(env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/ssh"), &program, - "#!/bin/sh\nsleep 0.1\necho $$ > \"$0.pid\"\nexec sleep 30\n", ) .unwrap(); - fs::set_permissions(&program, fs::Permissions::from_mode(0o700)).unwrap(); let connector = SystemConnector::new(&program); - let connection = Connection::ssh(SshDestination::parse("user@example.com").unwrap()); + let connection = Connection::ssh(SshDestination::parse("user@cancel").unwrap()); let pid = tokio::time::timeout(Duration::from_secs(5), async { // Finishing the readiness branch drops the pending connection and its SSH child. @@ -307,31 +300,16 @@ async fn missing_ssh_client_survives_connection_selection() { #[tokio::test] async fn stalled_ssh_probe_obeys_configured_timeout() { - let root = std::env::temp_dir().join(format!("ployz-ssh-timeout-{}", std::process::id())); - fs::create_dir_all(&root).unwrap(); - let program = root.join("ssh"); - fs::write(&program, "#!/bin/sh\nexec sleep 30\n").unwrap(); - fs::set_permissions(&program, fs::Permissions::from_mode(0o700)).unwrap(); - let connector = SystemConnector::new(&program).with_ssh_timeout(Duration::from_millis(100)); - let connection = Connection::ssh(SshDestination::parse("user@example.com").unwrap()); - let result = tokio::time::timeout(Duration::from_secs(2), async { - loop { - let result = connector.connect(&connection).await; - // Concurrent process creation can briefly inherit the script's write descriptor. - if matches!(&result, Err(ConnectError::Io(error)) if error.kind() == io::ErrorKind::ExecutableFileBusy) { - tokio::time::sleep(Duration::from_millis(10)).await; - continue; - } - break result; - } - }) - .await - .expect("SSH setup must stop at its own deadline"); + let program = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/ssh"); + let connector = SystemConnector::new(program).with_ssh_timeout(Duration::from_millis(100)); + let connection = Connection::ssh(SshDestination::parse("user@timeout").unwrap()); + let result = tokio::time::timeout(Duration::from_secs(2), connector.connect(&connection)) + .await + .expect("SSH setup must stop at its own deadline"); assert!( matches!(result, Err(ConnectError::Io(ref error)) if error.kind() == io::ErrorKind::TimedOut), "{result:?}" ); - fs::remove_dir_all(root).unwrap(); } #[test] diff --git a/crates/ployz/tests/fixtures/ssh b/crates/ployz/tests/fixtures/ssh new file mode 100755 index 0000000000..2bd1e78ae2 --- /dev/null +++ b/crates/ployz/tests/fixtures/ssh @@ -0,0 +1,16 @@ +#!/bin/sh +# Checked in so concurrent test forks cannot inherit a writable script handle. +for argument do + case "$argument" in + user@transient) echo 'Connection refused' >&2; exit 255 ;; + user@permanent) echo 'Permission denied (publickey)' >&2; exit 255 ;; + user@cancel) + # Startup deliberately exceeds the old 50 ms cancellation deadline. + sleep 0.1 + echo $$ > "$0.pid" + exec sleep 30 + ;; + user@timeout) exec sleep 30 ;; + esac +done +exit 64