Skip to content

Commit 656560a

Browse files
committed
Fix flaky HTTP integration test startup
Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
1 parent 450c551 commit 656560a

3 files changed

Lines changed: 49 additions & 19 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/testing-framework/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ http-body-util = { workspace = true }
1010
log = "0.4"
1111
nix = "0.29"
1212
reqwest = { workspace = true }
13+
serde_json = { workspace = true }
1314
test-environment = { workspace = true }
1415
spin-app = { path = "../../crates/app" }
1516
spin-http = { path = "../../crates/http" }
@@ -18,3 +19,4 @@ spin-runtime-factors = { path = "../../crates/runtime-factors" }
1819
spin-trigger = { path = "../../crates/trigger" }
1920
spin-trigger-http = { path = "../../crates/trigger-http" }
2021
tokio = { workspace = true }
22+
url = { workspace = true }

tests/testing-framework/src/runtimes/spin_cli.rs

Lines changed: 45 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,15 @@ impl SpinCli {
5656
spin_config: SpinConfig,
5757
env: &mut TestEnvironment<R>,
5858
) -> anyhow::Result<Self> {
59-
let port = get_random_port()?;
6059
let mut spin_cmd = Command::new(spin_config.binary_path);
6160
let child = spin_cmd
6261
.envs(env.env_vars())
6362
.arg("up")
6463
.current_dir(env.path())
65-
.args(["--listen", &format!("127.0.0.1:{port}")])
64+
// Bind an OS-assigned free port on Spin's own listener and read the actual
65+
// port back from its startup output. Pre-allocating a port here is racy
66+
// because parallel tests can claim it before `spin up` binds.
67+
.args(["--listen", "127.0.0.1:0"])
6668
.args(spin_config.spin_up_args)
6769
.stdout(Stdio::piped())
6870
.stderr(Stdio::piped());
@@ -72,24 +74,31 @@ impl SpinCli {
7274
let mut child = child.spawn()?;
7375
let stdout = OutputStream::new(child.stdout.take().unwrap());
7476
let stderr = OutputStream::new(child.stderr.take().unwrap());
75-
log::debug!("Awaiting spin binary to start up on port {port}...");
77+
log::debug!("Awaiting spin binary to report its listening port...");
7678
let mut spin = Self {
7779
process: child,
7880
stdout,
7981
stderr,
80-
io_mode: IoMode::Http(port),
82+
io_mode: IoMode::None,
8183
};
8284
let start = std::time::Instant::now();
8385
loop {
84-
match std::net::TcpStream::connect(format!("127.0.0.1:{port}")) {
85-
Ok(_) => {
86-
log::debug!("Spin started on port {port}.");
87-
return Ok(spin);
88-
}
89-
Err(e) => {
90-
let stderr = spin.stderr.output_as_str().unwrap_or("<non-utf8>");
91-
log::trace!("Checking that the Spin server started returned an error: {e}");
92-
log::trace!("Current spin stderr = '{stderr}'");
86+
// `spin up` prints its base URL once the listener is bound and the app is
87+
// loaded. Observing it confirms readiness and tells us the real port the OS
88+
// assigned to this Spin instance.
89+
let found_port = spin.stdout.output_as_str().and_then(parse_serving_port);
90+
if let Some(port) = found_port {
91+
match std::net::TcpStream::connect(("127.0.0.1", port)) {
92+
Ok(_) => {
93+
log::debug!("Spin started on port {port}.");
94+
spin.io_mode = IoMode::Http(port);
95+
return Ok(spin);
96+
}
97+
Err(e) => {
98+
log::trace!(
99+
"Spin reported port {port}, but it is not accepting connections yet: {e}"
100+
);
101+
}
93102
}
94103
}
95104
if let Some(status) = spin.try_wait()? {
@@ -107,7 +116,8 @@ impl SpinCli {
107116
std::thread::sleep(std::time::Duration::from_millis(50));
108117
}
109118
anyhow::bail!(
110-
"`spin up` did not start server or error after two minutes. stderr:\n\t{}",
119+
"`spin up` did not report a listening port within two minutes.\nstdout:\n\t{}\nstderr:\n\t{}",
120+
spin.stdout.output_as_str().unwrap_or("<non-utf8>"),
111121
spin.stderr.output_as_str().unwrap_or("<non-utf8>")
112122
)
113123
}
@@ -278,9 +288,25 @@ enum IoMode {
278288
None,
279289
}
280290

281-
/// Uses a track to ge a random unused port
282-
fn get_random_port() -> anyhow::Result<u16> {
283-
Ok(std::net::TcpListener::bind("localhost:0")?
284-
.local_addr()?
285-
.port())
291+
fn parse_serving_port(output: &str) -> Option<u16> {
292+
parse_plain_serving_port(output).or_else(|| parse_json_serving_port(output))
293+
}
294+
295+
fn parse_plain_serving_port(output: &str) -> Option<u16> {
296+
output
297+
.lines()
298+
.filter_map(|line| line.trim().strip_prefix("Serving "))
299+
.find_map(parse_base_url_port)
300+
}
301+
302+
fn parse_json_serving_port(output: &str) -> Option<u16> {
303+
let output: serde_json::Value = serde_json::from_str(output).ok()?;
304+
output
305+
.get("base_url")
306+
.and_then(serde_json::Value::as_str)
307+
.and_then(parse_base_url_port)
308+
}
309+
310+
fn parse_base_url_port(base_url: &str) -> Option<u16> {
311+
url::Url::parse(base_url).ok()?.port_or_known_default()
286312
}

0 commit comments

Comments
 (0)