Skip to content

Commit 3053678

Browse files
slytechnicalslytechnical
committed
Implement tests for LiteLLM component and enhance API configuration handling
- Added comprehensive tests for the LiteLLM component to validate API key and base URL handling during model refresh. - Updated the LiteLLMProps type to be exported for better accessibility. - Enhanced the SettingsView tests to ensure correct rendering and default values for LiteLLM configuration. - Mocked ApiOptions in tests to inspect props and validate API configuration behavior.
1 parent f92e4b9 commit 3053678

File tree

4 files changed

+266
-4
lines changed

4 files changed

+266
-4
lines changed

‎webview-ui/src/components/settings/__tests__/ApiOptions.test.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ jest.mock("vscrui", () => ({
4141

4242
// Mock @shadcn/ui components
4343
jest.mock("@/components/ui", () => ({
44-
Select: ({ children, value, onValueChange }: any) => (
45-
<div className="select-mock">
44+
Select: ({ children, value, onValueChange, ...rest }: any) => (
45+
<div className="select-mock" data-testid={rest["data-testid"]}>
4646
<select value={value} onChange={(e) => onValueChange && onValueChange(e.target.value)}>
4747
{children}
4848
</select>
@@ -152,11 +152,13 @@ jest.mock("@src/components/ui/hooks/useSelectedModel", () => ({
152152
if (apiConfiguration.apiModelId?.includes("thinking")) {
153153
return {
154154
provider: apiConfiguration.apiProvider,
155+
id: apiConfiguration.apiModelId,
155156
info: { thinking: true, contextWindow: 4000, maxTokens: 128000 },
156157
}
157158
} else {
158159
return {
159160
provider: apiConfiguration.apiProvider,
161+
id: apiConfiguration.apiModelId,
160162
info: { contextWindow: 4000 },
161163
}
162164
}

‎webview-ui/src/components/settings/__tests__/SettingsView.test.tsx‎

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,14 @@ jest.mock("@/components/ui", () => ({
134134
),
135135
}))
136136

137+
// Mock ApiOptions to inspect its props
138+
jest.mock("../ApiOptions", () => ({
139+
__esModule: true,
140+
default: jest.fn((props) => (
141+
<div data-testid="api-options-mock" data-apiconfiguration={JSON.stringify(props.apiConfiguration)} />
142+
)),
143+
}))
144+
137145
// Mock window.postMessage to trigger state hydration
138146
const mockPostMessage = (state: any) => {
139147
window.postMessage(
@@ -369,13 +377,87 @@ describe("SettingsView - Sound Settings", () => {
369377
describe("SettingsView - API Configuration", () => {
370378
beforeEach(() => {
371379
jest.clearAllMocks()
380+
// Reset ApiOptions mock calls before each test if needed
381+
require("../ApiOptions").default.mockClear()
372382
})
373383

374-
it("renders ApiConfigManagement with correct props", () => {
384+
it("renders ApiConfigManager with correct props", () => {
375385
renderSettingsView()
376386

377387
expect(screen.getByTestId("api-config-management")).toBeInTheDocument()
378388
})
389+
390+
it("defaults LiteLLM fields in apiConfiguration if provider is litellm and fields are missing", async () => {
391+
const initialExtensionState = {
392+
apiConfiguration: {
393+
apiProvider: "litellm",
394+
// litellmBaseUrl, litellmApiKey, litellmModelId are missing
395+
},
396+
// Add other necessary state fields for useExtensionState mock
397+
currentApiConfigName: "default",
398+
listApiConfigMeta: [],
399+
uriScheme: "vscode",
400+
version: "1.0.0",
401+
settingsImportedAt: null,
402+
}
403+
404+
// Mock useExtensionState to return our initial state
405+
const mockUseExtensionState = jest.spyOn(require("@/context/ExtensionStateContext"), "useExtensionState")
406+
mockUseExtensionState.mockReturnValue(initialExtensionState)
407+
408+
const { activateTab } = renderSettingsView() // onDone is part of the return, but we don't need it here
409+
410+
// Ensure providers tab is active (it should be by default, but explicit doesn't hurt)
411+
activateTab("providers")
412+
413+
// Wait for effects to run. Finding the mocked ApiOptions is a good way to ensure it has rendered with updated props.
414+
const apiOptionsMock = await screen.findByTestId("api-options-mock")
415+
const passedApiConfigString = apiOptionsMock.getAttribute("data-apiconfiguration")
416+
const passedApiConfig = JSON.parse(passedApiConfigString!)
417+
418+
expect(passedApiConfig.apiProvider).toBe("litellm")
419+
expect(passedApiConfig.litellmBaseUrl).toBe("http://localhost:4000")
420+
expect(passedApiConfig.litellmApiKey).toBe("sk-1234")
421+
expect(passedApiConfig.litellmModelId).toBeDefined() // Check it's defined (actual value is litellmDefaultModelId)
422+
423+
mockUseExtensionState.mockRestore()
424+
})
425+
426+
it("preserves existing LiteLLM fields in apiConfiguration if provider is litellm", async () => {
427+
const myCustomKey = "my-custom-key"
428+
const myCustomUrl = "http://my-custom-url.com"
429+
const myCustomModel = "custom-model/my-model"
430+
const initialExtensionState = {
431+
apiConfiguration: {
432+
apiProvider: "litellm",
433+
litellmBaseUrl: myCustomUrl,
434+
litellmApiKey: myCustomKey,
435+
litellmModelId: myCustomModel,
436+
},
437+
currentApiConfigName: "default",
438+
listApiConfigMeta: [],
439+
uriScheme: "vscode",
440+
version: "1.0.0",
441+
settingsImportedAt: null,
442+
}
443+
444+
const mockUseExtensionState = jest.spyOn(require("@/context/ExtensionStateContext"), "useExtensionState")
445+
mockUseExtensionState.mockReturnValue(initialExtensionState)
446+
447+
const { activateTab } = renderSettingsView()
448+
activateTab("providers")
449+
450+
const apiOptionsMock = await screen.findByTestId("api-options-mock")
451+
const passedApiConfigString = apiOptionsMock.getAttribute("data-apiconfiguration")
452+
const passedApiConfig = JSON.parse(passedApiConfigString!)
453+
454+
expect(passedApiConfig.apiProvider).toBe("litellm")
455+
expect(passedApiConfig.litellmBaseUrl).toBe(myCustomUrl)
456+
expect(passedApiConfig.litellmApiKey).toBe(myCustomKey)
457+
expect(passedApiConfig.litellmModelId).toBe(myCustomModel)
458+
459+
mockUseExtensionState.mockRestore()
460+
})
379461
})
380462

381463
describe("SettingsView - Allowed Commands", () => {

‎webview-ui/src/components/settings/providers/LiteLLM.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { ModelPicker } from "../ModelPicker"
1212
import { WebviewMessage } from "@roo/shared/WebviewMessage"
1313
import { ExtensionMessage } from "@roo/shared/ExtensionMessage"
1414

15-
type LiteLLMProps = {
15+
export type LiteLLMProps = {
1616
apiConfiguration: ProviderSettings
1717
setApiConfigurationField: (field: keyof ProviderSettings, value: ProviderSettings[keyof ProviderSettings]) => void
1818
// routerModels prop might need to be updated by parent if we want to show new models immediately.

‎webview-ui/src/components/settings/providers/__tests__/LiteLLM.test.tsx‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
import React from "react"
2+
import { render, screen } from "@testing-library/react"
3+
import { I18nextProvider } from "react-i18next"
4+
import i18next from "i18next"
5+
6+
import { LiteLLM, LiteLLMProps } from "../LiteLLM"
7+
import { vscode } from "@/utils/vscode"
8+
9+
// Minimal i18n instance for testing
10+
const testI18n = i18next.createInstance()
11+
testI18n.init({
12+
fallbackLng: "en",
13+
debug: false,
14+
resources: {
15+
en: {
16+
translation: {
17+
"settings:providers.refreshModels.label": "Refresh Models",
18+
"settings:providers.refreshModels.missingConfig": "API key or base URL missing.",
19+
},
20+
},
21+
},
22+
interpolation: {
23+
escapeValue: false, // Not needed for React
24+
},
25+
})
26+
27+
// Mock vscode API
28+
jest.mock("@/utils/vscode", () => ({
29+
vscode: {
30+
postMessage: jest.fn(),
31+
},
32+
}))
33+
34+
// Mock VSCodeTextField
35+
jest.mock("@vscode/webview-ui-toolkit/react", () => ({
36+
VSCodeTextField: ({ children, value, onInput, type }: any) => (
37+
<div>
38+
{children}
39+
<input
40+
type={type || "text"}
41+
value={value}
42+
onChange={(e) => onInput && onInput({ target: { value: e.target.value } })}
43+
/>
44+
</div>
45+
),
46+
}))
47+
48+
// Mock ModelPicker
49+
jest.mock("../../ModelPicker", () => ({
50+
ModelPicker: () => <div data-testid="model-picker-mock">ModelPicker</div>,
51+
}))
52+
53+
const mockT = jest.fn((key) => key) // Simple t mock
54+
55+
jest.mock("@/i18n/TranslationContext", () => ({
56+
useAppTranslation: () => ({
57+
t: mockT,
58+
}),
59+
}))
60+
61+
const defaultProps: LiteLLMProps = {
62+
apiConfiguration: { litellmApiKey: "", litellmBaseUrl: "" },
63+
setApiConfigurationField: jest.fn(),
64+
routerModels: {
65+
litellm: {},
66+
glama: {},
67+
openrouter: {},
68+
unbound: {},
69+
requesty: {},
70+
},
71+
}
72+
73+
const renderLiteLLM = (props?: Partial<LiteLLMProps>) => {
74+
return render(
75+
<I18nextProvider i18n={testI18n}>
76+
<LiteLLM {...defaultProps} {...props} />
77+
</I18nextProvider>,
78+
)
79+
}
80+
81+
describe("LiteLLM Component", () => {
82+
beforeEach(() => {
83+
jest.clearAllMocks()
84+
// Reset the ref module-level if needed, but usually refs are instance-based.
85+
// For this test, we rely on fresh mounts giving fresh refs.
86+
})
87+
88+
it("does not attempt initial model refresh if API key is missing", () => {
89+
renderLiteLLM({
90+
apiConfiguration: {
91+
...defaultProps.apiConfiguration,
92+
litellmBaseUrl: "http://localhost:4000",
93+
litellmApiKey: "",
94+
},
95+
})
96+
expect(vscode.postMessage).not.toHaveBeenCalledWith(expect.objectContaining({ type: "requestProviderModels" }))
97+
})
98+
99+
it("does not attempt initial model refresh if base URL is missing", () => {
100+
renderLiteLLM({
101+
apiConfiguration: { ...defaultProps.apiConfiguration, litellmApiKey: "test-key", litellmBaseUrl: "" },
102+
})
103+
expect(vscode.postMessage).not.toHaveBeenCalledWith(expect.objectContaining({ type: "requestProviderModels" }))
104+
})
105+
106+
it("attempts initial model refresh once if API key and base URL are present on mount", () => {
107+
renderLiteLLM({
108+
apiConfiguration: {
109+
...defaultProps.apiConfiguration,
110+
litellmApiKey: "test-key",
111+
litellmBaseUrl: "http://localhost:4000",
112+
},
113+
})
114+
expect(vscode.postMessage).toHaveBeenCalledTimes(1)
115+
expect(vscode.postMessage).toHaveBeenCalledWith({
116+
type: "requestProviderModels",
117+
payload: {
118+
provider: "litellm",
119+
apiKey: "test-key",
120+
baseUrl: "http://localhost:4000",
121+
},
122+
})
123+
})
124+
125+
it("does not re-attempt initial refresh if props change but refresh was already done", () => {
126+
const { rerender } = renderLiteLLM({
127+
apiConfiguration: {
128+
...defaultProps.apiConfiguration,
129+
litellmApiKey: "test-key",
130+
litellmBaseUrl: "http://localhost:4000",
131+
},
132+
})
133+
expect(vscode.postMessage).toHaveBeenCalledTimes(1) // Initial call
134+
135+
// Re-render with different routerModels (a prop that might change)
136+
rerender(
137+
<I18nextProvider i18n={testI18n}>
138+
<LiteLLM
139+
{...defaultProps}
140+
apiConfiguration={{
141+
...defaultProps.apiConfiguration,
142+
litellmApiKey: "test-key", // Same key
143+
litellmBaseUrl: "http://localhost:4000", // Same URL
144+
}}
145+
routerModels={{
146+
litellm: { "new-model": { contextWindow: 4096, supportsPromptCache: false } },
147+
glama: {},
148+
openrouter: {},
149+
unbound: {},
150+
requesty: {},
151+
}}
152+
/>
153+
</I18nextProvider>,
154+
)
155+
// Should still only be 1 call from the initial refresh
156+
expect(vscode.postMessage).toHaveBeenCalledTimes(1)
157+
})
158+
159+
it("manual refresh button is disabled if API key is missing", () => {
160+
renderLiteLLM({
161+
apiConfiguration: {
162+
...defaultProps.apiConfiguration,
163+
litellmBaseUrl: "http://localhost:4000",
164+
litellmApiKey: "",
165+
},
166+
})
167+
const refreshButton = screen.getByText("settings:providers.refreshModels.label").closest("button")
168+
expect(refreshButton).toBeDisabled()
169+
})
170+
171+
it("manual refresh button is disabled if base URL is missing", () => {
172+
renderLiteLLM({
173+
apiConfiguration: { ...defaultProps.apiConfiguration, litellmApiKey: "test-key", litellmBaseUrl: "" },
174+
})
175+
const refreshButton = screen.getByText("settings:providers.refreshModels.label").closest("button")
176+
expect(refreshButton).toBeDisabled()
177+
})
178+
})

0 commit comments

Comments
 (0)