Skip to content

Commit 14eaf6e

Browse files
czottmannczottmann
committed
fix: classify stderr warnings correctly and prioritize xcresult output
When xcresult parsing succeeds and tests actually ran (totalTestCount > 0), stderr lines are redundant noise (e.g. "multiple matching destinations") and are filtered out. The xcresult summary is placed first in the response. When xcresult reports 0 tests (build failed before tests could run), the xcresult is meaningless and stderr is preserved since it contains the actual compilation errors. Fixes #231
1 parent db9d7d0 commit 14eaf6e

6 files changed

Lines changed: 295 additions & 28 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## [Unreleased]
4+
5+
### Fixed
6+
7+
- Fixed stderr warnings (e.g. "multiple matching destinations") hiding actual test failures by prioritizing xcresult output when available ([#231](https://github.com/getsentry/XcodeBuildMCP/issues/231))
8+
39
## [2.1.0]
410

511
### Added

‎src/mcp/tools/device/__tests__/test_device.test.ts‎

Lines changed: 74 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,9 @@ describe('test_device plugin', () => {
168168
);
169169

170170
expect(result.content).toHaveLength(2);
171-
expect(result.content[0].text).toContain('✅');
172-
expect(result.content[1].text).toContain('Test Results Summary:');
173-
expect(result.content[1].text).toContain('MyScheme Tests');
171+
expect(result.content[0].text).toContain('Test Results Summary:');
172+
expect(result.content[0].text).toContain('MyScheme Tests');
173+
expect(result.content[1].text).toContain('✅');
174174
});
175175

176176
it('should handle test failure scenarios', async () => {
@@ -214,8 +214,8 @@ describe('test_device plugin', () => {
214214
);
215215

216216
expect(result.content).toHaveLength(2);
217-
expect(result.content[1].text).toContain('Test Failures:');
218-
expect(result.content[1].text).toContain('testExample');
217+
expect(result.content[0].text).toContain('Test Failures:');
218+
expect(result.content[0].text).toContain('testExample');
219219
});
220220

221221
it('should handle xcresult parsing failures gracefully', async () => {
@@ -264,6 +264,70 @@ describe('test_device plugin', () => {
264264
expect(result.content[0].text).toContain('✅');
265265
});
266266

267+
it('should preserve stderr when xcresult reports zero tests (build failure)', async () => {
268+
// When the build fails, xcresult exists but has totalTestCount: 0.
269+
// stderr contains the actual compilation errors and must be preserved.
270+
let callCount = 0;
271+
const mockExecutor = async (
272+
_args: string[],
273+
_description?: string,
274+
_useShell?: boolean,
275+
_opts?: { cwd?: string },
276+
_detached?: boolean,
277+
) => {
278+
callCount++;
279+
280+
// First call: xcodebuild test fails with compilation error
281+
if (callCount === 1) {
282+
return createMockCommandResponse({
283+
success: false,
284+
output: '',
285+
error: 'error: missing argument for parameter in call',
286+
});
287+
}
288+
289+
// Second call: xcresulttool succeeds but reports 0 tests
290+
return createMockCommandResponse({
291+
success: true,
292+
output: JSON.stringify({
293+
title: 'Test Results',
294+
result: 'unknown',
295+
totalTestCount: 0,
296+
passedTests: 0,
297+
failedTests: 0,
298+
skippedTests: 0,
299+
expectedFailures: 0,
300+
}),
301+
});
302+
};
303+
304+
const result = await testDeviceLogic(
305+
{
306+
projectPath: '/path/to/project.xcodeproj',
307+
scheme: 'MyScheme',
308+
deviceId: 'test-device-123',
309+
configuration: 'Debug',
310+
preferXcodebuild: false,
311+
platform: 'iOS',
312+
},
313+
mockExecutor,
314+
createMockFileSystemExecutor({
315+
mkdtemp: async () => '/tmp/xcodebuild-test-buildfail',
316+
tmpdir: () => '/tmp',
317+
stat: async () => ({ isDirectory: () => false, mtimeMs: 0 }),
318+
rm: async () => {},
319+
}),
320+
);
321+
322+
// stderr with compilation error must be preserved
323+
const allText = result.content.map((c) => c.text).join('\n');
324+
expect(allText).toContain('[stderr]');
325+
expect(allText).toContain('missing argument');
326+
327+
// xcresult summary should NOT be present
328+
expect(allText).not.toContain('Test Results Summary:');
329+
});
330+
267331
it('should support different platforms', async () => {
268332
// Mock xcresulttool output
269333
const mockExecutor = createMockExecutor({
@@ -298,7 +362,7 @@ describe('test_device plugin', () => {
298362
);
299363

300364
expect(result.content).toHaveLength(2);
301-
expect(result.content[1].text).toContain('WatchApp Tests');
365+
expect(result.content[0].text).toContain('WatchApp Tests');
302366
});
303367

304368
it('should handle optional parameters', async () => {
@@ -337,7 +401,7 @@ describe('test_device plugin', () => {
337401
);
338402

339403
expect(result.content).toHaveLength(2);
340-
expect(result.content[0].text).toContain('✅');
404+
expect(result.content[1].text).toContain('✅');
341405
});
342406

343407
it('should handle workspace testing successfully', async () => {
@@ -374,9 +438,9 @@ describe('test_device plugin', () => {
374438
);
375439

376440
expect(result.content).toHaveLength(2);
377-
expect(result.content[0].text).toContain('✅');
378-
expect(result.content[1].text).toContain('Test Results Summary:');
379-
expect(result.content[1].text).toContain('WorkspaceScheme Tests');
441+
expect(result.content[0].text).toContain('Test Results Summary:');
442+
expect(result.content[0].text).toContain('WorkspaceScheme Tests');
443+
expect(result.content[1].text).toContain('✅');
380444
});
381445
});
382446
});

‎src/mcp/tools/device/test_device.ts‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,18 @@ const publicSchemaObject = baseSchemaObject.omit({
7676
* (JavaScript implementation - no actual interface, this is just documentation)
7777
*/
7878

79+
interface XcresultSummary {
80+
formatted: string;
81+
totalTestCount: number;
82+
}
83+
7984
/**
8085
* Parse xcresult bundle using xcrun xcresulttool
8186
*/
8287
async function parseXcresultBundle(
8388
resultBundlePath: string,
8489
executor: CommandExecutor = getDefaultCommandExecutor(),
85-
): Promise<string> {
90+
): Promise<XcresultSummary> {
8691
try {
8792
// Use injected executor for testing
8893
const result = await executor(
@@ -98,7 +103,11 @@ async function parseXcresultBundle(
98103

99104
// Parse JSON response and format as human-readable
100105
const summaryData = JSON.parse(result.output) as Record<string, unknown>;
101-
return formatTestSummary(summaryData);
106+
return {
107+
formatted: formatTestSummary(summaryData),
108+
totalTestCount:
109+
typeof summaryData.totalTestCount === 'number' ? summaryData.totalTestCount : 0,
110+
};
102111
} catch (error) {
103112
const errorMessage = error instanceof Error ? error.message : String(error);
104113
log('error', `Error parsing xcresult bundle: ${errorMessage}`);
@@ -252,20 +261,31 @@ export async function testDeviceLogic(
252261
throw new Error(`xcresult bundle not found at ${resultBundlePath}`);
253262
}
254263

255-
const testSummary = await parseXcresultBundle(resultBundlePath, executor);
264+
const xcresult = await parseXcresultBundle(resultBundlePath, executor);
256265
log('info', 'Successfully parsed xcresult bundle');
257266

258267
// Clean up temporary directory
259268
await cleanup();
260269

261-
// Return combined result - preserve isError from testResult (test failures should be marked as errors)
270+
// If no tests ran (e.g. build failed), the xcresult is empty/meaningless.
271+
// Fall back to the original response which contains the actual build errors.
272+
if (xcresult.totalTestCount === 0) {
273+
log('info', 'xcresult reports 0 tests — falling back to raw build output');
274+
return testResult;
275+
}
276+
277+
// When xcresult has real test data, it's the authoritative source.
278+
// Drop stderr lines — they're redundant noise (e.g. "multiple matching destinations").
279+
const filteredContent = (testResult.content || []).filter(
280+
(item) => item.type !== 'text' || !item.text.includes('[stderr]'),
281+
);
262282
return {
263283
content: [
264-
...(testResult.content || []),
265284
{
266285
type: 'text',
267-
text: '\nTest Results Summary:\n' + testSummary,
286+
text: '\nTest Results Summary:\n' + xcresult.formatted,
268287
},
288+
...filteredContent,
269289
],
270290
isError: testResult.isError,
271291
};

‎src/mcp/tools/macos/__tests__/test_macos.test.ts‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,143 @@ describe('test_macos plugin (unified)', () => {
507507
);
508508
});
509509

510+
it('should filter out stderr lines when xcresult data is available', async () => {
511+
// Regression test for #231: stderr warnings (e.g. "multiple matching destinations")
512+
// should be dropped when xcresult parsing succeeds, since xcresult is authoritative.
513+
let callCount = 0;
514+
const mockExecutor = async (
515+
command: string[],
516+
logPrefix?: string,
517+
useShell?: boolean,
518+
opts?: { env?: Record<string, string> },
519+
detached?: boolean,
520+
) => {
521+
callCount++;
522+
void logPrefix;
523+
void useShell;
524+
void opts;
525+
void detached;
526+
527+
// First call: xcodebuild test fails with stderr warning
528+
if (callCount === 1) {
529+
return createMockCommandResponse({
530+
success: false,
531+
output: '',
532+
error:
533+
'WARNING: multiple matching destinations, using first match\n' + 'error: Test failed',
534+
});
535+
}
536+
537+
// Second call: xcresulttool succeeds
538+
if (command.includes('xcresulttool')) {
539+
return createMockCommandResponse({
540+
success: true,
541+
output: JSON.stringify({
542+
title: 'Test Results',
543+
result: 'FAILED',
544+
totalTestCount: 5,
545+
passedTests: 3,
546+
failedTests: 2,
547+
skippedTests: 0,
548+
expectedFailures: 0,
549+
}),
550+
});
551+
}
552+
553+
return createMockCommandResponse({ success: true, output: '' });
554+
};
555+
556+
const mockFileSystemExecutor = createTestFileSystemExecutor({
557+
mkdtemp: async () => '/tmp/xcodebuild-test-stderr',
558+
});
559+
560+
const result = await testMacosLogic(
561+
{
562+
workspacePath: '/path/to/MyProject.xcworkspace',
563+
scheme: 'MyScheme',
564+
},
565+
mockExecutor,
566+
mockFileSystemExecutor,
567+
);
568+
569+
// stderr lines should be filtered out
570+
const allText = result.content.map((c) => c.text).join('\n');
571+
expect(allText).not.toContain('[stderr]');
572+
573+
// xcresult summary should be present and first
574+
expect(result.content[0].text).toContain('Test Results Summary:');
575+
576+
// Build status line should still be present
577+
expect(allText).toContain('Test Run test failed for scheme MyScheme');
578+
});
579+
580+
it('should preserve stderr when xcresult reports zero tests (build failure)', async () => {
581+
// When the build fails, xcresult exists but has totalTestCount: 0.
582+
// In that case stderr contains the actual compilation errors and must be preserved.
583+
let callCount = 0;
584+
const mockExecutor = async (
585+
command: string[],
586+
logPrefix?: string,
587+
useShell?: boolean,
588+
opts?: { env?: Record<string, string> },
589+
detached?: boolean,
590+
) => {
591+
callCount++;
592+
void logPrefix;
593+
void useShell;
594+
void opts;
595+
void detached;
596+
597+
// First call: xcodebuild test fails with compilation error on stderr
598+
if (callCount === 1) {
599+
return createMockCommandResponse({
600+
success: false,
601+
output: '',
602+
error: 'error: missing argument for parameter in call',
603+
});
604+
}
605+
606+
// Second call: xcresulttool succeeds but reports 0 tests
607+
if (command.includes('xcresulttool')) {
608+
return createMockCommandResponse({
609+
success: true,
610+
output: JSON.stringify({
611+
title: 'Test Results',
612+
result: 'unknown',
613+
totalTestCount: 0,
614+
passedTests: 0,
615+
failedTests: 0,
616+
skippedTests: 0,
617+
expectedFailures: 0,
618+
}),
619+
});
620+
}
621+
622+
return createMockCommandResponse({ success: true, output: '' });
623+
};
624+
625+
const mockFileSystemExecutor = createTestFileSystemExecutor({
626+
mkdtemp: async () => '/tmp/xcodebuild-test-buildfail',
627+
});
628+
629+
const result = await testMacosLogic(
630+
{
631+
workspacePath: '/path/to/MyProject.xcworkspace',
632+
scheme: 'MyScheme',
633+
},
634+
mockExecutor,
635+
mockFileSystemExecutor,
636+
);
637+
638+
// stderr with compilation error must be preserved (not filtered)
639+
const allText = result.content.map((c) => c.text).join('\n');
640+
expect(allText).toContain('[stderr]');
641+
expect(allText).toContain('missing argument');
642+
643+
// xcresult summary should NOT be present (it's meaningless with 0 tests)
644+
expect(allText).not.toContain('Test Results Summary:');
645+
});
646+
510647
it('should return exact exception handling response', async () => {
511648
// Mock executor (won't be called due to mkdtemp failure)
512649
const mockExecutor = createMockExecutor({

0 commit comments

Comments
 (0)