mirror of
https://github.com/Jeuners/ECC.git
synced 2026-09-09 23:12:30 +02:00
d904edc615
Two round-1 review findings in `tests/lib/session-bridge.test.js`,
both about test correctness rather than the underlying fix:
1. **greptile P1 + coderabbitai Major + cubic P2 (all three): concurrent-write test ran sequentially.**
The test spawned two child processes with two consecutive
`spawnSync` calls. Because `spawnSync` blocks until the child
exits, the second writer started *after* the first finished —
the two writers never overlapped, so the rename race the fix
targets was never actually exercised. The test would have passed
with the old broken `${target}.tmp` suffix.
Fix: introduce a one-off "race runner" helper that runs inside
its own subprocess and uses async `spawn` to start both writers
simultaneously. The runner waits for both to exit (the event
loop is local to the runner subprocess, so this stays compatible
with the synchronous test harness used elsewhere in this file)
and reports both exit codes plus stderrs on stdout. The test
then calls the runner via `spawnSync` and parses the result.
Both writer children now overlap for the duration of their 200
`writeBridgeAtomic` calls each, which is enough wall time to
reliably trigger the rename race against the pre-fix code.
Verified: with the fixed `${target}.${pid}.${nonce}.tmp` suffix,
the test passes; with the old fixed `${target}.tmp` suffix
reintroduced, it fails as expected (one writer hits ENOENT on
roughly half its rename calls).
2. **greptile P2 + cubic P3: `assert.throws` used a string as the second argument.**
Node deprecated passing a string as the second argument to
`assert.throws` years ago: the string is silently treated as
the assertion failure message (what to print when the function
does *not* throw) rather than as an error matcher. The check
passed for any thrown error, not just the rename failure.
Fix: pass a regex matcher as the second arg and keep the
explanatory text as the third. The regex matches `EISDIR`,
`EPERM`, `ENOTDIR`, or `ENOENT` because `renameSync` of a
regular tmp file onto an existing directory raises different
codes on Linux / macOS / BSD — making the matcher portable
across CI runners.
Test count unchanged at 14; `npm test` green; `npm run lint` clean.
The two helper files (`tests/__tmp_bridge_writer.js`,
`tests/__tmp_bridge_race_runner.js`) are written and unlinked
inside the test's try/finally so they never persist beyond the
test run.
|
||
|---|---|---|
| .. | ||
| agent-compress.test.js | ||
| changed-files-store.test.js | ||
| command-plugin-root.test.js | ||
| cost-estimate.test.js | ||
| inspection.test.js | ||
| install-config.test.js | ||
| install-executor.test.js | ||
| install-lifecycle.test.js | ||
| install-manifests.test.js | ||
| install-request.test.js | ||
| install-state.test.js | ||
| install-targets.test.js | ||
| locale-install.test.js | ||
| mcp-config.test.js | ||
| observer-sessions.test.js | ||
| orchestration-session.test.js | ||
| package-manager.test.js | ||
| project-detect.test.js | ||
| resolve-ecc-root.test.js | ||
| resolve-formatter.test.js | ||
| selective-install.test.js | ||
| session-adapters.test.js | ||
| session-aliases.test.js | ||
| session-bridge.test.js | ||
| session-manager.test.js | ||
| shell-split.test.js | ||
| skill-dashboard.test.js | ||
| skill-evolution.test.js | ||
| skill-improvement.test.js | ||
| state-store.test.js | ||
| tmux-worktree-orchestrator.test.js | ||
| utils.test.js | ||