Repository navigation
chore(tests): ratchet real wall-clock waits in unit tests - #8954
Conversation
…-patterns real-wait) New `real-wait` rule in scripts/check-test-patterns.ts flags a unit test that waits on the wall clock with no fake timers in scope: - `sleep`/`interruptibleSleep` resolved through their `@sim/utils/helpers` import binding (aliases included; skipped when the file mocks that module), - `setTimeout` from `node:timers/promises`, - `new Promise((resolve) => setTimeout(resolve, ms))` and variants whose timer callback resolves the promise (reject-only timeout guards are not waits). Zero or omitted delays are macrotask flushes and are not flagged. Fake timers cover a wait when an earlier `vi.useFakeTimers()` in an enclosing function or the module, or a `beforeEach`/`beforeAll` of an enclosing describe or the module, fakes `setTimeout` (`toFake: ['Date']` and `shouldAdvanceTime: true` do not). Integration and *.live.test.ts files are exempt: their waits are on real services. Each occurrence is a numbered entry, capping the count per file. `--update --init-rule <rule>` admits the one-time baseline for a rule the baseline does not track yet; `--update` stays shrink-only. Baseline: 121 real-wait entries across 46 files (72 -> 193 total).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 4/5
scripts/check-test-patterns.tscan exemptawait sleep(100)that runs beforevi.useFakeTimers()in the same hook. Limit the exemption to waits after fake timers are installed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/check-test-patterns.ts">
<violation number="1" location="scripts/check-test-patterns.ts:383">
P2: This marks a hook-installed clock active across its whole suite, so `await sleep(100)` before `vi.useFakeTimers()` in the same hook is incorrectly exempted. Exclude hook-body waits that occur before installation while retaining the exemption for test callbacks.</violation>
</file>
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
Turn on auto-fix | Re-trigger cubic
… its own hook A beforeEach/beforeAll that installs fake timers covered its whole suite, including waits earlier in the same hook body, which run on the real clock. Only waits after the install in that hook are exempt now; tests and other hooks in the suite keep the exemption.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Turn on auto-fix | Re-trigger cubic
…it-rule - Remove `--update --init-rule`: it admitted entries for any rule with no remaining baseline entries. `--update` is strictly shrink-only again; the real-wait entries enter once, in this PR's baseline diff. - `vi.useRealTimers()` ends the exemption from where it runs, including a nested suite restoring real timers after an outer hook faked them. - A clock installed in `beforeEach` no longer covers `beforeAll`/`afterAll` or earlier sibling `beforeEach` hooks; `beforeAll` no longer covers earlier sibling `beforeAll` hooks. - Only `vi.useFakeTimers()` with no config, or a literal config proven to fake setTimeout without shouldAdvanceTime, exempts a wait. - Namespace imports (`helpers.sleep`, `timers.setTimeout`) are flagged.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
…ons declared above it Functions run only after the module loads, so a module-scope vi.useFakeTimers() (or vi.useRealTimers()) applies to every function body regardless of source order; module-scope waits still follow source order.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
Turn on auto-fix | Re-trigger cubic
…y when nothing ran before it A helper called at module scope before vi.useFakeTimers() waits on the real clock. A module-scope install now reaches function bodies only when every earlier module-scope statement is load-safe (imports, function/class declarations, type-only statements, vi.mock/doMock/hoisted, and variables whose initializers call nothing); otherwise source order applies. A module-scope vi.useRealTimers() still reaches every function body.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
Turn on auto-fix | Re-trigger cubic
… install Any unrelated module-scope call (const run = vi.fn()) used to drop the helper exemption. Now a module-scope vi.useFakeTimers() covers every function body except a helper (function declaration or variable bound to a function) that earlier module-scope code calls directly by name; calls through other helpers are not traced. WAIT_EXPORTS and HOOK_KINDS are as const.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
Turn on auto-fix | Re-trigger cubic
…ed in a type assertion helperName now unwraps as/satisfies/non-null/parenthesized initializers with the shared unwrap helper, so a wrapped arrow called before a module-scope vi.useFakeTimers() stays flagged.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
Turn on auto-fix | Re-trigger cubic
This branch was successfully deployed
1 participant
Summary
sleep(in tests went from 138 to 220 in 30 days, even thoughsim-testing.mdalready says "No real timers".use-chat.dom.test.tsxalone waits about 17s for real. This change makes that rule enforced instead of advisory, using the same approach as eslint-plugin-playwright'sno-wait-for-timeout.New
real-waitrule incheck:test-patterns, applied to*.test.ts(x)only:Flagged
sleepandinterruptibleSleep, resolved through their@sim/utils/helpersimport (aliases included)setTimeoutfromnode:timers/promisesnew Promiseresolved bysetTimeoutNot flagged
@sim/utils/helpers*.integration.tsand*.live.test.ts, which wait on real servicesFake timers: a wait passes when an earlier
vi.useFakeTimers()fakessetTimeoutfor it, either in the same function or module, or in abeforeEach/beforeAllof an enclosingdescribe.toFake: ['Date']andshouldAdvanceTime: truedon't count. Each wait is its own numbered baseline entry, so adding a wait to an already-baselined file still fails.Initial baseline: 121 existing waits across 46 files. The baseline normally only shrinks, so it gets a one-time
--update --init-rule <rule>, which admits entries only for a known rule the baseline doesn't track yet. It refuses unknown rules and rules already tracked. No other rule's entries change.Tests: 28 table-driven fixture tests in
check-test-patterns.test.ts. Each of the 16 guards was reverted in turn and turned at least one test red.Test plan
check:audits(check:test-patternswith 193 baselined entries)check-test-patterns.test.ts)