-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
[Infra] UI - Unit Testing Coverage: MCP Semantic Filter #21968
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| import React from "react"; | ||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||
| import { render, screen, act } from "@testing-library/react"; | ||
| import userEvent from "@testing-library/user-event"; | ||
| import MCPSemanticFilterSettings from "./MCPSemanticFilterSettings"; | ||
| import { useMCPSemanticFilterSettings } from "@/app/(dashboard)/hooks/mcpSemanticFilterSettings/useMCPSemanticFilterSettings"; | ||
| import { useUpdateMCPSemanticFilterSettings } from "@/app/(dashboard)/hooks/mcpSemanticFilterSettings/useUpdateMCPSemanticFilterSettings"; | ||
|
|
||
| vi.mock( | ||
| "@/app/(dashboard)/hooks/mcpSemanticFilterSettings/useMCPSemanticFilterSettings", | ||
| () => ({ useMCPSemanticFilterSettings: vi.fn() }) | ||
| ); | ||
|
|
||
| vi.mock( | ||
| "@/app/(dashboard)/hooks/mcpSemanticFilterSettings/useUpdateMCPSemanticFilterSettings", | ||
| () => ({ useUpdateMCPSemanticFilterSettings: vi.fn() }) | ||
| ); | ||
|
|
||
| vi.mock("@/components/playground/llm_calls/fetch_models", () => ({ | ||
| fetchAvailableModels: vi.fn().mockResolvedValue([]), | ||
| })); | ||
|
|
||
| vi.mock("./MCPSemanticFilterTestPanel", () => ({ | ||
| default: () => <div data-testid="mcp-test-panel" />, | ||
| })); | ||
|
|
||
| vi.mock("./semanticFilterTestUtils", () => ({ | ||
| getCurlCommand: vi.fn().mockReturnValue("curl ..."), | ||
| runSemanticFilterTest: vi.fn(), | ||
| })); | ||
|
|
||
| const mockMutate = vi.fn(); | ||
|
|
||
| const defaultSettingsData = { | ||
| field_schema: { | ||
| properties: { | ||
| enabled: { description: "Enable semantic filtering for MCP tools" }, | ||
| }, | ||
| }, | ||
| values: { | ||
| enabled: false, | ||
| embedding_model: "text-embedding-3-small", | ||
| top_k: 10, | ||
| similarity_threshold: 0.3, | ||
| }, | ||
| }; | ||
|
|
||
| // Helper that renders the component and flushes the fetchAvailableModels effect | ||
| async function renderSettings(props: React.ComponentProps<typeof MCPSemanticFilterSettings>) { | ||
| render(<MCPSemanticFilterSettings {...props} />); | ||
| if (props.accessToken) { | ||
| // Let the async fetchAvailableModels effect settle to avoid act() warnings | ||
| await act(async () => {}); | ||
| } | ||
| } | ||
|
|
||
| describe("MCPSemanticFilterSettings", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| vi.mocked(useMCPSemanticFilterSettings).mockReturnValue({ | ||
| data: defaultSettingsData, | ||
| isLoading: false, | ||
| isError: false, | ||
| error: null, | ||
| } as any); | ||
| vi.mocked(useUpdateMCPSemanticFilterSettings).mockReturnValue({ | ||
| mutate: mockMutate, | ||
| isPending: false, | ||
| error: null, | ||
| } as any); | ||
| }); | ||
|
|
||
| it("should render", async () => { | ||
| await renderSettings({ accessToken: "test-token" }); | ||
| expect(screen.getByText("Semantic Tool Filtering")).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("should show a login prompt when accessToken is null", () => { | ||
| render(<MCPSemanticFilterSettings accessToken={null} />); | ||
| expect(screen.getByText(/please log in/i)).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("should not render the form when accessToken is null", () => { | ||
| render(<MCPSemanticFilterSettings accessToken={null} />); | ||
| expect(screen.queryByText("Enable Semantic Filtering")).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("should not show the settings content while loading", async () => { | ||
| vi.mocked(useMCPSemanticFilterSettings).mockReturnValue({ | ||
| data: undefined, | ||
| isLoading: true, | ||
| isError: false, | ||
| error: null, | ||
| } as any); | ||
| await renderSettings({ accessToken: "test-token" }); | ||
| expect(screen.queryByText("Semantic Tool Filtering")).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("should show an error alert when data fails to load", async () => { | ||
| vi.mocked(useMCPSemanticFilterSettings).mockReturnValue({ | ||
| data: undefined, | ||
| isLoading: false, | ||
| isError: true, | ||
| error: new Error("Network error"), | ||
| } as any); | ||
| await renderSettings({ accessToken: "test-token" }); | ||
| expect( | ||
| screen.getByText("Could not load MCP Semantic Filter settings") | ||
| ).toBeInTheDocument(); | ||
| expect(screen.getByText("Network error")).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("should show the error message from the error object when loading fails", async () => { | ||
| vi.mocked(useMCPSemanticFilterSettings).mockReturnValue({ | ||
| data: undefined, | ||
| isLoading: false, | ||
| isError: true, | ||
| error: new Error("Connection refused"), | ||
| } as any); | ||
| await renderSettings({ accessToken: "test-token" }); | ||
| expect(screen.getByText("Connection refused")).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("should render the info alert and form fields when data is loaded", async () => { | ||
| await renderSettings({ accessToken: "test-token" }); | ||
| expect(screen.getByText("Semantic Tool Filtering")).toBeInTheDocument(); | ||
| expect(screen.getByText("Enable Semantic Filtering")).toBeInTheDocument(); | ||
| expect(screen.getByText("Top K Results")).toBeInTheDocument(); | ||
| expect(screen.getByText("Similarity Threshold")).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("should render the test panel", async () => { | ||
| await renderSettings({ accessToken: "test-token" }); | ||
| expect(screen.getByTestId("mcp-test-panel")).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("should have Save Settings button disabled initially", async () => { | ||
| await renderSettings({ accessToken: "test-token" }); | ||
| expect( | ||
| screen.getByRole("button", { name: /save settings/i }) | ||
| ).toBeDisabled(); | ||
| }); | ||
|
|
||
| it("should enable Save Settings button after a form field is changed", async () => { | ||
| const user = userEvent.setup(); | ||
| await renderSettings({ accessToken: "test-token" }); | ||
|
|
||
| expect(screen.getByRole("button", { name: /save settings/i })).toBeDisabled(); | ||
|
|
||
| await user.click(screen.getByRole("switch")); | ||
|
|
||
| expect(screen.getByRole("button", { name: /save settings/i })).not.toBeDisabled(); | ||
| }); | ||
|
|
||
| it("should show an error alert when the mutation fails", async () => { | ||
| vi.mocked(useUpdateMCPSemanticFilterSettings).mockReturnValue({ | ||
| mutate: mockMutate, | ||
| isPending: false, | ||
| error: new Error("Failed to update settings"), | ||
| } as any); | ||
| await renderSettings({ accessToken: "test-token" }); | ||
| expect(screen.getByText("Could not update settings")).toBeInTheDocument(); | ||
| expect(screen.getByText("Failed to update settings")).toBeInTheDocument(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,141 @@ | ||||||
| import React from "react"; | ||||||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||||||
| import { render, screen, fireEvent } from "@testing-library/react"; | ||||||
| import userEvent from "@testing-library/user-event"; | ||||||
| import MCPSemanticFilterTestPanel from "./MCPSemanticFilterTestPanel"; | ||||||
| import { TestResult } from "./semanticFilterTestUtils"; | ||||||
|
|
||||||
| vi.mock("@/components/common_components/ModelSelector", () => ({ | ||||||
| default: ({ onChange, value, labelText, disabled }: any) => ( | ||||||
| <div> | ||||||
| <label htmlFor="model-selector">{labelText ?? "Select Model"}</label> | ||||||
| <select | ||||||
| id="model-selector" | ||||||
| value={value} | ||||||
| onChange={(e) => onChange(e.target.value)} | ||||||
| disabled={disabled} | ||||||
| > | ||||||
| <option value="gpt-4o">gpt-4o</option> | ||||||
| <option value="gpt-3.5-turbo">gpt-3.5-turbo</option> | ||||||
| </select> | ||||||
| </div> | ||||||
| ), | ||||||
| })); | ||||||
|
|
||||||
| const buildProps = ( | ||||||
| overrides: Partial<React.ComponentProps<typeof MCPSemanticFilterTestPanel>> = {} | ||||||
| ) => ({ | ||||||
| accessToken: "test-token", | ||||||
| testQuery: "", | ||||||
| setTestQuery: vi.fn(), | ||||||
| testModel: "gpt-4o", | ||||||
| setTestModel: vi.fn(), | ||||||
| isTesting: false, | ||||||
| onTest: vi.fn(), | ||||||
| filterEnabled: true, | ||||||
| testResult: null as TestResult | null, | ||||||
| curlCommand: "curl --location 'http://localhost:4000/v1/responses'", | ||||||
| ...overrides, | ||||||
| }); | ||||||
|
|
||||||
| describe("MCPSemanticFilterTestPanel", () => { | ||||||
| beforeEach(() => { | ||||||
| vi.clearAllMocks(); | ||||||
| }); | ||||||
|
|
||||||
| it("should render the Test Configuration card", () => { | ||||||
| render(<MCPSemanticFilterTestPanel {...buildProps()} />); | ||||||
| expect(screen.getByText("Test Configuration")).toBeInTheDocument(); | ||||||
| }); | ||||||
|
|
||||||
| it("should show the test query textarea", () => { | ||||||
| render(<MCPSemanticFilterTestPanel {...buildProps()} />); | ||||||
| expect( | ||||||
| screen.getByPlaceholderText(/enter a test query to see which tools/i) | ||||||
| ).toBeInTheDocument(); | ||||||
| }); | ||||||
|
|
||||||
| it("should call setTestQuery when user types in the query field", () => { | ||||||
| const mockSetTestQuery = vi.fn(); | ||||||
| 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" } }); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Context Used: Context from 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! |
||||||
|
|
||||||
| expect(mockSetTestQuery).toHaveBeenCalledWith("find relevant tools"); | ||||||
| }); | ||||||
|
|
||||||
| it("should disable the Test Filter button when testQuery is empty", () => { | ||||||
| render(<MCPSemanticFilterTestPanel {...buildProps({ testQuery: "" })} />); | ||||||
| expect(screen.getByRole("button", { name: /test filter/i })).toBeDisabled(); | ||||||
| }); | ||||||
|
|
||||||
| it("should disable the Test Filter button when filterEnabled is false", () => { | ||||||
| render( | ||||||
| <MCPSemanticFilterTestPanel | ||||||
| {...buildProps({ testQuery: "search query", filterEnabled: false })} | ||||||
| /> | ||||||
| ); | ||||||
| expect(screen.getByRole("button", { name: /test filter/i })).toBeDisabled(); | ||||||
| }); | ||||||
|
|
||||||
| it("should enable the Test Filter button when testQuery is set and filter is enabled", () => { | ||||||
| render( | ||||||
| <MCPSemanticFilterTestPanel {...buildProps({ testQuery: "search query" })} /> | ||||||
| ); | ||||||
| expect(screen.getByRole("button", { name: /test filter/i })).not.toBeDisabled(); | ||||||
| }); | ||||||
|
|
||||||
| it("should call onTest when the Test Filter button is clicked", async () => { | ||||||
| const mockOnTest = vi.fn(); | ||||||
| const user = userEvent.setup(); | ||||||
| render( | ||||||
| <MCPSemanticFilterTestPanel | ||||||
| {...buildProps({ testQuery: "search query", onTest: mockOnTest })} | ||||||
| /> | ||||||
| ); | ||||||
|
|
||||||
| await user.click(screen.getByRole("button", { name: /test filter/i })); | ||||||
| expect(mockOnTest).toHaveBeenCalledOnce(); | ||||||
| }); | ||||||
|
|
||||||
| it("should show a warning when semantic filtering is disabled", () => { | ||||||
| render(<MCPSemanticFilterTestPanel {...buildProps({ filterEnabled: false })} />); | ||||||
| expect(screen.getByText("Semantic filtering is disabled")).toBeInTheDocument(); | ||||||
| }); | ||||||
|
|
||||||
| it("should not show the disabled warning when filterEnabled is true", () => { | ||||||
| render(<MCPSemanticFilterTestPanel {...buildProps({ filterEnabled: true })} />); | ||||||
| expect(screen.queryByText("Semantic filtering is disabled")).not.toBeInTheDocument(); | ||||||
| }); | ||||||
|
|
||||||
| it("should display test results when testResult is provided", () => { | ||||||
| const testResult: TestResult = { | ||||||
| totalTools: 10, | ||||||
| selectedTools: 3, | ||||||
| tools: ["wiki-fetch", "github-search", "slack-post"], | ||||||
| }; | ||||||
| render(<MCPSemanticFilterTestPanel {...buildProps({ testResult })} />); | ||||||
|
|
||||||
| expect(screen.getByText("3 tools selected")).toBeInTheDocument(); | ||||||
| expect(screen.getByText("Filtered from 10 available tools")).toBeInTheDocument(); | ||||||
| expect(screen.getByText("wiki-fetch")).toBeInTheDocument(); | ||||||
| expect(screen.getByText("github-search")).toBeInTheDocument(); | ||||||
| expect(screen.getByText("slack-post")).toBeInTheDocument(); | ||||||
| }); | ||||||
|
|
||||||
| it("should not render the results section when testResult is null", () => { | ||||||
| render(<MCPSemanticFilterTestPanel {...buildProps({ testResult: null })} />); | ||||||
| expect(screen.queryByText("Results")).not.toBeInTheDocument(); | ||||||
| }); | ||||||
|
|
||||||
| it("should show the curl command in the API Usage tab", async () => { | ||||||
| const user = userEvent.setup(); | ||||||
| const curlCommand = "curl --location 'http://localhost:4000/v1/responses' --header 'Authorization: Bearer sk-1234'"; | ||||||
| render(<MCPSemanticFilterTestPanel {...buildProps({ curlCommand })} />); | ||||||
|
|
||||||
| await user.click(screen.getByRole("tab", { name: "API Usage" })); | ||||||
|
|
||||||
| expect(screen.getByText(curlCommand)).toBeInTheDocument(); | ||||||
| }); | ||||||
| }); | ||||||
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.
Unused
fireEventimport can useactfireEventis imported but could be replaced withuserEvent(which is already imported and used elsewhere in this file) for consistency. If keepingfireEvent, also importactsince it's needed for wrapping per project conventions.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!