Repository navigation
test(portal): wait for an unrelated child's READY and exit past a loaded runner's start, not 2 s - #2409
Conversation
…ded runner's start, not 2 s "running Portal > does not treat an arbitrary interpreter as a Portal script" starts a stand-in Bun process and waits for its READY line, racing it against a 2 s bound. On a host at load ~70 a Bun start takes longer: it timed out at ~2.0 s five times in local check fast runs across four lanes. The wait stays on the child's own signal (READY, exit); the bound (CHILD_MS, 20 s) only names a hung child, under the case's own timeout (CASE_MS). The shell case next to it, with the same pattern, gets the same. Fail-first with a scratch 2.5 s sleep before READY: old 2/2 timed out, new 3/3 passed. portal.test.ts passed 5/5 (38 tests); bun run check fast ok.
📝 Summary
Merge Risk:
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly identifies the primary change: extending portal child-process waits beyond the 2-second limit. It is specific and related to the changeset, although somewhat long. |
| Description check | |
The description explains the failure, cause, affected tests, implementation, unchanged behavior, and validation results. It includes the required Change and Validation information, although skipped ch… |
| Docstring Coverage | |
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Description check
-
Explanation
The description explains the failure, cause, affected tests, implementation, unchanged behavior, and validation results. It includes the required Change and Validation information, although skipped checks are not explicitly listed.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.)
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Merging 52090d4. Test-only flake fix from the flake-hunt lane: the two portal.test.ts cases that start a stand-in Bun child already wait for its own READY line and its exit, but bounded them at the shared 2 s, which a loaded runner can exceed just starting Bun (5 local failures at load ~70). The child bound is now 20 s (it only names a hung child) with a case timeout above it; the signals waited for are unchanged. Diff read by the merge pass; test-only, so no CodeRabbit review needed. |
Found by the flake-hunt lane, from a Dori pin.
server/portal.test.ts"running Portal > does not treat an arbitrary interpreter as a Portal script" timed out at about 2.0 s ("Timed out: unrelated interpreter executed its script") in five localcheck fastruns across four lanes:It failed at a host load of about 70 and passes alone.
Cause (test flake)
The case starts a stand-in Bun process (
process.execPath <script>) and waits for itsREADYline on stdout. The signal is right, but the wait raced it against 2 s of wall clock, and a loaded host can take longer than that to start a Bun process.Change
READYline or its exit. The bound (CHILD_MS, 20 s) only names a child that hangs. It is far above any start time and below the case's own 30 s timeout.shwith the same pattern and the same 2 s bound but has not failed yet.boundedanduntilwaits in the file, which watch the service's own state rather than a process start.Proof
READY, the old test failed 2/2 with CI's "Timed out: unrelated interpreter executed its script", and the new one passed 3/3.bun test server/portal.test.tspassed 5/5 (38 tests).bun run check fastok.Test-only.
Review
omo-native-code-reviewer: APPROVE (CLEAR). It confirmed that both cases wait on an exact signal, the
READYline or the exit, and that the 30 s case timeout is required: Bun's 5 s default would cut a slow start off first. It found no clearly better injection: the case needs a real non-shell interpreter, becauseportal.tstreats only sh-family shells as script runners, andprocess.execPathis the only interpreter guaranteed to be installed.CASE_MS = CHILD_MS + 10_000)./bin/sh, and none has failed.portal versionstart::407,:811,:904,:971and:1128.bounded(service.shutdown())exit waits::189,:242,:489and others. Portal's own SIGKILL fallback comes after 4 s.Test-only, so no Codex review (notes/pr-policy.md).