test(ui): preserve global Button/Tooltip mocks in per-file @tremor/react vi.mock#27958
Conversation
…act vi.mock
Per-file `vi.mock("@tremor/react", ...)` factories fully replace the
setup-level mock from `tests/setupTests.ts`, so the global Button/Tooltip
overrides are lost in any file that re-mocks `@tremor/react`. Without
them, the real Tremor `<Button>` leaks through and its internal
`useTooltip(300)` schedules a native 300ms `setTimeout` on pointer
events. When the test environment is torn down before the timer fires,
the trailing `setState` calls `getCurrentEventPriority`, which reads
`window.event` against a destroyed jsdom -> "window is not defined"
flake observed on CI.
Patches the 7 leaky test files to re-supply `Button` (bare `<button>`)
and `Tooltip` (Fragment) overrides matching `setupTests.ts`. Also drops
a dead `afterEach` workaround in `user_edit_view.test.tsx` (the
fake-timer dance it ran could not drain a real timer scheduled before
the swap) and corrects a misleading comment in `MakeMCPPublicForm.test.tsx`.
…itellm_/funny-williams-dab711
Greptile SummaryThis PR fixes a flaky
Confidence Score: 4/5Safe to merge — all changes are confined to test files and directly address a documented, reproducible CI flakiness root cause. The fix is correct and well-scoped: it re-applies the two Tremor overrides that prevent the post-teardown timer error in every affected file. The one gap is that setupTests.ts also stubs Switch as a native checkbox, and that stub is silently dropped in all 7 per-file mocks (pre-existing before this PR). Tests that interact with a Switch inside these components will exercise the real Tremor Switch rather than the lightweight test double, which could affect interaction-based assertions. All 7 patched test files are missing the Switch re-override; user_edit_view.test.tsx is most likely to be affected given its breadth of user-interaction tests.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/user_edit_view.test.tsx | Adds Button/Tooltip overrides to the per-file tremor mock and removes the dead vi.useFakeTimers workaround; Switch mock is not re-applied (pre-existing gap) |
| ui/litellm-dashboard/src/components/AIHub/forms/MakeMCPPublicForm.test.tsx | Corrects a misleading comment and adds Button/Tooltip overrides with a dynamic React import needed for React.forwardRef inside the async factory |
| ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelRetrySettingsTab.test.tsx | Adds Button/Tooltip overrides to the existing per-file tremor mock; change is minimal and correct |
| ui/litellm-dashboard/src/app/(dashboard)/teams/components/TeamsTable/ModelsCell.test.tsx | Adds Button/Tooltip overrides to the existing per-file tremor mock; change is minimal and correct |
| ui/litellm-dashboard/src/components/GuardrailsMonitor/ScoreChart.test.tsx | Adds Button/Tooltip overrides to the existing per-file tremor mock; change is minimal and correct |
| ui/litellm-dashboard/src/components/molecules/models/columns.test.tsx | Adds Button/Tooltip overrides as named locals before the return object; consistent style with surrounding code |
| ui/litellm-dashboard/src/components/policies/impact_popover.test.tsx | Adds Button/Tooltip overrides to the existing per-file tremor mock; change is minimal and correct |
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/lit..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return React.createElement("option", { value, title }, childText || title || value); | ||
| }; | ||
| SelectItem.displayName = "SelectItem"; | ||
| // Re-apply the global Button/Tooltip overrides from tests/setupTests.ts. | ||
| // A file-level vi.mock fully replaces the setup-level mock, so without this | ||
| // the real Tremor Button leaks through and its useTooltip(300) schedules a | ||
| // native setTimeout that fires post-teardown -> "window is not defined". | ||
| const Button = React.forwardRef<HTMLButtonElement, any>(({ children, ...props }, ref) => | ||
| React.createElement("button", { ...props, ref }, children), | ||
| ); | ||
| const TremorTooltip = ({ children }: any) => React.createElement(React.Fragment, null, children); | ||
| return { | ||
| ...actual, | ||
| SelectItem, | ||
| Button, | ||
| Tooltip: TremorTooltip, |
There was a problem hiding this comment.
Switch mock not re-applied alongside Button/Tooltip
setupTests.ts overrides three Tremor exports: Button, Tooltip, and Switch (renders as a native <input type="checkbox" role="switch">). Each per-file vi.mock fully replaces the setup-level factory, so every file that re-mocks @tremor/react silently restores the real Tremor Switch via ...actual. This PR re-applies Button and Tooltip but leaves Switch as the real Tremor implementation in all 7 patched files. Any test that interacts with a Switch in these files — toggling it, asserting its checked state — will be exercising the real Tremor component instead of the lightweight native checkbox the setup mock was designed to provide. This is a pre-existing gap that this PR misses an opportunity to close.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2 participants
Summary
Fixes the flaky
ReferenceError: window is not definedthat surfaces againstsrc/components/user_edit_view.test.tsx(and other UI tests) in CI.Per-file
vi.mock("@tremor/react", ...)factories fully replace the setup-level mock fromtests/setupTests.ts, so the globalButton/Tooltipoverrides are lost in any file that re-mocks@tremor/react. Without them, the real Tremor<Button>leaks through and its internaluseTooltip(300)schedules a native 300mssetTimeouton pointer events. When the test environment is torn down before the timer fires, the trailingsetStatecallsgetCurrentEventPriority, which readswindow.eventagainst a destroyed jsdom — producing the unhandled error.This PR re-supplies the
Button(bare<button>) andTooltip(Fragment) overrides in each of the 7 leaky test files. It also drops a deadafterEachworkaround inuser_edit_view.test.tsx(thevi.useFakeTimers()+vi.runAllTimers()dance could not drain the real timer scheduled before the swap) and corrects a misleading comment inMakeMCPPublicForm.test.tsx.Files touched (all are test files):
ui/litellm-dashboard/src/components/user_edit_view.test.tsxui/litellm-dashboard/src/components/AIHub/forms/MakeMCPPublicForm.test.tsxui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelRetrySettingsTab.test.tsxui/litellm-dashboard/src/app/(dashboard)/teams/components/TeamsTable/ModelsCell.test.tsxui/litellm-dashboard/src/components/molecules/models/columns.test.tsxui/litellm-dashboard/src/components/GuardrailsMonitor/ScoreChart.test.tsxui/litellm-dashboard/src/components/policies/impact_popover.test.tsxTest plan
npx vitest run ...— 107/107 tests passCI=true npx vitest run --pool forks --poolOptions.forks.maxForks=8— 3844/3844 tests pass across 383 files, run 3× consecutively with zeroUnhandled Errorsand zerowindow is not definedtsc --noEmitproduces no new type errors in any patched file