Skip to content

Commit 00ad425

Browse files
authored
Fix flakey ModelPicker test (#4972)
1 parent 86ce767 commit 00ad425

File tree

2 files changed

+47
-5
lines changed

2 files changed

+47
-5
lines changed

webview-ui/src/components/settings/ModelPicker.tsx

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ export const ModelPicker = ({
5959
const [isDescriptionExpanded, setIsDescriptionExpanded] = useState(false)
6060
const isInitialized = useRef(false)
6161
const searchInputRef = useRef<HTMLInputElement>(null)
62+
const selectTimeoutRef = useRef<NodeJS.Timeout | null>(null)
63+
const closeTimeoutRef = useRef<NodeJS.Timeout | null>(null)
6264

6365
const modelIds = useMemo(() => {
6466
const filteredModels = filterModels(models, apiConfiguration.apiProvider, organizationAllowList)
@@ -79,8 +81,13 @@ export const ModelPicker = ({
7981
setOpen(false)
8082
setApiConfigurationField(modelIdKey, modelId)
8183

84+
// Clear any existing timeout
85+
if (selectTimeoutRef.current) {
86+
clearTimeout(selectTimeoutRef.current)
87+
}
88+
8289
// Delay to ensure the popover is closed before setting the search value.
83-
setTimeout(() => setSearchValue(modelId), 100)
90+
selectTimeoutRef.current = setTimeout(() => setSearchValue(modelId), 100)
8491
},
8592
[modelIdKey, setApiConfigurationField],
8693
)
@@ -91,8 +98,13 @@ export const ModelPicker = ({
9198

9299
// Abandon the current search if the popover is closed.
93100
if (!open) {
101+
// Clear any existing timeout
102+
if (closeTimeoutRef.current) {
103+
clearTimeout(closeTimeoutRef.current)
104+
}
105+
94106
// Delay to ensure the popover is closed before setting the search value.
95-
setTimeout(() => setSearchValue(selectedModelId), 100)
107+
closeTimeoutRef.current = setTimeout(() => setSearchValue(selectedModelId), 100)
96108
}
97109
},
98110
[selectedModelId],
@@ -112,6 +124,18 @@ export const ModelPicker = ({
112124
isInitialized.current = true
113125
}, [modelIds, setApiConfigurationField, modelIdKey, selectedModelId, defaultModelId])
114126

127+
// Cleanup timeouts on unmount to prevent test flakiness
128+
useEffect(() => {
129+
return () => {
130+
if (selectTimeoutRef.current) {
131+
clearTimeout(selectTimeoutRef.current)
132+
}
133+
if (closeTimeoutRef.current) {
134+
clearTimeout(closeTimeoutRef.current)
135+
}
136+
}
137+
}, [])
138+
115139
return (
116140
<>
117141
<div>

webview-ui/src/components/settings/__tests__/ModelPicker.spec.tsx

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { screen, fireEvent, render } from "@testing-library/react"
44
import { act } from "react"
55
import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
6+
import { vi } from "vitest"
67

78
import { ModelInfo } from "@roo-code/types"
89

@@ -58,6 +59,13 @@ describe("ModelPicker", () => {
5859

5960
beforeEach(() => {
6061
vi.clearAllMocks()
62+
vi.useFakeTimers()
63+
})
64+
65+
afterEach(() => {
66+
// Clear any pending timers to prevent test flakiness
67+
vi.clearAllTimers()
68+
vi.useRealTimers()
6169
})
6270

6371
it("calls setApiConfigurationField when a model is selected", async () => {
@@ -71,7 +79,7 @@ describe("ModelPicker", () => {
7179

7280
// Wait for popover to open and animations to complete.
7381
await act(async () => {
74-
await new Promise((resolve) => setTimeout(resolve, 100))
82+
vi.advanceTimersByTime(100)
7583
})
7684

7785
await act(async () => {
@@ -87,6 +95,11 @@ describe("ModelPicker", () => {
8795
fireEvent.click(modelItem)
8896
})
8997

98+
// Advance timers to trigger the setTimeout in onSelect
99+
await act(async () => {
100+
vi.advanceTimersByTime(100)
101+
})
102+
90103
// Verify the API config was updated.
91104
expect(mockSetApiConfigurationField).toHaveBeenCalledWith(defaultProps.modelIdKey, "model2")
92105
})
@@ -102,7 +115,7 @@ describe("ModelPicker", () => {
102115

103116
// Wait for popover to open and animations to complete.
104117
await act(async () => {
105-
await new Promise((resolve) => setTimeout(resolve, 100))
118+
vi.advanceTimersByTime(100)
106119
})
107120

108121
const customModelId = "custom-model-id"
@@ -115,7 +128,7 @@ describe("ModelPicker", () => {
115128

116129
// Wait for the UI to update
117130
await act(async () => {
118-
await new Promise((resolve) => setTimeout(resolve, 100))
131+
vi.advanceTimersByTime(100)
119132
})
120133

121134
// Find and click the "Use custom" option
@@ -125,6 +138,11 @@ describe("ModelPicker", () => {
125138
fireEvent.click(customOption)
126139
})
127140

141+
// Advance timers to trigger the setTimeout in onSelect
142+
await act(async () => {
143+
vi.advanceTimersByTime(100)
144+
})
145+
128146
// Verify the API config was updated with the custom model ID
129147
expect(mockSetApiConfigurationField).toHaveBeenCalledWith(defaultProps.modelIdKey, customModelId)
130148
})

0 commit comments

Comments
 (0)