fix: increase browser screenshot test timeouts from 20s to 30s#2059
fix: increase browser screenshot test timeouts from 20s to 30s#2059nick-inkeep merged 1 commit intomainfrom
Conversation
The nested error state browser test flakes on CI due to tight timeout budget — 20s total with 15s reserved for toMatchScreenshot leaves only 5s for Playwright init, React render, Monaco boot, and form validation. Bump all three browser screenshot tests to 30s for consistent headroom. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This PR increases browser screenshot test timeouts from 20s to 30s across three tests in agents-manage-ui. The change is well-justified and addresses a legitimate CI flakiness issue.
Analysis
The timeout adjustment is appropriate because:
-
Tight budget with previous timeout: The
toMatchScreenshot()matcher is configured with a 15-second timeout invitest.config.ts(line 64). With the previous 20-second test timeout, only 5 seconds remained for Playwright initialization, React component rendering, Monaco Editor boot, and form validation. -
Variable CI performance: Slower CI runners can exceed this tight headroom, causing intermittent failures even when tests pass locally.
-
Consistent application: All three browser screenshot tests now use the same 30-second timeout, providing uniform behavior.
-
No performance impact: As the author notes, timeouts are ceilings — passing tests complete at normal speed regardless of the configured maximum.
Verification
- ✅ Change is minimal and well-scoped (+3/-3 lines)
- ✅ All browser screenshot tests now have consistent timeout values
- ✅ No production code affected — purely test infrastructure
- ✅ No customer-facing surface impact
✅ APPROVE
Summary: Clean fix for flaky CI browser tests. The 30-second timeout provides appropriate headroom given the 15-second toMatchScreenshot matcher configuration. Ship it! 🚀
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-tests |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
The nested error state browser test flakes on CI due to tight timeout budget — 20s total with 15s reserved for toMatchScreenshot leaves only 5s for Playwright init, React render, Monaco boot, and form validation. Bump all three browser screenshot tests to 30s for consistent headroom. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
20_000to30_000ms (form.browser.test.tsxx2,nodes.browser.test.tsxx1)should properly highlight nested error statecaused by tight headroom — thetoMatchScreenshot()matcher alone reserves 15s of the 20s budget, leaving insufficient time for Playwright init + Monaco boot + form validation on slower CI runnersTest plan
agents-manage-uitests unaffected