Skip to content

Commit 8600c24

Browse files
cameroncookecameroncooke
committed
🐛 fix(smoke-tests): address code review findings for test reliability
- Remove false-positive "provide" match from isErrorResponse (keep "must provide") - Set exitCode dynamically based on success in command response mock - Sort commandResponses by pattern length to prevent shorter patterns shadowing longer ones - Consolidate createMockFileSystemExecutor() calls to single shared instance - Add expectContent() helper and replace ~40 inline content-extraction patterns - Replace remaining inline error-checking blocks with shared isErrorResponse - Guard tool.remove() with try-catch in __resetToolRegistryForTests for safe cleanup - Build before smoke tests in test:smoke npm script to prevent stale-build issues
1 parent dfad2c9 commit 8600c24

12 files changed

Lines changed: 93 additions & 292 deletions

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
"docs:update": "npx tsx scripts/update-tools-docs.ts",
3434
"docs:update:dry-run": "npx tsx scripts/update-tools-docs.ts --dry-run --verbose",
3535
"test": "vitest run",
36-
"test:smoke": "vitest run --config vitest.smoke.config.ts",
36+
"test:smoke": "npm run build && vitest run --config vitest.smoke.config.ts",
3737
"test:watch": "vitest",
3838
"test:ui": "vitest --ui",
3939
"test:coverage": "vitest run --coverage"

‎src/smoke-tests/__tests__/e2e-mcp-device-macos.test.ts‎

Lines changed: 18 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
22
import { createMcpTestHarness, type McpTestHarness } from '../mcp-test-harness.ts';
3-
import { isErrorResponse } from '../test-helpers.ts';
3+
import { isErrorResponse, expectContent } from '../test-helpers.ts';
44

55
let harness: McpTestHarness;
66

@@ -41,10 +41,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
4141
arguments: {},
4242
});
4343

44-
expect(result).toBeDefined();
45-
const content = 'content' in result ? result.content : [];
46-
expect(Array.isArray(content)).toBe(true);
47-
expect(content.length).toBeGreaterThan(0);
44+
expectContent(result);
4845

4946
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
5047
expect(commandStrs.some((c) => c.includes('xcodebuild') && c.includes('MyApp'))).toBe(true);
@@ -66,10 +63,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
6663
arguments: {},
6764
});
6865

69-
expect(result).toBeDefined();
70-
const content = 'content' in result ? result.content : [];
71-
expect(Array.isArray(content)).toBe(true);
72-
expect(content.length).toBeGreaterThan(0);
66+
expectContent(result);
7367

7468
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
7569
expect(commandStrs.some((c) => c.includes('xcodebuild') && c.includes('test'))).toBe(true);
@@ -90,10 +84,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
9084
arguments: {},
9185
});
9286

93-
expect(result).toBeDefined();
94-
const content = 'content' in result ? result.content : [];
95-
expect(Array.isArray(content)).toBe(true);
96-
expect(content.length).toBeGreaterThan(0);
87+
expectContent(result);
9788

9889
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
9990
expect(commandStrs.some((c) => c.includes('devicectl') && c.includes('launch'))).toBe(true);
@@ -113,10 +104,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
113104
arguments: { processId: 12345 },
114105
});
115106

116-
expect(result).toBeDefined();
117-
const content = 'content' in result ? result.content : [];
118-
expect(Array.isArray(content)).toBe(true);
119-
expect(content.length).toBeGreaterThan(0);
107+
expectContent(result);
120108

121109
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
122110
expect(commandStrs.some((c) => c.includes('devicectl') && c.includes('terminate'))).toBe(
@@ -138,10 +126,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
138126
arguments: { appPath: '/path/to/MyApp.app' },
139127
});
140128

141-
expect(result).toBeDefined();
142-
const content = 'content' in result ? result.content : [];
143-
expect(Array.isArray(content)).toBe(true);
144-
expect(content.length).toBeGreaterThan(0);
129+
expectContent(result);
145130

146131
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
147132
expect(commandStrs.some((c) => c.includes('devicectl') && c.includes('install'))).toBe(true);
@@ -162,10 +147,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
162147
arguments: {},
163148
});
164149

165-
expect(result).toBeDefined();
166-
const content = 'content' in result ? result.content : [];
167-
expect(Array.isArray(content)).toBe(true);
168-
expect(content.length).toBeGreaterThan(0);
150+
expectContent(result);
169151

170152
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
171153
expect(
@@ -180,10 +162,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
180162
arguments: {},
181163
});
182164

183-
expect(result).toBeDefined();
184-
const content = 'content' in result ? result.content : [];
185-
expect(Array.isArray(content)).toBe(true);
186-
expect(content.length).toBeGreaterThan(0);
165+
expectContent(result);
187166

188167
expect(harness.capturedCommands.length).toBeGreaterThan(0);
189168
});
@@ -197,10 +176,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
197176
arguments: { workspaceRoot: '/path/to/workspace' },
198177
});
199178

200-
expect(result).toBeDefined();
201-
const content = 'content' in result ? result.content : [];
202-
expect(Array.isArray(content)).toBe(true);
203-
expect(content.length).toBeGreaterThan(0);
179+
expectContent(result);
204180
});
205181

