diff --git a/webview-ui/src/components/settings/__tests__/ApiOptions.test.tsx b/webview-ui/src/components/settings/__tests__/ApiOptions.test.tsx index b9b1d1d21e..9c7507cb27 100644 --- a/webview-ui/src/components/settings/__tests__/ApiOptions.test.tsx +++ b/webview-ui/src/components/settings/__tests__/ApiOptions.test.tsx @@ -41,8 +41,8 @@ jest.mock("vscrui", () => ({ // Mock @shadcn/ui components jest.mock("@/components/ui", () => ({ - Select: ({ children, value, onValueChange }: any) => ( -
+ Select: ({ children, value, onValueChange, ...rest }: any) => ( +
@@ -152,11 +152,13 @@ jest.mock("@src/components/ui/hooks/useSelectedModel", () => ({ if (apiConfiguration.apiModelId?.includes("thinking")) { return { provider: apiConfiguration.apiProvider, + id: apiConfiguration.apiModelId, info: { thinking: true, contextWindow: 4000, maxTokens: 128000 }, } } else { return { provider: apiConfiguration.apiProvider, + id: apiConfiguration.apiModelId, info: { contextWindow: 4000 }, } } diff --git a/webview-ui/src/components/settings/__tests__/SettingsView.test.tsx b/webview-ui/src/components/settings/__tests__/SettingsView.test.tsx index 072296a754..46acab8b92 100644 --- a/webview-ui/src/components/settings/__tests__/SettingsView.test.tsx +++ b/webview-ui/src/components/settings/__tests__/SettingsView.test.tsx @@ -134,6 +134,14 @@ jest.mock("@/components/ui", () => ({ ), })) +// Mock ApiOptions to inspect its props +jest.mock("../ApiOptions", () => ({ + __esModule: true, + default: jest.fn((props) => ( +
+ )), +})) + // Mock window.postMessage to trigger state hydration const mockPostMessage = (state: any) => { window.postMessage( @@ -369,13 +377,87 @@ describe("SettingsView - Sound Settings", () => { describe("SettingsView - API Configuration", () => { beforeEach(() => { jest.clearAllMocks() + // Reset ApiOptions mock calls before each test if needed + require("../ApiOptions").default.mockClear() }) - it("renders ApiConfigManagement with correct props", () => { + it("renders ApiConfigManager with correct props", () => { renderSettingsView() expect(screen.getByTestId("api-config-management")).toBeInTheDocument() }) + + it("defaults LiteLLM fields in apiConfiguration if provider is litellm and fields are missing", async () => { + const initialExtensionState = { + apiConfiguration: { + apiProvider: "litellm", + // litellmBaseUrl, litellmApiKey, litellmModelId are missing + }, + // Add other necessary state fields for useExtensionState mock + currentApiConfigName: "default", + listApiConfigMeta: [], + uriScheme: "vscode", + version: "1.0.0", + settingsImportedAt: null, + } + + // Mock useExtensionState to return our initial state + const mockUseExtensionState = jest.spyOn(require("@/context/ExtensionStateContext"), "useExtensionState") + mockUseExtensionState.mockReturnValue(initialExtensionState) + + const { activateTab } = renderSettingsView() // onDone is part of the return, but we don't need it here + + // Ensure providers tab is active (it should be by default, but explicit doesn't hurt) + activateTab("providers") + + // Wait for effects to run. Finding the mocked ApiOptions is a good way to ensure it has rendered with updated props. + const apiOptionsMock = await screen.findByTestId("api-options-mock") + const passedApiConfigString = apiOptionsMock.getAttribute("data-apiconfiguration") + const passedApiConfig = JSON.parse(passedApiConfigString!) + + expect(passedApiConfig.apiProvider).toBe("litellm") + expect(passedApiConfig.litellmBaseUrl).toBe("http://localhost:4000") + expect(passedApiConfig.litellmApiKey).toBe("sk-1234") + expect(passedApiConfig.litellmModelId).toBeDefined() // Check it's defined (actual value is litellmDefaultModelId) + + mockUseExtensionState.mockRestore() + }) + + it("preserves existing LiteLLM fields in apiConfiguration if provider is litellm", async () => { + const myCustomKey = "my-custom-key" + const myCustomUrl = "http://my-custom-url.com" + const myCustomModel = "custom-model/my-model" + const initialExtensionState = { + apiConfiguration: { + apiProvider: "litellm", + litellmBaseUrl: myCustomUrl, + litellmApiKey: myCustomKey, + litellmModelId: myCustomModel, + }, + currentApiConfigName: "default", + listApiConfigMeta: [], + uriScheme: "vscode", + version: "1.0.0", + settingsImportedAt: null, + } + + const mockUseExtensionState = jest.spyOn(require("@/context/ExtensionStateContext"), "useExtensionState") + mockUseExtensionState.mockReturnValue(initialExtensionState) + + const { activateTab } = renderSettingsView() + activateTab("providers") + + const apiOptionsMock = await screen.findByTestId("api-options-mock") + const passedApiConfigString = apiOptionsMock.getAttribute("data-apiconfiguration") + const passedApiConfig = JSON.parse(passedApiConfigString!) + + expect(passedApiConfig.apiProvider).toBe("litellm") + expect(passedApiConfig.litellmBaseUrl).toBe(myCustomUrl) + expect(passedApiConfig.litellmApiKey).toBe(myCustomKey) + expect(passedApiConfig.litellmModelId).toBe(myCustomModel) + + mockUseExtensionState.mockRestore() + }) }) describe("SettingsView - Allowed Commands", () => { diff --git a/webview-ui/src/components/settings/providers/LiteLLM.tsx b/webview-ui/src/components/settings/providers/LiteLLM.tsx index fd2071383f..0dbcbc1385 100644 --- a/webview-ui/src/components/settings/providers/LiteLLM.tsx +++ b/webview-ui/src/components/settings/providers/LiteLLM.tsx @@ -12,7 +12,7 @@ import { ModelPicker } from "../ModelPicker" import { WebviewMessage } from "@roo/shared/WebviewMessage" import { ExtensionMessage } from "@roo/shared/ExtensionMessage" -type LiteLLMProps = { +export type LiteLLMProps = { apiConfiguration: ProviderSettings setApiConfigurationField: (field: keyof ProviderSettings, value: ProviderSettings[keyof ProviderSettings]) => void // routerModels prop might need to be updated by parent if we want to show new models immediately. diff --git a/webview-ui/src/components/settings/providers/__tests__/LiteLLM.test.tsx b/webview-ui/src/components/settings/providers/__tests__/LiteLLM.test.tsx new file mode 100644 index 0000000000..6a8ccc4a82 --- /dev/null +++ b/webview-ui/src/components/settings/providers/__tests__/LiteLLM.test.tsx @@ -0,0 +1,178 @@ +import React from "react" +import { render, screen } from "@testing-library/react" +import { I18nextProvider } from "react-i18next" +import i18next from "i18next" + +import { LiteLLM, LiteLLMProps } from "../LiteLLM" +import { vscode } from "@/utils/vscode" + +// Minimal i18n instance for testing +const testI18n = i18next.createInstance() +testI18n.init({ + fallbackLng: "en", + debug: false, + resources: { + en: { + translation: { + "settings:providers.refreshModels.label": "Refresh Models", + "settings:providers.refreshModels.missingConfig": "API key or base URL missing.", + }, + }, + }, + interpolation: { + escapeValue: false, // Not needed for React + }, +}) + +// Mock vscode API +jest.mock("@/utils/vscode", () => ({ + vscode: { + postMessage: jest.fn(), + }, +})) + +// Mock VSCodeTextField +jest.mock("@vscode/webview-ui-toolkit/react", () => ({ + VSCodeTextField: ({ children, value, onInput, type }: any) => ( +
+ {children} + onInput && onInput({ target: { value: e.target.value } })} + /> +
+ ), +})) + +// Mock ModelPicker +jest.mock("../../ModelPicker", () => ({ + ModelPicker: () =>
ModelPicker
, +})) + +const mockT = jest.fn((key) => key) // Simple t mock + +jest.mock("@/i18n/TranslationContext", () => ({ + useAppTranslation: () => ({ + t: mockT, + }), +})) + +const defaultProps: LiteLLMProps = { + apiConfiguration: { litellmApiKey: "", litellmBaseUrl: "" }, + setApiConfigurationField: jest.fn(), + routerModels: { + litellm: {}, + glama: {}, + openrouter: {}, + unbound: {}, + requesty: {}, + }, +} + +const renderLiteLLM = (props?: Partial) => { + return render( + + + , + ) +} + +describe("LiteLLM Component", () => { + beforeEach(() => { + jest.clearAllMocks() + // Reset the ref module-level if needed, but usually refs are instance-based. + // For this test, we rely on fresh mounts giving fresh refs. + }) + + it("does not attempt initial model refresh if API key is missing", () => { + renderLiteLLM({ + apiConfiguration: { + ...defaultProps.apiConfiguration, + litellmBaseUrl: "http://localhost:4000", + litellmApiKey: "", + }, + }) + expect(vscode.postMessage).not.toHaveBeenCalledWith(expect.objectContaining({ type: "requestProviderModels" })) + }) + + it("does not attempt initial model refresh if base URL is missing", () => { + renderLiteLLM({ + apiConfiguration: { ...defaultProps.apiConfiguration, litellmApiKey: "test-key", litellmBaseUrl: "" }, + }) + expect(vscode.postMessage).not.toHaveBeenCalledWith(expect.objectContaining({ type: "requestProviderModels" })) + }) + + it("attempts initial model refresh once if API key and base URL are present on mount", () => { + renderLiteLLM({ + apiConfiguration: { + ...defaultProps.apiConfiguration, + litellmApiKey: "test-key", + litellmBaseUrl: "http://localhost:4000", + }, + }) + expect(vscode.postMessage).toHaveBeenCalledTimes(1) + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: "requestProviderModels", + payload: { + provider: "litellm", + apiKey: "test-key", + baseUrl: "http://localhost:4000", + }, + }) + }) + + it("does not re-attempt initial refresh if props change but refresh was already done", () => { + const { rerender } = renderLiteLLM({ + apiConfiguration: { + ...defaultProps.apiConfiguration, + litellmApiKey: "test-key", + litellmBaseUrl: "http://localhost:4000", + }, + }) + expect(vscode.postMessage).toHaveBeenCalledTimes(1) // Initial call + + // Re-render with different routerModels (a prop that might change) + rerender( + + + , + ) + // Should still only be 1 call from the initial refresh + expect(vscode.postMessage).toHaveBeenCalledTimes(1) + }) + + it("manual refresh button is disabled if API key is missing", () => { + renderLiteLLM({ + apiConfiguration: { + ...defaultProps.apiConfiguration, + litellmBaseUrl: "http://localhost:4000", + litellmApiKey: "", + }, + }) + const refreshButton = screen.getByText("settings:providers.refreshModels.label").closest("button") + expect(refreshButton).toBeDisabled() + }) + + it("manual refresh button is disabled if base URL is missing", () => { + renderLiteLLM({ + apiConfiguration: { ...defaultProps.apiConfiguration, litellmApiKey: "test-key", litellmBaseUrl: "" }, + }) + const refreshButton = screen.getByText("settings:providers.refreshModels.label").closest("button") + expect(refreshButton).toBeDisabled() + }) +})