Repository navigation
fix: the browser test teardown sweeps again, so CI stops going red - #17
Merged
Merged
Conversation
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.
CI is red on
oh-my-pocketon 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:pi-sessions-uiTimed out waiting for the titleproviders-uiWhat changed
test/helpers.ts:cleanUp()usedrmSync(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 barermdir, 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 thanENOTEMPTYis 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.untilstill returns on the first poll that sees it.Proof
afterhook.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.rmSync: on 22.19 it retries thermdirwithout re-reading, which the four runs' eleven second teardowns match.Open
scripts/check-web.mjs(a tsc configuration error exits 2 and is accepted as "cannot run"; an untrimmedweb-clean.txtline never matches; nomaxBufferon the findings) and four groupsINVISIBLE_CLASSnames thattest/fence.test.tsdoes 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.