Skip to content

Commit 7f52417

Browse files
roomotedaniel-lxs
authored andcommitted
Add comprehensive tests for MarketplaceViewStateManager
- Add tests for displayItems initialization fix that prevents marketplace blanking - Test state management scenarios including filtering, tab switching, and error handling - Verify that displayItems properly falls back to allItems when no filters are active - Test infinite loop prevention and proper state change notifications - Ensure immutability of state objects returned by getState() Addresses comment in PR #4796: #4796 (comment)
1 parent 46755d1 commit 7f52417

File tree

1 file changed

+397
-0
lines changed

1 file changed

+397
-0
lines changed
Lines changed: 397 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,397 @@
1+
import { MarketplaceViewStateManager, ViewState, ViewStateTransition } from "../MarketplaceViewStateManager"
2+
import { MarketplaceItem } from "@roo-code/types"
3+
4+
// Mock vscode module
5+
jest.mock("@/utils/vscode", () => ({
6+
vscode: {
7+
postMessage: jest.fn(),
8+
},
9+
}))
10+
11+
describe("MarketplaceViewStateManager", () => {
12+
let stateManager: MarketplaceViewStateManager
13+
let mockStateChangeHandler: jest.Mock
14+
15+
const mockMarketplaceItems: MarketplaceItem[] = [
16+
{
17+
id: "test-mcp-1",
18+
name: "Test MCP Server 1",
19+
description: "A test MCP server",
20+
type: "mcp",
21+
url: "https://example.com/test-mcp-1",
22+
content: "test content",
23+
tags: ["test", "mcp"],
24+
},
25+
{
26+
id: "test-mode-1",
27+
name: "Test Mode 1",
28+
description: "A test mode",
29+
type: "mode",
30+
content: "test content",
31+
tags: ["test", "mode"],
32+
},
33+
{
34+
id: "test-mcp-2",
35+
name: "Test MCP Server 2",
36+
description: "Another test MCP server",
37+
type: "mcp",
38+
url: "https://example.com/test-mcp-2",
39+
content: "test content",
40+
tags: ["test", "server"],
41+
},
42+
]
43+
44+
beforeEach(() => {
45+
stateManager = new MarketplaceViewStateManager()
46+
mockStateChangeHandler = jest.fn()
47+
stateManager.onStateChange(mockStateChangeHandler)
48+
})
49+
50+
afterEach(() => {
51+
stateManager.cleanup()
52+
jest.clearAllMocks()
53+
})
54+
55+
describe("initialization", () => {
56+
it("should initialize with default state", () => {
57+
const state = stateManager.getState()
58+
59+
expect(state.allItems).toEqual([])
60+
expect(state.displayItems).toEqual([])
61+
expect(state.isFetching).toBe(false)
62+
expect(state.activeTab).toBe("mcp")
63+
expect(state.filters).toEqual({
64+
type: "",
65+
search: "",
66+
tags: [],
67+
})
68+
})
69+
70+
it("should ensure displayItems is never undefined in getState", () => {
71+
const state = stateManager.getState()
72+
73+
expect(state.displayItems).toBeDefined()
74+
expect(Array.isArray(state.displayItems)).toBe(true)
75+
})
76+
})
77+
78+
describe("displayItems initialization fix", () => {
79+
it("should fall back to allItems when displayItems is undefined", () => {
80+
// Simulate the scenario where displayItems might be undefined
81+
const transition: ViewStateTransition = {
82+
type: "FETCH_COMPLETE",
83+
payload: { items: mockMarketplaceItems },
84+
}
85+
86+
stateManager.transition(transition)
87+
const state = stateManager.getState()
88+
89+
// Verify that displayItems is properly initialized with allItems when no filters are active
90+
expect(state.displayItems).toEqual(mockMarketplaceItems)
91+
expect(state.allItems).toEqual(mockMarketplaceItems)
92+
})
93+
94+
it("should ensure displayItems defaults to allItems when no filters are active", async () => {
95+
// Handle message with marketplace items (simulating the fix scenario)
96+
const message = {
97+
type: "state",
98+
state: {
99+
marketplaceItems: mockMarketplaceItems,
100+
},
101+
}
102+
103+
await stateManager.handleMessage(message)
104+
const state = stateManager.getState()
105+
106+
// Verify the fix: displayItems should equal allItems when no filters are active
107+
expect(state.displayItems).toEqual(mockMarketplaceItems)
108+
expect(state.allItems).toEqual(mockMarketplaceItems)
109+
expect(state.displayItems?.length).toBeGreaterThan(0)
110+
})
111+
112+
it("should prevent marketplace blanking by ensuring displayItems is never empty when allItems has content", async () => {
113+
// This test specifically addresses the bug described in the PR
114+
const message = {
115+
type: "state",
116+
state: {
117+
marketplaceItems: mockMarketplaceItems,
118+
},
119+
}
120+
121+
await stateManager.handleMessage(message)
122+
const state = stateManager.getState()
123+
124+
// The key fix: displayItems should never be empty when allItems has content and no filters are active
125+
expect(state.allItems.length).toBeGreaterThan(0)
126+
expect(state.displayItems?.length).toBeGreaterThan(0)
127+
expect(state.displayItems).toEqual(state.allItems)
128+
})
129+
})
130+
131+
describe("state change notifications", () => {
132+
it("should notify handlers when marketplace items are loaded", async () => {
133+
const message = {
134+
type: "state",
135+
state: {
136+
marketplaceItems: mockMarketplaceItems,
137+
},
138+
}
139+
140+
await stateManager.handleMessage(message)
141+
142+
expect(mockStateChangeHandler).toHaveBeenCalled()
143+
const notifiedState = mockStateChangeHandler.mock.calls[0][0]
144+
expect(notifiedState.allItems).toEqual(mockMarketplaceItems)
145+
expect(notifiedState.displayItems).toEqual(mockMarketplaceItems)
146+
})
147+
148+
it("should prevent infinite loops by properly handling initial state", async () => {
149+
// Simulate multiple rapid state updates that could cause infinite loops
150+
const message1 = {
151+
type: "state",
152+
state: {
153+
marketplaceItems: [],
154+
},
155+
}
156+
157+
const message2 = {
158+
type: "state",
159+
state: {
160+
marketplaceItems: mockMarketplaceItems,
161+
},
162+
}
163+
164+
await stateManager.handleMessage(message1)
165+
await stateManager.handleMessage(message2)
166+
167+
// Should have been called twice, not infinitely
168+
expect(mockStateChangeHandler).toHaveBeenCalledTimes(2)
169+
170+
const finalState = stateManager.getState()
171+
expect(finalState.displayItems).toEqual(mockMarketplaceItems)
172+
})
173+
})
174+
175+
describe("filtering behavior", () => {
176+
beforeEach(async () => {
177+
// Set up initial state with items
178+
const message = {
179+
type: "state",
180+
state: {
181+
marketplaceItems: mockMarketplaceItems,
182+
},
183+
}
184+
await stateManager.handleMessage(message)
185+
})
186+
187+
it("should show all items when no filters are active", () => {
188+
const state = stateManager.getState()
189+
190+
expect(stateManager.isFilterActive()).toBe(false)
191+
expect(state.displayItems).toEqual(mockMarketplaceItems)
192+
})
193+
194+
it("should filter items when filters are applied", async () => {
195+
// First update the filters
196+
const filterTransition: ViewStateTransition = {
197+
type: "UPDATE_FILTERS",
198+
payload: {
199+
filters: { type: "mcp" },
200+
},
201+
}
202+
203+
await stateManager.transition(filterTransition)
204+
205+
// Then simulate a state message that would trigger filtering
206+
const message = {
207+
type: "state",
208+
state: {
209+
marketplaceItems: mockMarketplaceItems,
210+
},
211+
}
212+
await stateManager.handleMessage(message)
213+
214+
const state = stateManager.getState()
215+
216+
expect(stateManager.isFilterActive()).toBe(true)
217+
expect(state.displayItems?.length).toBe(2) // Only MCP items
218+
expect(state.displayItems?.every((item) => item.type === "mcp")).toBe(true)
219+
})
220+
221+
it("should restore all items when filters are cleared", async () => {
222+
// First apply a filter
223+
await stateManager.transition({
224+
type: "UPDATE_FILTERS",
225+
payload: { filters: { type: "mcp" } },
226+
})
227+
228+
// Simulate state message with filter active
229+
let message = {
230+
type: "state",
231+
state: {
232+
marketplaceItems: mockMarketplaceItems,
233+
},
234+
}
235+
await stateManager.handleMessage(message)
236+
237+
// Then clear the filter
238+
await stateManager.transition({
239+
type: "UPDATE_FILTERS",
240+
payload: { filters: { type: "" } },
241+
})
242+
243+
// Simulate state message with filter cleared
244+
message = {
245+
type: "state",
246+
state: {
247+
marketplaceItems: mockMarketplaceItems,
248+
},
249+
}
250+
await stateManager.handleMessage(message)
251+
252+
const state = stateManager.getState()
253+
expect(stateManager.isFilterActive()).toBe(false)
254+
expect(state.displayItems).toEqual(mockMarketplaceItems)
255+
})
256+
257+
it("should handle search filters correctly", async () => {
258+
await stateManager.transition({
259+
type: "UPDATE_FILTERS",
260+
payload: { filters: { search: "Mode" } },
261+
})
262+
263+
// Simulate state message to trigger filtering
264+
const message = {
265+
type: "state",
266+
state: {
267+
marketplaceItems: mockMarketplaceItems,
268+
},
269+
}
270+
await stateManager.handleMessage(message)
271+
272+
const state = stateManager.getState()
273+
expect(state.displayItems?.length).toBe(1)
274+
expect(state.displayItems?.[0].name).toBe("Test Mode 1")
275+
})
276+
277+
it("should handle tag filters correctly", async () => {
278+
await stateManager.transition({
279+
type: "UPDATE_FILTERS",
280+
payload: { filters: { tags: ["server"] } },
281+
})
282+
283+
// Simulate state message to trigger filtering
284+
const message = {
285+
type: "state",
286+
state: {
287+
marketplaceItems: mockMarketplaceItems,
288+
},
289+
}
290+
await stateManager.handleMessage(message)
291+
292+
const state = stateManager.getState()
293+
expect(state.displayItems?.length).toBe(1)
294+
expect(state.displayItems?.[0].name).toBe("Test MCP Server 2")
295+
})
296+
})
297+
298+
describe("tab switching", () => {
299+
it("should update active tab", async () => {
300+
await stateManager.transition({
301+
type: "SET_ACTIVE_TAB",
302+
payload: { tab: "mode" },
303+
})
304+
305+
const state = stateManager.getState()
306+
expect(state.activeTab).toBe("mode")
307+
})
308+
309+
it("should preserve items when switching tabs", async () => {
310+
// Load items first
311+
const message = {
312+
type: "state",
313+
state: {
314+
marketplaceItems: mockMarketplaceItems,
315+
},
316+
}
317+
await stateManager.handleMessage(message)
318+
319+
// Switch tab
320+
await stateManager.transition({
321+
type: "SET_ACTIVE_TAB",
322+
payload: { tab: "mode" },
323+
})
324+
325+
const state = stateManager.getState()
326+
expect(state.activeTab).toBe("mode")
327+
expect(state.allItems).toEqual(mockMarketplaceItems)
328+
expect(state.displayItems).toEqual(mockMarketplaceItems)
329+
})
330+
})
331+
332+
describe("error handling", () => {
333+
it("should handle empty or invalid messages gracefully", async () => {
334+
await stateManager.handleMessage(null)
335+
await stateManager.handleMessage({})
336+
await stateManager.handleMessage({ type: "invalidType" })
337+
338+
const state = stateManager.getState()
339+
expect(state.allItems).toEqual([])
340+
expect(state.displayItems).toEqual([])
341+
})
342+
343+
it("should handle fetch errors", async () => {
344+
// First load some items
345+
const message = {
346+
type: "state",
347+
state: {
348+
marketplaceItems: mockMarketplaceItems,
349+
},
350+
}
351+
await stateManager.handleMessage(message)
352+
353+
// Then trigger an error
354+
await stateManager.transition({ type: "FETCH_ERROR" })
355+
356+
const state = stateManager.getState()
357+
expect(state.isFetching).toBe(false)
358+
// Items should be preserved during error
359+
expect(state.allItems).toEqual(mockMarketplaceItems)
360+
})
361+
})
362+
363+
describe("state copying and immutability", () => {
364+
it("should return new arrays in getState to prevent mutation", () => {
365+
const state1 = stateManager.getState()
366+
const state2 = stateManager.getState()
367+
368+
expect(state1.allItems).not.toBe(state2.allItems)
369+
expect(state1.displayItems).not.toBe(state2.displayItems)
370+
expect(state1.filters.tags).not.toBe(state2.filters.tags)
371+
})
372+
373+
it("should not mutate original state when modifying returned state", async () => {
374+
const message = {
375+
type: "state",
376+
state: {
377+
marketplaceItems: mockMarketplaceItems,
378+
},
379+
}
380+
await stateManager.handleMessage(message)
381+
382+
const state = stateManager.getState()
383+
state.allItems.push({
384+
id: "mutated",
385+
name: "Mutated Item",
386+
description: "Should not affect original",
387+
type: "mcp",
388+
url: "https://example.com/mutated",
389+
content: "test",
390+
})
391+
392+
const newState = stateManager.getState()
393+
expect(newState.allItems.length).toBe(mockMarketplaceItems.length)
394+
expect(newState.allItems.find((item) => item.id === "mutated")).toBeUndefined()
395+
})
396+
})
397+
})

0 commit comments

Comments
 (0)