test: set a 5s repo-wide unit-suite timeout floor - #2135
Merged
Conversation
*Why* The unit suite builds Mocha with only `ui`/`color`, so mocha's 2s default applies. On cold CI runners (AV scan + cold FS cache) a test's first real I/O can exceed 2s, and the penalty attaches to whichever spawn test the glob happens to run first — an unstable contract, since adding or renaming a file under `src/` migrates the flake to the next spawn test. Raising the timeout on individual files only chases the class one file at a time. *What* Set `timeout: 5000` on the shared Mocha instance in `src/test/suite.ts`. 5s clears the cold-runner I/O spike while staying tight enough to fail a genuine hang fast. Tests that spawn the real CLI keep their own higher overrides (CliWrapper 10s, the packaging smoke describe 30s), which still win per-suite. *Verification* - `yarn test:unit` — 877 passing, 0 failing. - `yarn fix` clean. Co-authored-by: Isaac
rugpanov
had a problem deploying
to
test-trigger-is
August 20, 2026 08:38 — with 
GitHub Actions
Error
GitHub Actions
Error
Contributor
Author
|
🤖 Integration tests triggered for |
*Why* Review of the 5s timeout floor flagged two comment nits: the new suite.ts comment narrated mechanism beyond the load-bearing why (CODE_CONVENTIONS §4b), and a pre-existing shellUtils.test.ts comment justified its 60s override against "the 2s default", which this change makes stale. *What* - Trim the suite.ts timeout comment to the why plus the one fact worth stating (per-suite this.timeout() still overrides the floor); drop the mechanism narration and the rot-prone file reference. - Drop the specific "2s" from the shellUtils.test.ts comment so it doesn't lie once the repo-wide default is 5s. *Verification* - `yarn test:unit` — 877 passing, 0 failing. - `yarn fix` clean. Co-authored-by: Isaac
rugpanov
marked this pull request as ready for review
August 20, 2026 08:57
rugpanov
had a problem deploying
to
test-trigger-is
August 20, 2026 08:58 — with
GitHub Actions
Error
GitHub Actions
Error
Contributor
Author
|
🤖 Integration tests ✅ all 41 test jobs passed for |
anton-107
approved these changes
Aug 20, 2026
rugpanov
enabled auto-merge (squash)
August 21, 2026 06:23
Contributor
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
rugpanov
requested a deployment
to
test-trigger-is
August 21, 2026 06:25 — with
GitHub Actions
Queued
GitHub Actions
Queued
Merged
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
2 participants
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.
Why
The unit suite builds Mocha with only
ui/color, so mocha's 2 s default applies. On cold CI runners (AV scan + cold FS cache) a test's first real I/O can exceed 2 s, and the penalty attaches to whichever spawn test the glob happens to run first — an unstable contract, since adding or renaming a file undersrc/migrates the flake to the next spawn test. Raising the timeout per-file only chases the class one file at a time (this was the follow-up flagged in review of #2134).What
Set
timeout: 5000on the shared Mocha instance insrc/test/suite.ts. 5 s clears the cold-runner I/O spike while staying tight enough to fail a genuine hang fast. Tests that spawn the real CLI keep their own higher overrides (CliWrapper.test.ts10 s, the packaging smoke describe 30 s), which still win per-suite.Verification
yarn test:unit— 877 passing, 0 failing.eslint+prettierclean.This pull request and its description were written by Isaac.