-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: add timeout configurations to prevent flaky tests in CI #6442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add explicit test timeout (15s), hook timeout (10s), and teardown timeout (10s) to vitest config - Configure retry logic for CI environments to handle occasional flaky tests - Add asyncUtilTimeout configuration to testing library to fail faster - Resolves issue where tests would hang indefinitely in CI showing dots continuously Fixes the flaky test issue reported in GitHub Actions run 16625858640
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for addressing the CI timeout issues! I've reviewed the changes and found some important inconsistencies between the two vitest configurations that should be addressed to ensure consistent behavior across the project.
| environment: "jsdom", | ||
| include: ["src/**/*.spec.ts", "src/**/*.spec.tsx"], | ||
| // Add timeout configurations to prevent flaky tests in CI | ||
| testTimeout: 15000, // 15 seconds for individual tests (increased from default 5s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timeout values here (15s/10s) are inconsistent with src/vitest.config.ts which uses 20s/20s. Should these be aligned for consistency across the project? If there's a specific reason for different timeouts, could we document it in comments?
| // Add timeout configurations to prevent flaky tests in CI | ||
| testTimeout: 15000, // 15 seconds for individual tests (increased from default 5s) | ||
| hookTimeout: 10000, // 10 seconds for setup/teardown hooks | ||
| teardownTimeout: 10000, // 10 seconds for teardown |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This teardownTimeout configuration is missing from src/vitest.config.ts. If teardown timeouts are needed for CI stability, shouldn't both test suites have this configuration?
| hookTimeout: 10000, // 10 seconds for setup/teardown hooks | ||
| teardownTimeout: 10000, // 10 seconds for teardown | ||
| // Retry flaky tests once in CI environments | ||
| retry: process.env.CI ? 1 : 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This retry logic for CI environments is missing from src/vitest.config.ts. If CI flakiness is a project-wide issue, wouldn't both test suites benefit from retry logic?
|
|
||
| configure({ | ||
| // Reduce default timeout for waitFor to fail faster in CI | ||
| asyncUtilTimeout: 5000, // 5 seconds instead of default 1000ms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This asyncUtilTimeout configuration only affects webview-ui tests. Are there similar async utilities used in src tests that might benefit from timeout configuration?
Summary
This PR fixes the flaky test issue reported in GitHub Actions run 16625858640 where tests would hang indefinitely in CI environments, showing dots continuously until timeout.
Root Cause
The issue was caused by missing timeout configurations in the vitest setup. Tests were running without explicit timeouts, which could cause them to hang indefinitely in CI environments where timing and resource constraints are different from local development.
Changes Made
vitest.config.ts
vitest.setup.ts
Testing
Impact
This should resolve the CI timeout issues while maintaining test reliability.
Important
Adds timeout configurations in
vitest.config.tsandvitest.setup.tsto prevent flaky tests in CI by increasing timeouts and adding retry logic.vitest.config.ts:testTimeout: Increased to 15s for individual tests.hookTimeoutandteardownTimeout: Set to 10s.retry: Added 1 retry for CI environments, 0 locally.vitest.setup.ts:asyncUtilTimeout: Set to 5s for@testing-library/reactwaitForoperations.This description was created by
for ebadc1c. You can customize this summary. It will automatically update as commits are pushed.