Skip to content

fix: the browser test teardown sweeps again, so CI stops going red - #17

Merged
T-Crypt merged 2 commits into
oh-my-pocketfrom
fix/ci-teardown-sweep
Oct 13, 2026
Merged

T-Crypt merged 2 commits into
oh-my-pocketfrom
fix/ci-teardown-sweep

Conversation

@T-Crypt

@T-Crypt T-Crypt commented Oct 13, 2026

Copy link
Copy Markdown
Owner

CI is red on oh-my-pocket on every merge, and it is one cause, not four: a browser that has just been closed can still be writing its profile while the test's temporary folder is removed. The four push runs of today's merges all show it, always on the Node 22.19 leg, always the same message:

not ok 260 - test/pi-sessions-ui.test.ts
  error: "ENOTEMPTY: directory not empty, rmdir '/tmp/pi-pocket-test-.../browser/profile/Default'"
Run Node 22.19 Node 24
38429736022 (merge #11) fail, pi-sessions-ui pass
38429841208 (merge #12) fail fail, Timed out waiting for the title
38429902721 (merge #13) fail pass
38429961722 (merge #16) fail, providers-ui pass

What changed

test/helpers.ts: cleanUp() used rmSync(root, { recursive: true, force: true, maxRetries: 10, retryDelay: 200 }). Node's retry does not re-read the tree: it sweeps the children once and then retries a bare rmdir, so a file the browser writes afterwards fails all eleven attempts, which is what the eleven second teardown says. The removal now sweeps a freshly read tree on every attempt, and pauses a quarter of a second between attempts, up to twenty. Any error other than ENOTEMPTY is rethrown at once.

test/browser.test.ts: two waits for the page's title now allow thirty seconds instead of the helper's ten. A freshly launched browser on a loaded runner answered one of them at 10.6 s, and the page's title is static, so the wait is for the browser to answer at all. until still returns on the first poll that sees it.

Proof

  • The failing teardown is in the log of all four runs above, each inside its own after hook.
  • This branch, npm test: 640 tests, 640 pass, 0 fail, 0 skipped, 79 s, with the browser tests running.
  • npm run check, npm run lint, npm run format:check: exit 0.
  • The mechanism is Node's own rmSync: on 22.19 it retries the rmdir without re-reading, which the four runs' eleven second teardowns match.

Open

  • Node 24 bit this once with the title wait made the teardown visible, and no run of this branch has been on a runner yet.
  • The review that found this also reported three holes in scripts/check-web.mjs (a tsc configuration error exits 2 and is accepted as "cannot run"; an untrimmed web-clean.txt line never matches; no maxBuffer on the findings) and four groups INVISIBLE_CLASS names that test/fence.test.ts does not pin. None of them can redden a push run; they are a follow-up.
  • The browser closed. still cannot name its own cause: the DevTools pipe's close event closes the connection before the child's exit handler can report the exit code and the browser's stderr. Worth its own ticket.

@T-Crypt
T-Crypt merged commit 847f87c into oh-my-pocket Oct 13, 2026
1 of 2 checks passed
@T-Crypt
T-Crypt deleted the fix/ci-teardown-sweep branch October 14, 2026 15:13
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.

1 participant