Skip to content

feat(tests): make the suite runnable under bun test --parallel - #1023

Merged
TheGreatAxios merged 7 commits into
mainfrom
cl-7909-make-the-test-suite-runnable-under-bun-test-parallel-track
Sep 14, 2026
Merged

TheGreatAxios merged 7 commits into
mainfrom
cl-7909-make-the-test-suite-runnable-under-bun-test-parallel-track

Conversation

@TheGreatAxios

Copy link
Copy Markdown
Collaborator

Summary

  • Adds bun run test:parallel, a seeded bun test --parallel wrapper with a stall watchdog that kills livelocked runs and retries them
  • Exposes config overrides for the tool-watchdog salvage grace, exec-shutdown teardown deadline, and agents-panel linger so timing-sensitive tests inject deterministic values
  • Replaces wall-clock sleeps with condition polling, flush barriers, and a frozen clock in the approval-log, transcript-anchor, and stall-recovery tests

Verification

  • bun run check passes, including the serial suite (7313 pass, 0 fail)
  • scripts/test-parallel.test.ts covers the watchdog retry and argument handling

Fixes CL-7909

@linear-code

linear-code Bot commented Sep 14, 2026

Copy link
Copy Markdown

CL-7909

A stall-declared run that exits on its own is never a stall: SIGTERM death reads as null or 143, any other code is the child's own and must survive unretried.
@TheGreatAxios

Copy link
Copy Markdown
Collaborator Author

Critic review · Comment (merge-ready with doc nits)

Note: this PR is feat(tests): make the suite runnable under bun test --parallel (16 files), not a PerfTrace doc fix — reviewing what is actually here.

Mechanism verified by reading the diff plus runtime probes on Bun 1.4.0: detached: true does put the child in its own process group (pgid == pid; without it pgid == parent group), so the group SIGTERM/SIGKILL stall kill and the grandchild-reap test are sound. Bun.spawn + detached also passes this repo's tsc flags clean. Approval-log flush() chains synchronously at settle() time, so awaiting it post-decision is deterministic. Linger/salvage/teardown overrides thread correctly (constructor arg order, spread order, and Partial forwarding all check out), and CI (.github/workflows/ci.yml) is indeed still test:paths-only.

Should-fix (doc accuracy, non-blocking)

  • scripts/test-parallel.ts header + docs/IMPLEMENTATION.md parallel-runs bullet — the failure signature ("worker spins at 100% CPU holding a zombie git child") contradicts the cited upstream issue: bun test --parallel intermittently deadlocks: one worker wedges with all threads in condvar waits (lost wakeup), parent hangs forever oven-sh/bun#36235 is now Closed, and its diagnostics say the opposite — all worker threads parked in condvar waits, nothing spinning, and "no child processes". Either describe the locally-observed signature without pinning it on #36235, or re-cite. "Still open as of 1.4.2" is stale.
  • src/tui/transcript-anchor.test.ts frameWith — returns the last frame on deadline instead of throwing, so a genuine never-paints failure surfaces as a confusing content diff rather than a timeout. Consider an explicit timeout assertion.

Nits

  • src/index.ts teardownDeadlineMs — process-global, last-wins, no validation; the documented single-setter assumption makes this fine, noting only.
  • src/subagent/nudge-director.test.ts frozenNow — frozen clock means the stall nudge can never fire in those three tests; correct for non-timing tests, just do not copy the pattern into a stall-behavior test.

Verdict

Merge-ready. The watchdog design is signature-agnostic, so the issue-citation drift does not affect correctness; fix the citation wording before or just after merge.

@TheGreatAxios
TheGreatAxios merged commit 1a94470 into main Sep 14, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants