Skip to content

Commit 113bf09

Browse files
committed
refactor: make tests property-based, not data-dependent
Refactored tests to focus on behavioral properties rather than specific mock data. Tests now verify the filtering rules themselves, not hardcoded expectations about specific command counts. **Changes:** - Removed large mockActions array - Each test creates minimal data to test specific property - Tests organized by property (e.g., "default mode shows only ws:switch:*") - Tests are resilient to command list changes **Properties tested:** 1. Default mode only returns ws:switch:* commands 2. > mode never returns ws:switch:* commands 3. Modes partition the command space (no overlap, no gaps) 4. / prefix always returns empty 5. Text after > doesn't affect our filter (cmdk handles it) **Benefits:** - Tests won't break when commands are added/removed - Clear intent - each test verifies one property - More maintainable - minimal test data per property - Better test names describe what they verify Addresses review feedback about brittle tests.
1 parent deab377 commit 113bf09

File tree

1 file changed

+97
-70
lines changed

1 file changed

+97
-70
lines changed

src/components/CommandPalette.test.ts

Lines changed: 97 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -3,89 +3,116 @@ import { filterCommandsByPrefix } from "@/utils/commandPaletteFiltering";
33

44
/**
55
* Tests for command palette filtering logic
6-
* Verifies the "workspace switcher by default, commands with >" behavior
6+
* Property-based tests that verify behavior regardless of specific command data
77
*/
88

