HarperFast / HarperFast/harper

Flaky unit tests: listen port derived from process.pid collides in CI (uWS adapter, keys)

Open
#2,192 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 2h
Merged PRs (30d)
205

Description

Two unit tests choose a listen port by arithmetic on the process ID instead of binding an ephemeral port, so they fail intermittently in CI with a bind error rather than an assertion.

## Symptom

```
1) uWS WebSocket adapter (createUwsServer wsHandler)
enforces wsMaxPayload by closing a connection that sends an oversized frame:
Error: uWS failed to bind :35478
at onListen (server/serverHelpers/uwsServer.ts:229:38)
at async Context. (unitTests/server/serverHelpers/uwsServer.test.js:473:25)
```

Seen on `Unit Test (Node.js v24)`, run 32046998700 (4461 passing, this 1 failing). The same job passed on re-run of the identical commit, and v22/v26 passed on the first attempt — the failure tracks port availability, not the code under test. It surfaced on an unrelated PR (CLI deploy credentials, touching nothing in `server/` or the uWS tests).

## Root cause

`unitTests/server/serverHelpers/uwsServer.test.js:389`

```js
const port = 34100 + (process.pid % 1500); // avoid collisions across concurrent suites
```

`35478 == 34100 + 1378`, so that run's mocha PID was congruent to 1378 mod 1500. The scheme narrows collisions but cannot remove them:

- two concurrent mocha processes whose PIDs are congruent mod 1500 pick the same port;
- the 34100–35599 range can collide with anything else already listening on the runner;
- a port used by an earlier suite in the same job may still be in `TIME_WAIT`.

A second instance of the same pattern, with a narrower modulus (so proportionally likelier to collide):

`unitTests/security/keys.test.js:517`

```js
let nextPort = 40000 + (process.pid % 1000);
```

## Suggested fix

Bind an ephemeral port and read back what the OS assigned, rather than guessing a free one:

1. Pass port `0` and recover the actual port (`us_listen_socket_local_port`, or surface it from `createUwsServer`/`onListen`), then point the suite's client connections at that value. If the current `createUwsServer` signature can't report the bound port, extending it to return the resolved port is worthwhile on its own — callers other than tests benefit from knowing what they actually bound.
2. Apply the same treatment to `keys.test.js`, and grep for further instances: `grep -rn 'process\.pid %' unitTests/ integrationTests/`.

Please avoid the two shortcuts that look like fixes: widening the modulus (still a guess, just a rarer collision) and wrapping the bind in a retry loop (hides a genuine bind failure, which is the thing [#2112](https://github.com/HarperFast/harper/pull/2112) went to some trouble to stop swallowing).

## Related

- #2112 — surfaces swallowed uWS startup-listen failures instead of hanging. Complementary rather than overlapping: that work is why this failure appears as a clear `failed to bind` error instead of a timeout, and it does not touch the test's port selection.

🤖 Filed by Claude Code (Opus 5) after diagnosing this failure on an unrelated PR.

Contributor guide

Open the contributing guide

Research direction

Start with unitTests/server/serverHelpers/uwsServer.test.js:389 and unitTests/security/keys.test.js:517, then inspect createUwsServer and its onListen path in server/serverHelpers/uwsServer.ts. Run the affected unit tests and grep unitTests/ and integrationTests/ for other process.pid port selection. Done means the tests use the OS-assigned port, no PID-derived port selection remains, and the affected suites pass reliably.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, nodejs
Domain
backend, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
75/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.