test(cli): give the e2e suite a realistic timeout - #380
Merged
Merged
Conversation
The CLI e2e tests spawn the built CLI and call the live Argos API, so they are bound by network latency rather than local compute. Most ran on vitest's 5s default, which is too tight on a loaded runner: in CI run 34607908169 the `e2e-core (24, macos-latest)` cell failed because `build get 999999` timed out at 5000ms, while the other 8 matrix cells passed. Split vitest.config.ts into `unit` and `e2e` projects so the e2e files get a 30s testTimeout, plus a matching 30s hookTimeout for the six files that seed state from the API in `beforeAll` (the default hook budget is 10s). Unit tests keep the 5s default. That makes the hand-written per-test timeouts redundant, so drop them from comment, deploy, skip, upload, upload-oidc and upload-tokenless. The three e2e scripts keep working: they pass path and tag filters, which vitest applies across projects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
Description
The CLI e2e tests spawn the built CLI and call the live Argos API, so they are bound by network latency rather than local compute. Most of them ran on vitest's 5s default, which is too tight on a loaded runner.
In CI run 34607908169 the
e2e-core (24, macos-latest)cell failed becauseargos build get 999999timed out at 5000ms, while the other 8 matrix cells passed. That is runner and API latency, not a code defect.packages/cli/vitest.config.tsis now split into two vitest projects so only the e2e files get a generous budget:unit(src)e2e(e2e)testTimeouthookTimeouthookTimeoutis raised too: six e2e files seed state from the live API insidebeforeAll, and that hook runs on its own 10s budget that the same latency would blow.Centralizing the timeout makes the hand-written per-test timeouts redundant, so they are removed from
comment,deploy,skip,upload,upload-oidcandupload-tokenless. Three of those were set below 30s, so leaving them would have made those tests stricter than the rest of the suite.Type of changes
bugChecklist
Optional checks:
Further comments
What a reviewer should know
The three e2e scripts still work.
e2e,e2e-oidcande2e-tokenlesspass path and tag filters, which vitest applies across projects.tagsstays declared at the root and both projects inherit it viaextends: true. OnlytagsFilteris a root-only option in vitest;tags,testTimeoutandhookTimeoutare all project-level.Verification.
check-types,lint,check-formatand the unit suite (84 tests) all pass. The live e2e suite was not run, as no API token was available locally. Instead:vitest list(collects without executing, so no API calls) confirms all three scripts resolve their files, and that the tag filter yields 63 tests vs 64 unfiltered, correctly dropping the OIDC one.beforeAllpasses where the 10s default would have failed it; and the same 6s test placed insrcstill fails at 5000ms, confirming the split does not leak into unit tests.packages/core/e2eneeded nothing. It is plain node scripts with no vitest involved.One thing worth a second opinion: the
e2e-corejobs in.github/workflows/ci.ymlsettimeout-minutes: 5. A pathological run that now burns 30s per failing test could hit the job ceiling and report as a cancelled job rather than a clean test timeout. Left untouched here as it is out of scope, but happy to bump it in this PR if you'd prefer.🤖 Generated with Claude Code