test(chromium): stop electron boot tests depending on host CDP ports - #709
Draft
latekvo wants to merge 1 commit into
Draft
test(chromium): stop electron boot tests depending on host CDP ports#709latekvo wants to merge 1 commit into
latekvo wants to merge 1 commit into
Conversation
boot-electron-spawn-error.test.ts drove bootElectronApp against hardcoded CDP ports 19222-19226 and relied on nothing answering there. Any Chromium or Electron endpoint on one of those ports lets waitForCdpReady win bootElectronApp's readiness race, so the boot resolves and detaches the child's boot listeners before the test emits its synthetic spawn error; the emit then throws on a listener-less EventEmitter instead of rejecting. 19222 is argent's own example port and appears in nine sibling test files, so a live endpoint there is a realistic host condition rather than a hypothetical one. Route those boots through a single UNREACHABLE_CDP_PORT constant pinned to the privileged port 1, which no unprivileged process can bind. Apply the same fix to the one sibling sharing the shape: chromium-discovery's dead-port pruning test asserted an unreachable port in the ephemeral range, which a listen(0) fake CDP server in another test file can be handed. Also redirect ARGENT_CHROMIUM_PORTS_FILE to a throwaway path. The file's successful-boot test ran trackChromiumPort against the real ~/.argent/chromium-cdp-ports.json and appended an ephemeral port on every run; both sibling files already guard against that.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
boot-electron-spawn-error.test.tsfails intermittently with a single failure:It is not load, and it is not the timeout
I chased the obvious hypothesis first — vitest's 5000 ms default colliding with a ~3.5 s built-in wait — and it is wrong. Measured at
origin/mainbefore any edit:38/38, a 0% spontaneous failure rate. And the one test whose
readyTimeoutMsis 5000 against vitest's 5000 ms default completes in 15-54 ms at load ~20 — a ~100x margin, nowhere near the boundary.The real trigger, and it is deterministic
Anything on the host listening on the hardcoded CDP ports the test uses. Squatting one makes it fail 100% of the time:
Mechanism. When a real endpoint answers,
waitForCdpReadywinsbootElectronApp'sPromise.racein ~1 ms — long before the test's 10 mssetTimeoutfires its synthetic error. The boot then resolves,detachBootListeners()removes the'error'listener, and the test'schild.emit("error", err)reaches a listener-less EventEmitter, so Node throws instead of the promise rejecting.The source is correct throughout. The test binds a global shared resource it does not own, and
19222is argent's own documented example CDP port — used in nine sibling test files and by everychromium-cdp-19222device this repo's docs tell people to create. A developer with an Electron app open, or a second agent on the same machine, is enough.Fix
One
UNREACHABLE_CDP_PORT = 1constant. Port 1 is privileged, so no unprivileged process can bind it and the probe cannot be answered by accident — the idiom three tests in this same file already used. Not a timeout widening.Second defect, same file
This file was the only one of the three Electron boot tests that did not isolate port persistence, so its success-path boot wrote a real ephemeral port into the developer's actual
~/.argent/chromium-cdp-ports.jsonon every run. I watched it grow 55 → 56 → 91 entries across a session.Both siblings already guard this — one mocks the module, the other redirects
ARGENT_CHROMIUM_PORTS_FILE, with a comment stating tests "never touch the real" file. This file now redirects too. Verified by md5: before this change a run mutates the real file, after it the file is byte-identical.Sibling fixed
chromium-discovery.test.tsasserted port43211was unreachable, but that sits inside Linux's ephemeral range (/proc/sys/net/ipv4/ip_local_port_range→32768 60999), so alisten(0)fake CDP server in a parallel test file can legitimately be handed it. Same fix.Left alone, verified safe:
boot-electron-kill.test.ts,flows/flow-chromium-boot.test.ts,list-devices.test.ts— each owns its port vialisten(0)or fully mocks the launcher. The fixed ports43210,54321,54322are only ever tracked or untracked, never network-probed.Verification
origin/mainand re-run after the change with the squatter still live.19223with a squatter present fails four named tests —registers an 'error' listener…,rejects…ENOENT,rejects…EACCES,detaches BOTH boot listeners after a FAILURE path….eslint .0,prettier --check .0,tsc --build0 — unpiped, exit codes read directly.packages/tool-server: identical to theorigin/mainbaseline over 3 runs. The 6boot-device-hotbootfailures on that baseline are a separate host-environment defect, fixed in test(tool-server): stop unit tests inheriting the developer's ARGENT_* overrides #707, not touched here.