[Test] UI: Add vitest coverage for 10 untested components#24239
[Test] UI: Add vitest coverage for 10 untested components#24239yuneng-jiang merged 1 commit intolitellm_yj_march_20_2026from
Conversation
Add tests for ResponseTimeIndicator, KeyValueInput, QueryParamInput, RoutePreview, OnboardingModal, EditUserModal, ModelFilters, AuditLogDrawer, PassThroughInfoView, and EmailSettings. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds vitest + React Testing Library test coverage for 10 previously untested UI components, bringing 69 new tests across rendering, conditional rendering, user interaction, and callback-verification scenarios. All new tests correctly use mocks for network calls (satisfying the repo's CI safety requirements), and the global Key findings:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/email_settings.test.tsx | Adds 8 tests for EmailSettings; the "disable premium fields for non-premium users" test is vacuous — mockAlerts lacks premium-gated keys, so the assertion never exercises the guarded code path. |
| ui/litellm-dashboard/src/components/model_filters.test.tsx | Adds 8 tests for ModelFilters; the Card-wrapper test relies on a Tremor-internal CSS class (.tremor-Card-root) and the describe-level vi.fn() mock is not reset between tests — otherwise solid coverage. |
| ui/litellm-dashboard/src/components/onboarding_link.test.tsx | Adds 8 tests for OnboardingModal; URL generation expectations are verified against the actual getInvitationUrl() logic and all match correctly. |
| ui/litellm-dashboard/src/components/pass_through_info.test.tsx | Adds 10 tests for PassThroughInfoView with correct mocks for networking, security, and guardrails sub-components; admin vs non-admin tab assertions align with component logic. |
| ui/litellm-dashboard/src/components/view_logs/AuditLogDrawer/AuditLogDrawer.test.tsx | Adds 9 tests for AuditLogDrawer; table name mapping, diff section rendering, and close callback are all verified against the actual component implementation. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
ST["setupTests.ts\n(global mocks: Tremor Button/Switch,\nNotificationsManager,\nmatchMedia, ResizeObserver)"]
subgraph "New Test Files"
T1["response_time_indicator.test.tsx\n4 tests"]
T2["key_value_input.test.tsx\n5 tests"]
T3["query_param_input.test.tsx\n5 tests"]
T4["route_preview.test.tsx\n7 tests"]
T5["onboarding_link.test.tsx\n8 tests"]
T6["edit_user.test.tsx\n5 tests"]
T7["model_filters.test.tsx\n8 tests ⚠️"]
T8["AuditLogDrawer.test.tsx\n9 tests"]
T9["pass_through_info.test.tsx\n10 tests"]
T10["email_settings.test.tsx\n8 tests ⚠️"]
end
ST --> T1 & T2 & T3 & T4 & T5 & T6 & T7 & T8 & T9 & T10
T4 -->|"vi.mock('./networking')"| NM1["networking mock\ngetProxyBaseUrl"]
T9 -->|"vi.mock('./networking')\nvi.mock sub-components"| NM2["networking mock\nupdatePassThrough\ndeletePassThrough"]
T10 -->|"vi.mock('./networking')\nvi.mock('./email_events')"| NM3["networking mock\nEmailEventSettings mock"]
T7 -.->|"⚠️ .tremor-Card-root brittle CSS class\n⚠️ shared vi.fn() not reset"| WARN1["Flakiness risk"]
T10 -.->|"⚠️ premium-gate test is vacuous\n(no EMAIL_LOGO_URL in mockAlerts)"| WARN2["False coverage"]
Last reviewed commit: "[Test] UI: Add vites..."
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| expect(screen.queryByText("Clear Filters")).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("should render without Card wrapper when showFiltersCard is false", () => { | ||
| const { container } = render(<ModelFilters {...defaultProps} showFiltersCard={false} />); | ||
| // When showFiltersCard=false, it renders a plain div instead of a Card | ||
| expect(container.querySelector(".tremor-Card-root")).toBeNull(); | ||
| }); |
There was a problem hiding this comment.
container.querySelector(".tremor-Card-root") relies on the Tremor library emitting a CSS class named tremor-Card-root. That class name is not part of any public API contract and can change silently across Tremor versions, causing the test to produce a false pass (always null) or a false fail without any code change.
A more robust assertion would verify the structural difference between the two render paths — e.g., checking that when showFiltersCard is false, no <article> (Tremor Card typically renders as <article>) wraps the filter controls, or checking that the search input is still reachable:
| expect(screen.queryByText("Clear Filters")).not.toBeInTheDocument(); | |
| }); | |
| it("should render without Card wrapper when showFiltersCard is false", () => { | |
| const { container } = render(<ModelFilters {...defaultProps} showFiltersCard={false} />); | |
| // When showFiltersCard=false, it renders a plain div instead of a Card | |
| expect(container.querySelector(".tremor-Card-root")).toBeNull(); | |
| }); | |
| it("should render without Card wrapper when showFiltersCard is false", () => { | |
| const { container } = render(<ModelFilters {...defaultProps} showFiltersCard={false} />); | |
| // When showFiltersCard=false the outermost element is a plain <div>, not a Tremor Card (<article>) | |
| expect(container.firstChild?.nodeName).toBe("DIV"); | |
| expect(container.querySelector("article")).toBeNull(); | |
| }); |
| onFilteredDataChange: vi.fn(), | ||
| }; | ||
|
|
||
| it("should render", () => { |
There was a problem hiding this comment.
vi.fn() not reset between tests
defaultProps.onFilteredDataChange is a single vi.fn() instance defined at describe scope and shared by every test that renders with defaultProps. Because cleanup() re-mounts the component but does not call vi.clearAllMocks(), call counts and recorded arguments from one test bleed into the next.
This is harmless today because only one test asserts on the mock's calls (and it instantiates its own vi.fn() locally), but it is a latent source of flakiness for any future assertion on the shared mock.
Add a beforeEach to reset it:
| onFilteredDataChange: vi.fn(), | |
| }; | |
| it("should render", () => { | |
| const defaultProps = { | |
| modelHubData: mockData, | |
| onFilteredDataChange: vi.fn(), | |
| }; | |
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| }); |
| render(<EmailSettings {...defaultProps} premiumUser={false} />); | ||
| // EMAIL_LOGO_URL and EMAIL_SUPPORT_CONTACT should have a sparkle prefix for non-premium | ||
| // They'll still show but with different rendering | ||
| expect(screen.getByText("SMTP_HOST")).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("should render EmailEventSettings sub-component", () => { |
There was a problem hiding this comment.
The test is named "should disable premium fields for non-premium users" and its comment says EMAIL_LOGO_URL and EMAIL_SUPPORT_CONTACT should render differently for non-premium users — but mockAlerts only contains SMTP fields, so those two premium-gated keys are never present in the rendered output.
The actual assertion (expect(screen.getByText("SMTP_HOST")).toBeInTheDocument()) passes identically whether premiumUser is true or false. The branching logic in the component (premiumUser != true && (key === "EMAIL_LOGO_URL" || key === "EMAIL_SUPPORT_CONTACT")) is never reached, making the test a no-op for the behaviour it claims to cover.
To actually test premium gating, mockAlerts.variables needs EMAIL_LOGO_URL and EMAIL_SUPPORT_CONTACT entries, and the assertion should verify the premium-specific rendering (e.g., the ✨ sparkle/link vs. the plain text label):
const mockAlertsWithPremiumFields = [
{
name: "email",
variables: {
SMTP_HOST: "smtp.example.com",
EMAIL_LOGO_URL: "logo.png",
EMAIL_SUPPORT_CONTACT: "[email protected]",
},
},
];
it("should show sparkle prefix for premium fields for non-premium users", () => {
render(<EmailSettings {...defaultProps} premiumUser={false} alerts={mockAlertsWithPremiumFields} />);
// Non-premium: premium-gated key rendered inside a link (sparkle), not as plain text
expect(screen.queryByText("EMAIL_LOGO_URL")).not.toBeInTheDocument();
expect(screen.getByText(/✨ EMAIL_LOGO_URL/)).toBeInTheDocument();
});
yuneng-jiang
merged commit 75bd742
into
litellm_yj_march_20_2026
1 participant
Summary
Problem
Several UI components had no test coverage, making it harder to catch regressions during refactors.
Fix
Added vitest + React Testing Library tests for 10 previously untested components, covering rendering, conditional rendering, user interactions, and callback behavior.
Testing
All 69 tests pass across 10 new test files:
ResponseTimeIndicator(4 tests) - null rendering, value formattingKeyValueInput(5 tests) - add/remove pairs, onChange callbacksQueryParamInput(5 tests) - add/remove params, onChange callbacksRoutePreview(7 tests) - conditional rendering, URL composition, subpath toggleOnboardingModal(8 tests) - invitation vs reset password modes, URL generation, SSOEditUserModal(5 tests) - null user guard, field rendering, cancel callbackModelFilters(8 tests) - search filtering, dropdowns, clear filtersAuditLogDrawer(9 tests) - table name mapping, diff sections, close buttonPassThroughInfoView(10 tests) - badges, HTTP methods, admin vs non-admin tabsEmailSettings(8 tests) - SMTP fields, required markers, premium gatingType
✅ Test