Repository navigation
fix: eliminate two flaky tests (skill-extractor timeout, endpoints server churn) - #1375
Merged
Cedric Vidal (cedricvidal) merged 3 commits intoJul 31, 2026
Conversation
…rver churn) skill-extractor: replace execSync tar/rm with in-process node-tar. Under 2600+ parallel tests, spawning tar/rm subprocesses exceeded the 5s timeout; in-process extraction drops the isolated run from ~1935ms to ~27ms. endpoints.test.ts: bind the Express app to one long-lived server for the whole suite instead of letting supertest spin up a fresh ephemeral server per request. The per-call servers closed asynchronously, so a socket still closing from one test corrupted the next request's HTTP stream, causing random 'Parse Error: Expected HTTP/' failures and 5s timeouts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 090a5035-5871-412e-a5ea-6866f44f4931
… test suites The HAR suite — and any api test that hands the shared Express `app` straight to supertest — intermittently failed with spurious 400s, HTTP "Parse Error", and 5s timeouts. `supertest(app)` spins up and asynchronously tears down a fresh server per call, so churn from one test corrupts the next test's request; the victim rotates run to run (endpoints, then har, ...). Extract a `useTestServer(app)` helper that binds the app to one long-lived server per file (supertest reuses an already-listening server and never closes it), and adopt it across the 9 shared-`app` suites. Also migrates the earlier endpoints.test.ts inline fix onto the shared helper for a single pattern. Verified: endpoints 15/15, the 9 suites together 15/15, full pnpm test 3/3 (2602/2602). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 090a5035-5871-412e-a5ea-6866f44f4931
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 090a5035-5871-412e-a5ea-6866f44f4931
Test Results (Node.js 22)test: Run #28
🎉 All tests passed! |
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.
Summary
Fixes two independent flaky tests surfaced while verifying the suite. The full suite now passes 2602/2602 deterministically (previously 2601/2602 with a rotating failure).
Root causes & fixes
1.
skill-extractor.test.ts › copilot— subprocess spawn latencyextractSkillsToWorkspaceshelled out totar/rmviaexecSync. Under 2600+ parallel tests, subprocess spawn latency intermittently blew past the 5s test timeout.blob-storage.ts):tarExtract({ file, cwd, strip: 1, sync: true })+rmSync.execSynctarcall to in-processtarCreate.2.
endpoints.test.ts— supertest server churnrequest(app)with an Express app spins up a fresh ephemeral server per call — 159x per run. Those per-call servers and their sockets close asynchronously, so a socket still closing from test N could corrupt test N+1's HTTP stream, producingError: Parse Error: Expected HTTP/, RTSP/ or ICE/and 5s timeouts with a random victim each run.async_hookstracing confirmedTCPSERVERWRAP/TCPWRAPlingering live at every test boundary, with no application frames in the pending promises.expressApp.listen(0)inbeforeAll, closed inafterAll). All 159request(app)call sites are unchanged.Verification
pnpm --filter api build— typechecks cleanendpoints.test.tslooped 20x — 20/20 green (was ~60% pass rate before)pnpm test— 2602/2602 passed across 208 filesNotes