trending/OpenShell
1
0
Fork 0
mirror of https://github.com/NVIDIA/OpenShell.git synced 2026-10-30 07:33:59 +08:00
Code Issues Packages Projects Releases Wiki Activity

test(cli): retry forward tests when another process takes the port (#4570)

Browse Source 
* test(cli): pick forward test ports below the ephemeral range

The forward tests released an OS-assigned port before the CLI bound it,
so a concurrent test's bind to port 0 could claim it. Pick ports below
the Linux and macOS ephemeral ranges from a random offset instead.

Closes #4569

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* test(cli): retry forward tests when another process takes the port

Replace the fixed port range with a retry. The CLI checks a forward port
before binding it, so a concurrent test can claim it in between; on that
error, pick a new OS-assigned port and run the forward again.

Refs #4569

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

---------

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
This commit is contained in:
Matthew Grossman
2026-10-14 21:54:49 +00:00
committed by GitHub
parent d20c090a69
commit 4dd6151578
1 changed files with 60 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs
+60 -64
View File
@@ -2991,25 +2991,25 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() {
.run_async(async {
let tls = test_tls(&server);
install_fake_forwarding_ssh(&fake_ssh_dir);
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let forward_port = listener.local_addr().unwrap().port();
drop(listener);
run::sandbox_create(
&server.endpoint,
"openshell",
run::SandboxCreateConfig {
name: Some("persistent-forward"),
keep: false,
forward: Some(openshell_core::forward::ForwardSpec::new(forward_port)),
command: &["echo".into(), "OK".into()],
..test_config()
},
"default",
&tls,
)
.await
.expect("sandbox create with forward should succeed");
let (endpoint, tls) = (server.endpoint.as_str(), &tls);
let (forward_port, result) = with_free_forward_port(|spec| async move {
run::sandbox_create(
endpoint,
"openshell",
run::SandboxCreateConfig {
name: Some("persistent-forward"),
keep: false,
forward: Some(spec),
command: &["echo".into(), "OK".into()],
..test_config()
},
"default",
tls,
)
.await
})
.await;
result.expect("sandbox create with forward should succeed");
assert!(deleted_names(&server).await.is_empty());
let record = openshell_core::forward::read_forward_pid(
@@ -3104,21 +3104,14 @@ async fn sandbox_forward_background_tracks_owned_child_when_pid_discovery_fails(
let tls = test_tls(&server);
install_fake_forwarding_ssh(&fake_ssh_dir);
install_fake_pgrep_no_match(&fake_ssh_dir);
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let forward_port = listener.local_addr().unwrap().port();
drop(listener);
let spec = openshell_core::forward::ForwardSpec::new(forward_port);
run::sandbox_forward(
&server.endpoint,
"owned-forward",
&spec,
true,
&tls,
"default",
)
.await
.expect("background forward should track the owned SSH child without PID discovery");
let (endpoint, tls) = (server.endpoint.as_str(), &tls);
let (forward_port, result) = with_free_forward_port(|spec| async move {
run::sandbox_forward(endpoint, "owned-forward", &spec, true, tls, "default").await
})
.await;
result.expect(
"background forward should track the owned SSH child without PID discovery",
);
let record =
openshell_core::forward::read_forward_pid("default", "owned-forward", forward_port)
.expect("owned background forward should write a PID file");
@@ -3147,21 +3140,15 @@ async fn sandbox_forward_foreground_fails_when_ssh_exits_before_listener_opens()
.run_async(async {
let tls = test_tls(&server);
install_fake_ssh(&fake_ssh_dir);
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let forward_port = listener.local_addr().unwrap().port();
drop(listener);
let spec = openshell_core::forward::ForwardSpec::new(forward_port);
let err = run::sandbox_forward(
&server.endpoint,
"foreground-forward",
&spec,
false,
&tls,
"default",
)
.await
.expect_err("foreground forward should fail when ssh exits before listener readiness");
let (endpoint, tls) = (server.endpoint.as_str(), &tls);
let (_, result) = with_free_forward_port(|spec| async move {
run::sandbox_forward(endpoint, "foreground-forward", &spec, false, tls, "default")
.await
})
.await;
let err = result.expect_err(
"foreground forward should fail when ssh exits before listener readiness",
);
let msg = format!("{err}");
assert!(
msg.contains("ssh exited before local forward listener opened"),
@@ -3181,21 +3168,12 @@ async fn sandbox_forward_background_terminates_owned_child_when_listener_never_o
test_env(&fake_ssh_dir, &xdg_dir).run_async(async {
let tls = test_tls(&server);
let fake_forward = install_fake_unreachable_forwarding_ssh(&fake_ssh_dir);
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let forward_port = listener.local_addr().unwrap().port();
drop(listener);
let spec = openshell_core::forward::ForwardSpec::new(forward_port);
let err = run::sandbox_forward(
&server.endpoint,
"unreachable-forward",
&spec,
true,
&tls,
"default",
)
.await
.expect_err("background forward should fail when the listener never opens");
let (endpoint, tls) = (server.endpoint.as_str(), &tls);
let (forward_port, result) = with_free_forward_port(|spec| async move {
run::sandbox_forward(endpoint, "unreachable-forward", &spec, true, tls, "default").await
})
.await;
let err = result.expect_err("background forward should fail when the listener never opens");
let msg = format!("{err}");
assert!(
msg.contains("ssh process started but local forward listener was not reachable"),
@@ -3852,6 +3830,24 @@ fn reserve_local_port() -> u16 {
listener.local_addr().unwrap().port()
}
/// Run a CLI forward on a free loopback port, retrying on a new port if another
/// process takes it first. The CLI checks the port before it binds it, which can
/// be seconds later for `sandbox create`, so a concurrent test can claim it.
async fn with_free_forward_port<F, Fut, T>(mut forward: F) -> (u16, miette::Result<T>)
where
F: FnMut(openshell_core::forward::ForwardSpec) -> Fut,
Fut: Future<Output = miette::Result<T>>,
{
for _ in 0..10 {
let port = reserve_local_port();
let result = forward(openshell_core::forward::ForwardSpec::new(port)).await;
if !matches!(&result, Err(err) if err.to_string().contains("is already in use")) {
return (port, result);
}
}
panic!("another process took the forward port on every attempt");
}
fn spawn_service_forward(
server: &TestServer,
sandbox: &str,