Skip to content

Commit b490e57

Browse files
cameroncookecameroncooke
committed
fix(coverage): Inject filesystem into coverage tool handlers
Use createTypedToolWithContext so coverage handlers receive both command and filesystem executors. This keeps handler execution aligned with dependency injection instead of falling back to direct fs checks. Update coverage tests to use context-based logic signatures and add handler-path DI assertions for both tools.
1 parent 1361f28 commit b490e57

4 files changed

Lines changed: 320 additions & 93 deletions

File tree

‎src/mcp/tools/coverage/__tests__/get_coverage_report.test.ts‎

Lines changed: 142 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,13 @@
33
* Covers happy-path, target filtering, showFiles, and failure paths
44
*/
55

6-
import { describe, it, expect } from 'vitest';
6+
import { afterEach, describe, it, expect } from 'vitest';
77
import { createMockExecutor, createMockFileSystemExecutor } from '../../../../test-utils/mock-executors.ts';
8+
import {
9+
__setTestCommandExecutorOverride,
10+
__setTestFileSystemExecutorOverride,
11+
__clearTestExecutorOverrides,
12+
} from '../../../../utils/execution/index.ts';
813
import { schema, handler, get_coverage_reportLogic } from '../get_coverage_report.ts';
914

1015
const sampleTargets = [
@@ -20,8 +25,20 @@ const sampleTargetsWithFiles = [
2025
executableLines: 200,
2126
lineCoverage: 0.5,
2227
files: [
23-
{ name: 'AppDelegate.swift', path: '/src/AppDelegate.swift', coveredLines: 10, executableLines: 50, lineCoverage: 0.2 },
24-
{ name: 'ViewModel.swift', path: '/src/ViewModel.swift', coveredLines: 90, executableLines: 150, lineCoverage: 0.6 },
28+
{
29+
name: 'AppDelegate.swift',
30+
path: '/src/AppDelegate.swift',
31+
coveredLines: 10,
32+
executableLines: 50,
33+
lineCoverage: 0.2,
34+
},
35+
{
36+
name: 'ViewModel.swift',
37+
path: '/src/ViewModel.swift',
38+
coveredLines: 90,
39+
executableLines: 150,
40+
lineCoverage: 0.6,
41+
},
2542
],
2643
},
2744
{
@@ -30,14 +47,30 @@ const sampleTargetsWithFiles = [
3047
executableLines: 500,
3148
lineCoverage: 0.1,
3249
files: [
33-
{ name: 'Service.swift', path: '/src/Service.swift', coveredLines: 0, executableLines: 300, lineCoverage: 0 },
34-
{ name: 'Model.swift', path: '/src/Model.swift', coveredLines: 50, executableLines: 200, lineCoverage: 0.25 },
50+
{
51+
name: 'Service.swift',
52+
path: '/src/Service.swift',
53+
coveredLines: 0,
54+
executableLines: 300,
55+
lineCoverage: 0,
56+
},
57+
{
58+
name: 'Model.swift',
59+
path: '/src/Model.swift',
60+
coveredLines: 50,
61+
executableLines: 200,
62+
lineCoverage: 0.25,
63+
},
3564
],
3665
},
3766
];
3867

3968
const mockFileSystem = createMockFileSystemExecutor({ existsSync: () => true });
4069

70+
afterEach(() => {
71+
__clearTestExecutorOverrides();
72+
});
73+
4174
describe('get_coverage_report', () => {
4275
describe('Export Validation', () => {
4376
it('should export get_coverage_reportLogic function', () => {
@@ -55,16 +88,59 @@ describe('get_coverage_report', () => {
5588
});
5689
});
5790

91+
describe('Handler DI', () => {
92+
it('should use injected fileSystem from handler context', async () => {
93+
const mockExecutor = createMockExecutor({ success: true, output: JSON.stringify(sampleTargets) });
94+
const missingFs = createMockFileSystemExecutor({ existsSync: () => false });
95+
96+
__setTestCommandExecutorOverride(mockExecutor);
97+
__setTestFileSystemExecutorOverride(missingFs);
98+
99+
const result = await handler({ xcresultPath: '/tmp/missing.xcresult', showFiles: false });
100+
101+
expect(result.isError).toBe(true);
102+
const text = result.content[0].type === 'text' ? result.content[0].text : '';
103+
expect(text).toContain('File not found');
104+
});
105+
106+
it('should use injected command executor on handler happy path', async () => {
107+
const commands: string[][] = [];
108+
const mockExecutor = createMockExecutor({
109+
success: true,
110+
output: JSON.stringify(sampleTargets),
111+
onExecute: (command) => {
112+
commands.push(command);
113+
},
114+
});
115+
const existingFs = createMockFileSystemExecutor({ existsSync: () => true });
116+
117+
__setTestCommandExecutorOverride(mockExecutor);
118+
__setTestFileSystemExecutorOverride(existingFs);
119+
120+
const result = await handler({ xcresultPath: '/tmp/test.xcresult' });
121+
122+
expect(result.isError).toBeUndefined();
123+
expect(commands).toHaveLength(1);
124+
expect(commands[0]).toContain('--only-targets');
125+
expect(commands[0]).toContain('/tmp/test.xcresult');
126+
});
127+
});
128+
58129
describe('Command Generation', () => {
59130
it('should use --only-targets when showFiles is false', async () => {
60131
const commands: string[][] = [];
61132
const mockExecutor = createMockExecutor({
62133
success: true,
63134
output: JSON.stringify(sampleTargets),
64-
onExecute: (command) => { commands.push(command); },
135+
onExecute: (command) => {
136+
commands.push(command);
137+
},
65138
});
66139

67-
await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', showFiles: false }, mockExecutor, mockFileSystem);
140+
await get_coverage_reportLogic(
141+
{ xcresultPath: '/tmp/test.xcresult', showFiles: false },
142+
{ executor: mockExecutor, fileSystem: mockFileSystem },
143+
);
68144

69145
expect(commands).toHaveLength(1);
70146
expect(commands[0]).toContain('--only-targets');
@@ -77,10 +153,15 @@ describe('get_coverage_report', () => {
77153
const mockExecutor = createMockExecutor({
78154
success: true,
79155
output: JSON.stringify(sampleTargetsWithFiles),
80-
onExecute: (command) => { commands.push(command); },
156+
onExecute: (command) => {
157+
commands.push(command);
158+
},
81159
});
82160

83-
await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', showFiles: true }, mockExecutor, mockFileSystem);
161+
await get_coverage_reportLogic(
162+
{ xcresultPath: '/tmp/test.xcresult', showFiles: true },
163+
{ executor: mockExecutor, fileSystem: mockFileSystem },
164+
);
84165

85166
expect(commands).toHaveLength(1);
86167
expect(commands[0]).not.toContain('--only-targets');
@@ -94,15 +175,17 @@ describe('get_coverage_report', () => {
94175
output: JSON.stringify(sampleTargets),
95176
});
96177

97-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', showFiles: false }, mockExecutor, mockFileSystem);
178+
const result = await get_coverage_reportLogic(
179+
{ xcresultPath: '/tmp/test.xcresult', showFiles: false },
180+
{ executor: mockExecutor, fileSystem: mockFileSystem },
181+
);
98182

99183
expect(result.isError).toBeUndefined();
100184
expect(result.content).toHaveLength(1);
101185
const text = result.content[0].type === 'text' ? result.content[0].text : '';
102186
expect(text).toContain('Code Coverage Report');
103187
expect(text).toContain('Overall: 24.7%');
104188
expect(text).toContain('180/730 lines');
105-
// Should be sorted ascending: Core (10%), MyApp (50%), Tests (100%)
106189
const coreIdx = text.indexOf('Core');
107190
const appIdx = text.indexOf('MyApp.app');
108191
const testIdx = text.indexOf('MyAppTests.xctest');
@@ -116,7 +199,10 @@ describe('get_coverage_report', () => {
116199
output: JSON.stringify(sampleTargets),
117200
});
118201

119-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', showFiles: false }, mockExecutor, mockFileSystem);
202+
const result = await get_coverage_reportLogic(
203+
{ xcresultPath: '/tmp/test.xcresult', showFiles: false },
204+
{ executor: mockExecutor, fileSystem: mockFileSystem },
205+
);
120206

121207
expect(result.nextStepParams).toEqual({
122208
get_file_coverage: { xcresultPath: '/tmp/test.xcresult' },
@@ -130,7 +216,10 @@ describe('get_coverage_report', () => {
130216
output: JSON.stringify(nestedData),
131217
});
132218

133-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', showFiles: false }, mockExecutor, mockFileSystem);
219+
const result = await get_coverage_reportLogic(
220+
{ xcresultPath: '/tmp/test.xcresult', showFiles: false },
221+
{ executor: mockExecutor, fileSystem: mockFileSystem },
222+
);
134223

135224
expect(result.isError).toBeUndefined();
136225
const text = result.content[0].type === 'text' ? result.content[0].text : '';
@@ -146,7 +235,10 @@ describe('get_coverage_report', () => {
146235
output: JSON.stringify(sampleTargets),
147236
});
148237

149-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', target: 'MyApp', showFiles: false }, mockExecutor, mockFileSystem);
238+
const result = await get_coverage_reportLogic(
239+
{ xcresultPath: '/tmp/test.xcresult', target: 'MyApp', showFiles: false },
240+
{ executor: mockExecutor, fileSystem: mockFileSystem },
241+
);
150242

151243
expect(result.isError).toBeUndefined();
152244
const text = result.content[0].type === 'text' ? result.content[0].text : '';
@@ -161,7 +253,10 @@ describe('get_coverage_report', () => {
161253
output: JSON.stringify(sampleTargets),
162254
});
163255

164-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', target: 'core', showFiles: false }, mockExecutor, mockFileSystem);
256+
const result = await get_coverage_reportLogic(
257+
{ xcresultPath: '/tmp/test.xcresult', target: 'core', showFiles: false },
258+
{ executor: mockExecutor, fileSystem: mockFileSystem },
259+
);
165260

166261
expect(result.isError).toBeUndefined();
167262
const text = result.content[0].type === 'text' ? result.content[0].text : '';
@@ -174,7 +269,10 @@ describe('get_coverage_report', () => {
174269
output: JSON.stringify(sampleTargets),
175270
});
176271

177-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', target: 'NonExistent', showFiles: false }, mockExecutor, mockFileSystem);
272+
const result = await get_coverage_reportLogic(
273+
{ xcresultPath: '/tmp/test.xcresult', target: 'NonExistent', showFiles: false },
274+
{ executor: mockExecutor, fileSystem: mockFileSystem },
275+
);
178276

179277
expect(result.isError).toBe(true);
180278
const text = result.content[0].type === 'text' ? result.content[0].text : '';
@@ -189,7 +287,10 @@ describe('get_coverage_report', () => {
189287
output: JSON.stringify(sampleTargetsWithFiles),
190288
});
191289

192-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', showFiles: true }, mockExecutor, mockFileSystem);
290+
const result = await get_coverage_reportLogic(
291+
{ xcresultPath: '/tmp/test.xcresult', showFiles: true },
292+
{ executor: mockExecutor, fileSystem: mockFileSystem },
293+
);
193294

194295
expect(result.isError).toBeUndefined();
195296
const text = result.content[0].type === 'text' ? result.content[0].text : '';
@@ -205,10 +306,12 @@ describe('get_coverage_report', () => {
205306
output: JSON.stringify(sampleTargetsWithFiles),
206307
});
207308

208-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', showFiles: true }, mockExecutor, mockFileSystem);
309+
const result = await get_coverage_reportLogic(
310+
{ xcresultPath: '/tmp/test.xcresult', showFiles: true },
311+
{ executor: mockExecutor, fileSystem: mockFileSystem },
312+
);
209313

210314
const text = result.content[0].type === 'text' ? result.content[0].text : '';
211-
// Under MyApp.app: AppDelegate (20%) before ViewModel (60%)
212315
const appDelegateIdx = text.indexOf('AppDelegate.swift');
213316
const viewModelIdx = text.indexOf('ViewModel.swift');
214317
expect(appDelegateIdx).toBeLessThan(viewModelIdx);
@@ -220,7 +323,10 @@ describe('get_coverage_report', () => {
220323
const missingFs = createMockFileSystemExecutor({ existsSync: () => false });
221324
const mockExecutor = createMockExecutor({ success: true, output: '{}' });
222325

223-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/missing.xcresult', showFiles: false }, mockExecutor, missingFs);
326+
const result = await get_coverage_reportLogic(
327+
{ xcresultPath: '/tmp/missing.xcresult', showFiles: false },
328+
{ executor: mockExecutor, fileSystem: missingFs },
329+
);
224330

225331
expect(result.isError).toBe(true);
226332
const text = result.content[0].type === 'text' ? result.content[0].text : '';
@@ -234,7 +340,10 @@ describe('get_coverage_report', () => {
234340
error: 'Failed to load result bundle',
235341
});
236342

237-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/bad.xcresult', showFiles: false }, mockExecutor, mockFileSystem);
343+
const result = await get_coverage_reportLogic(
344+
{ xcresultPath: '/tmp/bad.xcresult', showFiles: false },
345+
{ executor: mockExecutor, fileSystem: mockFileSystem },
346+
);
238347

239348
expect(result.isError).toBe(true);
240349
const text = result.content[0].type === 'text' ? result.content[0].text : '';
@@ -248,7 +357,10 @@ describe('get_coverage_report', () => {
248357
output: 'not valid json',
249358
});
250359

251-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', showFiles: false }, mockExecutor, mockFileSystem);
360+
const result = await get_coverage_reportLogic(
361+
{ xcresultPath: '/tmp/test.xcresult', showFiles: false },
362+
{ executor: mockExecutor, fileSystem: mockFileSystem },
363+
);
252364

253365
expect(result.isError).toBe(true);
254366
const text = result.content[0].type === 'text' ? result.content[0].text : '';
@@ -261,7 +373,10 @@ describe('get_coverage_report', () => {
261373
output: JSON.stringify({ unexpected: 'format' }),
262374
});
263375

264-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', showFiles: false }, mockExecutor, mockFileSystem);
376+
const result = await get_coverage_reportLogic(
377+
{ xcresultPath: '/tmp/test.xcresult', showFiles: false },
378+
{ executor: mockExecutor, fileSystem: mockFileSystem },
379+
);
265380

266381
expect(result.isError).toBe(true);
267382
const text = result.content[0].type === 'text' ? result.content[0].text : '';
@@ -274,7 +389,10 @@ describe('get_coverage_report', () => {
274389
output: JSON.stringify([]),
275390
});
276391

277-
const result = await get_coverage_reportLogic({ xcresultPath: '/tmp/test.xcresult', showFiles: false }, mockExecutor, mockFileSystem);
392+
const result = await get_coverage_reportLogic(
393+
{ xcresultPath: '/tmp/test.xcresult', showFiles: false },
394+
{ executor: mockExecutor, fileSystem: mockFileSystem },
395+
);
278396

279397
expect(result.isError).toBe(true);
280398
const text = result.content[0].type === 'text' ? result.content[0].text : '';

0 commit comments

Comments
 (0)