206182
it('get_app_bundle_id responds with content', async () => {
@@ -210,10 +186,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
210186
arguments: { appPath: '/path/to/MyApp.app' },
211187
});
212188

213-
expect(result).toBeDefined();
214-
const content = 'content' in result ? result.content : [];
215-
expect(Array.isArray(content)).toBe(true);
216-
expect(content.length).toBeGreaterThan(0);
189+
expectContent(result);
217190
});
218191

219192
it('get_mac_bundle_id responds with content', async () => {
@@ -223,10 +196,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
223196
arguments: { appPath: '/path/to/MyApp.app' },
224197
});
225198

226-
expect(result).toBeDefined();
227-
const content = 'content' in result ? result.content : [];
228-
expect(Array.isArray(content)).toBe(true);
229-
expect(content.length).toBeGreaterThan(0);
199+
expectContent(result);
230200
});
231201
});
232202

@@ -246,10 +216,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
246216
arguments: {},
247217
});
248218

249-
expect(result).toBeDefined();
250-
const content = 'content' in result ? result.content : [];
251-
expect(Array.isArray(content)).toBe(true);
252-
expect(content.length).toBeGreaterThan(0);
219+
expectContent(result);
253220

254221
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
255222
expect(commandStrs.some((c) => c.includes('xcodebuild') && c.includes('MyMacApp'))).toBe(
@@ -272,10 +239,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
272239
arguments: {},
273240
});
274241

275-
expect(result).toBeDefined();
276-
const content = 'content' in result ? result.content : [];
277-
expect(Array.isArray(content)).toBe(true);
278-
expect(content.length).toBeGreaterThan(0);
242+
expectContent(result);
279243

280244
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
281245
expect(commandStrs.some((c) => c.includes('xcodebuild'))).toBe(true);
@@ -296,10 +260,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
296260
arguments: {},
297261
});
298262

299-
expect(result).toBeDefined();
300-
const content = 'content' in result ? result.content : [];
301-
expect(Array.isArray(content)).toBe(true);
302-
expect(content.length).toBeGreaterThan(0);
263+
expectContent(result);
303264

304265
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
305266
expect(commandStrs.some((c) => c.includes('xcodebuild') && c.includes('test'))).toBe(true);
@@ -312,10 +273,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
312273
arguments: { appPath: '/path/to/MyMacApp.app' },
313274
});
314275

315-
expect(result).toBeDefined();
316-
const content = 'content' in result ? result.content : [];
317-
expect(Array.isArray(content)).toBe(true);
318-
expect(content.length).toBeGreaterThan(0);
276+
expectContent(result);
319277
});
320278

321279
it('stop_mac_app captures kill command with processId', async () => {
@@ -325,10 +283,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
325283
arguments: { processId: 54321 },
326284
});
327285

328-
expect(result).toBeDefined();
329-
const content = 'content' in result ? result.content : [];
330-
expect(Array.isArray(content)).toBe(true);
331-
expect(content.length).toBeGreaterThan(0);
286+
expectContent(result);
332287

333288
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
334289
expect(commandStrs.some((c) => c.includes('kill') && c.includes('54321'))).toBe(true);
@@ -341,10 +296,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
341296
arguments: { appName: 'MyMacApp' },
342297
});
343298

344-
expect(result).toBeDefined();
345-
const content = 'content' in result ? result.content : [];
346-
expect(Array.isArray(content)).toBe(true);
347-
expect(content.length).toBeGreaterThan(0);
299+
expectContent(result);
348300

349301
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
350302
expect(commandStrs.some((c) => c.includes('MyMacApp'))).toBe(true);
@@ -365,10 +317,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
365317
arguments: {},
366318
});
367319

368-
expect(result).toBeDefined();
369-
const content = 'content' in result ? result.content : [];
370-
expect(Array.isArray(content)).toBe(true);
371-
expect(content.length).toBeGreaterThan(0);
320+
expectContent(result);
372321

373322
const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
374323
expect(

‎src/smoke-tests/__tests__/e2e-mcp-doctor.test.ts‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
22
import { createMcpTestHarness, type McpTestHarness } from '../mcp-test-harness.ts';
3+
import { getContent } from '../test-helpers.ts';
34

45
let harness: McpTestHarness;
56

@@ -23,16 +24,12 @@ describe('MCP Doctor Tool (e2e)', () => {
2324
arguments: {},
2425
});
2526

26-
expect(result).toBeDefined();
27-
const content = 'content' in result ? result.content : [];
28-
expect(Array.isArray(content)).toBe(true);
27+
const content = getContent(result);
2928
expect(content.length).toBeGreaterThan(0);
3029

31-
const hasText =
32-
Array.isArray(content) &&
33-
content.some(
34-
(c) => 'text' in c && typeof c.text === 'string' && c.text.includes('XcodeBuildMCP Doctor'),
35-
);
30+
const hasText = content.some(
31+
(c) => typeof c.text === 'string' && c.text.includes('XcodeBuildMCP Doctor'),
32+
);
3633
expect(hasText).toBe(true);
3734
});
3835
});

0 commit comments

Comments
 (0)