test: give container-awaiting beforeAll hooks a 120s timeout

[alii]

What this does

describeWithContainer (and the valkey/autobahn tests' direct ensure() callers) await compose up --wait-timeout 60 plus a compose build step inside beforeAll, but the hook had bun:test's default 5s timeout. On a cold or busy machine the hook fired first and the runner's auto-killer SIGTERMed the in-flight compose subprocess; compose then exited non-zero with stderr at whatever progress line it was on, which doUp() reported as Failed to start service X: Container ... Creating.

This bumps the four beforeAll hooks that await ensure() to 120s — matching the existing pattern at websocket-proxy.test.ts:602. test/docker/index.ts is unchanged.

Why not the alternatives

• Retry compose up once (earlier version of this PR): wrong layer — the failure is bun:test killing compose, not compose actually failing. Retrying papers over the symptom.
• Serialize doUp() across services: not needed. 4 services brought up concurrently from cold all return exit=0. The coordinator (ci: serialize docker service startup through a per-shard coordinator #32033) solved a cross-process race (warmup-ci.ts vs the first test process); within one bun bd test process the per-service in-flight dedup at test/docker/index.ts:200-217 already covers same-service calls, and cross-service concurrency works.
• Spawn a coordinator for local dev: heavyweight when one process already owns all compose calls.

Verification

2× docker compose down then bun bd test sql-mysql.test.ts sql-onconnect-onclose-throw.test.ts sql-mysql.auth.test.ts from cold: 0 hook timeouts, 0 'Failed to start service' across 5 services starting concurrently.

Separate pre-existing flake (out of scope)

sql-mysql.test.ts:325 'should not timeout in long results' (10K inserts + 3 selects, 10s test timeout) can fail on a freshly-cold MySQL and pass warm. Unchanged by this PR; different timeout class (per-test 10s, not the 5s hook timeout fixed here).

[robobun]

^{Updated 9:21 PM PT - Jun 12th, 2026}

✅ @alii, your commit 780f4e4e439bfbb995c94700e0342835a92abf1d passed in Build #62198! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 32211

That installs a local version of the PR into your bun-32211 executable, so you can run:

bun-32211 --bun

[coderabbitai]

Walkthrough

Test hook timeout bounds and documentation are adjusted across test fixtures to accommodate cold Docker container startup. The harness layer documents startup timing expectations globally, and test files apply matching 120-second timeouts with clarifying comments about compose wait-timeout and build latency.

Changes

Cold Container Startup Documentation and Timeouts

Layer / File(s)	Summary
Harness-level startup timing documentation test/harness.ts	describeWithContainer now documents cold container startup expectations and how hook timeout/runner termination interact with compose up --wait-timeout 60 and the one-shot retry behavior.
Applied test timeouts and per-suite documentation test/js/valkey/test-utils.ts, test/js/valkey/valkey.test.ts, test/js/web/websocket/autobahn.test.ts	Valkey and websocket test suites increase beforeAll timeout to 120 seconds and add documentation explaining that setupDockerContainer() cold-start can exceed default hook timeouts due to compose wait-timeout plus build/cache latency.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: extending the beforeAll hook timeout to 120 seconds for container-awaiting tests.
Description check	✅ Passed	The description comprehensively covers what the PR does, why the changes were made, alternatives considered, and verification steps. It follows the template structure with clear sections.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

LGTM — straightforward test-infra deflake (longer beforeAll timeout + one-shot compose-up retry).

Extended reasoning...

Overview

This PR touches two test-infrastructure files only:

• test/harness.ts: passes 150_000 as the timeout to the beforeAll hook inside describeWithContainer, replacing the default 5s hook timeout that was firing before compose up --wait-timeout 60 could finish on cold starts.
• test/docker/index.ts: in doUp(), retries docker compose up -d --wait --wait-timeout 60 <service> exactly once on non-zero exit, logging the first failure's stderr. The const destructure becomes let and is reassigned via parenthesized destructuring on retry; the existing failure path (ps + logs + throw) still runs if the retry also fails.

Both changes are small, mechanical, and well-commented inline. The PR description includes a clear diagnosis (the "Creating" lines were captured progress output at the moment the 5s hook gave up, not a docker race) and cold-start verification (218/218 passing across two runs).

Security risks

None. No production/runtime code is touched — only the test harness and the docker-compose helper used by tests. No auth, crypto, network exposure, or input handling changes.

Level of scrutiny

Low. This is a flaky-test fix confined to test scaffolding. The worst-case downside of either change is that a genuinely broken container takes a bit longer to fail (one extra 60s wait, or up to 150s in the hook) — which is still bounded and surfaces the real ensure() error instead of an opaque (unnamed) hook timeout. No CODEOWNERS apply to these paths.

Other factors

The bug-hunting system found no issues. There are no outstanding reviewer comments. The retry is idempotent (up -d --wait on an already-started container just waits for health), and the change interacts correctly with the existing upPromises dedup / coordinator paths since it's contained inside doUp().

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/harness.ts`:
- Around line 1049-1053: The beforeAll timeout in test harness is set to
120_000ms which is too tight and duplicated in test/js/valkey/test-utils.ts;
define a single shared constant like COLD_START_TIMEOUT_MS = 150_000 and replace
the numeric literals used as beforeAll timeouts in both the harness (the
beforeAll block that currently ends with }, 120_000)) and the corresponding
beforeAll/timeout in valkey test-utils with that constant so both tests use the
same extended timeout and avoid drift/flakiness.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 62521346-6faa-49b6-94bd-93619aaedada

📥 Commits

Reviewing files that changed from the base of the PR and between 5c10fb8 and 780f4e4.

📒 Files selected for processing (4)

• test/harness.ts
• test/js/valkey/test-utils.ts
• test/js/valkey/valkey.test.ts
• test/js/web/websocket/autobahn.test.ts

[claude]

LGTM — straightforward test-only timeout bump matching the existing pattern in websocket-proxy.test.ts.

Extended reasoning...

Overview

This PR adds a 120_000 ms timeout argument to four beforeAll hooks that await docker-compose container startup (ensure()), along with explanatory comments. Files touched are all test infrastructure: test/harness.ts (describeWithContainer), test/js/valkey/test-utils.ts, test/js/valkey/valkey.test.ts, and test/js/web/websocket/autobahn.test.ts. No production/runtime code is modified.

Security risks

None. The change only adjusts test hook timeout values and adds comments. No auth, crypto, network, or data-handling logic is touched.

Level of scrutiny

Low. This is a mechanical test-harness fix: each hunk changes }); → }, 120_000); on a beforeAll callback plus an inline comment explaining why. It exactly mirrors the pre-existing pattern at test/js/web/websocket/websocket-proxy.test.ts:603, so it's following established convention rather than introducing a new approach. The worst-case failure mode is a test taking longer to time out, which is benign.

Other factors

The PR description gives a clear root-cause analysis (bun:test's 5s default hook timeout SIGTERMs the in-flight compose up subprocess on cold starts) and explains why the earlier retry-based approach was dropped. The bug-hunting system found no issues. The diff is small, self-contained, and the intent is unambiguous.