Skip to content

test: random listen ports make the integration tier fail unreproducibly (3.45% per run) #174

Description

@AlexJeffcott

Summary

Eight files pick a listen port as 30000 + Math.floor(Math.random() * 10000) with no retry and no port-0 bind. A collision fails listen(), and the test reads as flake — so the natural response is a retry, which is exactly the wrong instinct.

Context

Background

Found while verifying #173. A bun run test:tiers --all run failed with integration.bun-test — exit code 1. It did not reproduce: the tier passed 5/5 standalone, 3/3 through the tier runner, and 3/3 tiers together. The failing run took 10776ms against a normal 4633ms, which is the signature of a bind failure and retry-free abort rather than a slow assertion.

The change under test (tools/analysis/src/extract/handlers.ts) cannot reach tests/integration/ — those are relay and signalling tests and none imports the extractor. The failure was pre-existing.

This is the same shape as #173: a real defect that presents as flake, where retrying makes it worse rather than better.

Current State

packages/polly/tools/test/src/e2e-mesh/with-relay.ts:47-51 states the risk and accepts it:

function pickPort(): number {
  // Same window the integration suite uses. Random port collision is
  // possible but rare; e2e scripts run sequentially by default so the
  // failure surface is small.
  return 30000 + Math.floor(Math.random() * 10000);
}

That reasoning holds for the e2e scripts it was written for. It does not hold for tests/integration/, where 27 pickPort() call sites run in one bun test process:

file call sites
tests/integration/signaling-server.test.ts 5
tests/integration/peer-repo-relay.test.ts 4
tests/integration/peer-state-resilience.test.ts 4
tests/integration/peer-state-relay.test.ts 2
tests/integration/signaling-client-reconnect.test.ts 2
tests/integration/signaling-custom-frames.test.ts 2
tests/integration/signaling-peer-notifications.test.ts 2

27 draws from a 10,000-port window is a 3.45% self-collision probability per integration run (birthday bound, 1 - exp(-n(n-1)/2N)). That is a lower bound: it counts only the suite colliding with itself, not with Docker, dev servers, or anything else holding a port in 30000–39999. This machine runs Docker containers continuously.

No site retries. signaling-server.test.ts:118 is representative:

const port = pickPort();
const app = new Elysia().use(signalingServer({ path: "/polly/signaling" })).listen(port);

Desired State

A test binds a port it is guaranteed to own. Port 0 asks the kernel for a free port and returns it, which removes the collision class entirely rather than making it rarer.

Requirements

Functional Requirements

  • Replace pickPort() with a helper that binds port 0 and reports the assigned port
  • Put the helper in one shared module rather than duplicating it across 8 files
  • Update all 27 call sites in tests/integration/ to use it
  • Update tools/test/src/e2e-mesh/with-relay.ts:47 to use it, and delete the comment that accepts the risk
  • Where port 0 is genuinely unavailable (a port must be known before the server starts), retry on EADDRINUSE with a bounded attempt count and fail with the bind error, not a timeout

Edge Cases

  • Two servers needed at once (peer-state-resilience.test.ts:80-81 binds portA and portB): both must be distinct and both live at the same time — a helper that binds, reads the port and closes before the real listen reintroduces the race
  • Elysia .listen(0): confirm the assigned port is readable from the app instance (app.server?.port) before any client connects
  • A test that passes the port into a child process or a URL string must read the resolved port, not the requested 0

Acceptance Criteria

  • grep -rn "30000 + Math.floor" packages/polly returns nothing
  • bun test --cwd tests integration passes 66/66
  • bun run test:tiers --all passes with the integration tier green
  • Running the integration suite concurrently with a second copy of itself does not fail on bind — the current code fails this by construction

Implementation Notes

Files to Modify

  • packages/polly/tests/integration/signaling-server.test.ts:40 — definition; 5 call sites
  • packages/polly/tests/integration/peer-repo-relay.test.ts:52 — definition; 4 call sites
  • packages/polly/tests/integration/peer-state-resilience.test.ts:75 — definition; 4 call sites (two concurrent)
  • packages/polly/tests/integration/peer-state-relay.test.ts:81 — definition; 2 call sites
  • packages/polly/tests/integration/signaling-client-reconnect.test.ts:54 — definition; 2 call sites
  • packages/polly/tests/integration/signaling-custom-frames.test.ts:50 — definition; 2 call sites
  • packages/polly/tests/integration/signaling-peer-notifications.test.ts:41 — definition; 2 call sites
  • packages/polly/tools/test/src/e2e-mesh/with-relay.ts:47-51 and :86 — definition and listen

Suggested Approach

Bind port 0 and read back what the kernel assigned:

const app = new Elysia().use(signalingServer({ path })).listen(0);
const port = app.server?.port;

The server owns the port from the moment it is assigned, so there is no window in which another process can take it. That is strictly better than any retry loop, which only narrows the window.

Similar Patterns

packages/polly/tools/test/src/e2e-cli/with-temp-dir.ts:21-31 is the repo's mint-plus-cleanup shape (withTempDir(prefix) -> { dir, cleanup }). A withRelay-style helper returning { port, url, close } would match it — with-relay.ts already returns that shape and only needs its port source changed.

Dependencies

Out of Scope

  • The coverage.enforce timeout seen in the same run — filed separately
  • Changing how the tier engine reports a failing case
  • Port selection in tools/test/src/browser/run.ts:84 unless it shares the helper naturally

Technical Notes

Observed across three bun run test:tiers --all runs on the same machine, same day:

run integration tier
1 pass, 4633ms
2 fail, exit code 1, 10776ms
3 pass, 5807ms

Isolation runs after the failure: 5/5 pass via bun test --cwd tests integration, 3/3 pass via bun run test:tiers integration. The failure is load- and timing-dependent, not deterministic.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions