[Infra] UI - Unit Testing Coverage: MCP Semantic Filter#21968
[Infra] UI - Unit Testing Coverage: MCP Semantic Filter#21968yuneng-jiang merged 1 commit intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds 33 Vitest unit tests across three new test files for the
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/Settings/AdminSettings/MCPSemanticFilterSettings/MCPSemanticFilterSettings.test.tsx | New test file with 10 well-structured tests for the main settings component, covering no-token guard, loading/error states, form rendering, save button dirty-state gating, and mutation error display. Good use of mocks and async helpers. |
| ui/litellm-dashboard/src/components/Settings/AdminSettings/MCPSemanticFilterSettings/MCPSemanticFilterTestPanel.test.tsx | New test file with 12 tests for the test panel component. Good coverage of rendering, input callbacks, button states, filter-disabled warning, results display, and curl command tab. Minor style issue: fireEvent.change at line 63 is not wrapped in act() per AGENTS.md guidelines. |
| ui/litellm-dashboard/src/components/Settings/AdminSettings/MCPSemanticFilterSettings/semanticFilterTestUtils.test.ts | New test file with 10 tests for pure utility functions. Thorough coverage of input validation, API call lifecycle (isTesting toggling, result clearing), result parsing, warning/error notification paths. |
| ui/litellm-dashboard/src/components/TeamSSOSettings.test.tsx | Removed redundant local @tremor/react mock that duplicated the global mock in tests/setupTests.ts. All existing tests remain intact and benefit from the shared global mock. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["MCPSemanticFilterSettings.test.tsx\n(10 tests)"] -->|mocks| B["useMCPSemanticFilterSettings hook"]
A -->|mocks| C["useUpdateMCPSemanticFilterSettings hook"]
A -->|mocks| D["MCPSemanticFilterTestPanel"]
A -->|mocks| E["semanticFilterTestUtils"]
F["MCPSemanticFilterTestPanel.test.tsx\n(12 tests)"] -->|mocks| G["ModelSelector"]
F -->|tests| H["MCPSemanticFilterTestPanel component"]
I["semanticFilterTestUtils.test.ts\n(10 tests)"] -->|mocks| J["testMCPSemanticFilter API"]
I -->|tests| K["getCurlCommand"]
I -->|tests| L["runSemanticFilterTest"]
L --> J
L -->|uses| M["NotificationManager\n(global mock from setupTests.ts)"]
N["TeamSSOSettings.test.tsx\n(cleanup)"] -.->|removed redundant mock| O["@tremor/react\n(now uses global mock)"]
Last reviewed commit: 715e134
| render(<MCPSemanticFilterTestPanel {...buildProps({ setTestQuery: mockSetTestQuery })} />); | ||
|
|
||
| const textarea = screen.getByPlaceholderText(/enter a test query to see which tools/i); | ||
| fireEvent.change(textarea, { target: { value: "find relevant tools" } }); |
There was a problem hiding this comment.
fireEvent not wrapped in act()
Per the project's AGENTS.md testing guidelines: "Always wrap fireEvent calls with act() to ensure React state updates are properly handled." This fireEvent.change call should be wrapped in act() for consistency with the codebase convention, even though in this specific case the component is controlled and only a mock callback fires.
| fireEvent.change(textarea, { target: { value: "find relevant tools" } }); | |
| await act(() => { fireEvent.change(textarea, { target: { value: "find relevant tools" } }); }); |
Context Used: Context from dashboard - AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| @@ -0,0 +1,141 @@ | |||
| import React from "react"; | |||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | |||
| import { render, screen, fireEvent } from "@testing-library/react"; | |||
There was a problem hiding this comment.
Unused fireEvent import can use act
fireEvent is imported but could be replaced with userEvent (which is already imported and used elsewhere in this file) for consistency. If keeping fireEvent, also import act since it's needed for wrapping per project conventions.
| import { render, screen, fireEvent } from "@testing-library/react"; | |
| import { render, screen, fireEvent, act } from "@testing-library/react"; |
Context Used: Context from dashboard - AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…rage_00 [Infra] UI - Unit Testing Coverage: MCP Semantic Filter
Relevant issues
Summary
Adds Vitest unit test coverage for the
MCPSemanticFilterSettingsUI feature.Changes
Three new test files covering:
semanticFilterTestUtils.test.ts— pure utility functions (getCurlCommand,runSemanticFilterTest): input validation, API call lifecycle, result parsing, warning/error notification paths.MCPSemanticFilterTestPanel.test.tsx— presentational test panel component: rendering, controlled input callbacks, button disabled states, filter-disabled warning, test results display, curl command in API Usage tab.MCPSemanticFilterSettings.test.tsx— main settings component: no-access-token guard, loading/error states, form field rendering, Save button dirty-state gating, mutation error display.Also removes a redundant local
@tremor/reactmock fromTeamSSOSettings.test.tsxthat duplicated the global mock already defined intests/setupTests.ts.Testing
npx vitest run src/components/Settings/AdminSettings/MCPSemanticFilterSettings/— 33 tests, all passing.Type
✅ Test