9-
interface Action {
10-
id: string;
11-
title: string;
12-
section: string;
13-
}
14-
15-
const mockActions: Action[] = [
16-
{ id: "ws:switch:1", title: "Switch to Workspace A", section: "Workspaces" },
17-
{ id: "ws:switch:2", title: "Switch to Workspace B", section: "Workspaces" },
18-
{ id: "ws:new", title: "Create New Workspace", section: "Workspaces" },
19-
{ id: "ws:remove", title: "Remove Current Workspace", section: "Workspaces" },
20-
{ id: "ws:rename", title: "Rename Current Workspace", section: "Workspaces" },
21-
{ id: "ws:open-terminal", title: "Open Workspace in Terminal", section: "Workspaces" },
22-
{ id: "nav1", title: "Toggle Sidebar", section: "Navigation" },
23-
{ id: "chat1", title: "Clear Chat", section: "Chat" },
24-
];
25-
269
describe("CommandPalette filtering", () => {
27-
test("default (no prefix) shows only workspace switching commands", () => {
28-
const result = filterCommandsByPrefix("", mockActions);
29-
30-
expect(result).toHaveLength(2);
31-
expect(result.every((a) => a.id.startsWith("ws:switch:"))).toBe(true);
32-
expect(result.some((a) => a.id === "ws:switch:1")).toBe(true);
33-
expect(result.some((a) => a.id === "ws:switch:2")).toBe(true);
10+
describe("property: default mode shows only ws:switch:* commands", () => {
11+
test("all results start with ws:switch:", () => {
12+
const actions = [
13+
{ id: "ws:switch:1" },
14+
{ id: "ws:switch:2" },
15+
{ id: "ws:new" },
16+
{ id: "nav:toggle" },
17+
];
18+
19+
const result = filterCommandsByPrefix("", actions);
20+
21+
expect(result.every((a) => a.id.startsWith("ws:switch:"))).toBe(true);
22+
});
23+
24+
test("excludes all non-switching commands", () => {
25+
const actions = [
26+
{ id: "ws:switch:1" },
27+
{ id: "ws:new" },
28+
{ id: "ws:remove" },
29+
{ id: "nav:toggle" },
30+
];
31+
32+
const result = filterCommandsByPrefix("", actions);
33+
34+
expect(result.some((a) => !a.id.startsWith("ws:switch:"))).toBe(false);
35+
});
3436
});
3537

36-
test("default query excludes workspace mutations", () => {
37-
const result = filterCommandsByPrefix("", mockActions);
38-
39-
expect(result.some((a) => a.id === "ws:new")).toBe(false);
40-
expect(result.some((a) => a.id === "ws:remove")).toBe(false);
41-
expect(result.some((a) => a.id === "ws:rename")).toBe(false);
38+
describe("property: > mode shows all EXCEPT ws:switch:* commands", () => {
39+
test("no results start with ws:switch:", () => {
40+
const actions = [
41+
{ id: "ws:switch:1" },
42+
{ id: "ws:new" },
43+
{ id: "nav:toggle" },
44+
{ id: "chat:clear" },
45+
];
46+
47+
const result = filterCommandsByPrefix(">", actions);
48+
49+
expect(result.every((a) => !a.id.startsWith("ws:switch:"))).toBe(true);
50+
});
51+
52+
test("includes all non-switching commands", () => {
53+
const actions = [
54+
{ id: "ws:switch:1" },
55+
{ id: "ws:new" },
56+
{ id: "ws:remove" },
57+
{ id: "nav:toggle" },
58+
];
59+
60+
const result = filterCommandsByPrefix(">", actions);
61+
62+
// Should include workspace mutations
63+
expect(result.some((a) => a.id === "ws:new")).toBe(true);
64+
expect(result.some((a) => a.id === "ws:remove")).toBe(true);
65+
// Should include navigation
66+
expect(result.some((a) => a.id === "nav:toggle")).toBe(true);
67+
// Should NOT include switching
68+
expect(result.some((a) => a.id === "ws:switch:1")).toBe(false);
69+
});
4270
});
4371

44-
test("> prefix shows all commands EXCEPT switching", () => {
45-
const result = filterCommandsByPrefix(">", mockActions);
46-
47-
// Should show 6 commands (3 workspace mutations + 1 terminal + 1 nav + 1 chat)
48-
expect(result).toHaveLength(6);
49-
50-
// Should NOT include switching commands
51-
expect(result.every((a) => !a.id.startsWith("ws:switch:"))).toBe(true);
52-
53-
// Should include workspace mutations
54-
expect(result.some((a) => a.id === "ws:new")).toBe(true);
55-
expect(result.some((a) => a.id === "ws:remove")).toBe(true);
56-
expect(result.some((a) => a.id === "ws:rename")).toBe(true);
57-
58-
// Should include other sections
59-
expect(result.some((a) => a.id === "nav1")).toBe(true);
60-
expect(result.some((a) => a.id === "chat1")).toBe(true);
72+
describe("property: modes partition the command space", () => {
73+
test("default + > modes cover all commands (no overlap, no gaps)", () => {
74+
const actions = [
75+
{ id: "ws:switch:1" },
76+
{ id: "ws:switch:2" },
77+
{ id: "ws:new" },
78+
{ id: "ws:remove" },
79+
{ id: "nav:toggle" },
80+
{ id: "chat:clear" },
81+
];
82+
83+
const defaultResult = filterCommandsByPrefix("", actions);
84+
const commandResult = filterCommandsByPrefix(">", actions);
85+
86+
// No overlap - disjoint sets
87+
const defaultIds = new Set(defaultResult.map((a) => a.id));
88+
const commandIds = new Set(commandResult.map((a) => a.id));
89+
const intersection = [...defaultIds].filter((id) => commandIds.has(id));
90+
expect(intersection).toHaveLength(0);
91+
92+
// No gaps - covers everything
93+
expect(defaultResult.length + commandResult.length).toBe(actions.length);
94+
});
6195
});
6296

63-
test(">query with text shows non-switching commands (cmdk filters further)", () => {
64-
const result = filterCommandsByPrefix(">new", mockActions);
97+
describe("property: / prefix always returns empty", () => {
98+
test("returns empty array regardless of actions", () => {
99+
const actions = [{ id: "ws:switch:1" }, { id: "ws:new" }, { id: "nav:toggle" }];
65100

66-
// Our filter shows all non-switching commands
67-
// (cmdk's built-in filter will narrow this down by "new")
68-
expect(result).toHaveLength(6);
69-
expect(result.every((a) => !a.id.startsWith("ws:switch:"))).toBe(true);
101+
expect(filterCommandsByPrefix("/", actions)).toHaveLength(0);
102+
expect(filterCommandsByPrefix("/help", actions)).toHaveLength(0);
103+
expect(filterCommandsByPrefix("/ ", actions)).toHaveLength(0);
104+
});
70105
});
71106

72-
test("/ prefix returns empty (slash commands handled separately)", () => {
73-
const result = filterCommandsByPrefix("/", mockActions);
74-
expect(result).toHaveLength(0);
75-
});
76-
77-
test("clean separation: switching XOR other commands", () => {
78-
const defaultResult = filterCommandsByPrefix("", mockActions);
79-
const commandResult = filterCommandsByPrefix(">", mockActions);
80-
81-
// No overlap
82-
const defaultIds = new Set(defaultResult.map((a) => a.id));
83-
const commandIds = new Set(commandResult.map((a) => a.id));
84-
const intersection = [...defaultIds].filter((id) => commandIds.has(id));
107+
describe("property: query with > prefix applies to all non-switching", () => {
108+
test(">text shows same set as > (cmdk filters further)", () => {
109+
const actions = [{ id: "ws:switch:1" }, { id: "ws:new" }, { id: "nav:toggle" }];
85110

86-
expect(intersection).toHaveLength(0);
111+
// Our filter doesn't care about text after >, just the prefix
112+
const resultEmpty = filterCommandsByPrefix(">", actions);
113+
const resultWithText = filterCommandsByPrefix(">abc", actions);
87114

88-
// Together they cover all non-slash commands
89-
expect(defaultResult.length + commandResult.length).toBe(mockActions.length);
115+
expect(resultEmpty).toEqual(resultWithText);
116+
});
90117
});
91118
});

0 commit comments

Comments
 (0)