Skip to content

Commit ecf2288

Browse files
czottmannczottmann
committed
fix: classify stderr warnings correctly and prioritize xcresult output
xcodebuild dumps warnings (e.g. "multiple matching destinations") to stderr. These were indiscriminately tagged as errors, hiding actual test failure details from xcresult parsing. Now stderr lines containing WARNING get a warning prefix instead of error, and xcresult test summaries appear before raw stderr output in the response. Applied consistently to test_macos, test_device, and test-common. Part of #231
1 parent db9d7d0 commit ecf2288

8 files changed

Lines changed: 244 additions & 49 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") being reported as errors, hiding actual test failures from xcresult output ([#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: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@
55
* NO VITEST MOCKING ALLOWED - Only createMockExecutor and manual stubs
66
*/
77

8-
import { describe, it, expect, beforeEach } from 'vitest';
8+
import { beforeEach, describe, expect, it } from 'vitest';
99
import * as z from 'zod';
1010
import {
1111
createMockCommandResponse,
1212
createMockExecutor,
1313
createMockFileSystemExecutor,
1414
} from '../../../../test-utils/mock-executors.ts';
15-
import { schema, handler, testDeviceLogic } from '../test_device.ts';
15+
import { handler, schema, testDeviceLogic } from '../test_device.ts';
1616
import { sessionStore } from '../../../../utils/session-store.ts';
1717

1818
describe('test_device plugin', () => {
@@ -107,7 +107,10 @@ describe('test_device plugin', () => {
107107
});
108108

109109
it('should require project or workspace when defaults provide scheme and device', async () => {
110-
sessionStore.setDefaults({ scheme: 'MyScheme', deviceId: 'test-device-123' });
110+
sessionStore.setDefaults({
111+
scheme: 'MyScheme',
112+
deviceId: 'test-device-123',
113+
});
111114

112115
const result = await handler({});
113116

@@ -116,7 +119,10 @@ describe('test_device plugin', () => {
116119
});
117120

118121
it('should reject mutually exclusive project inputs when defaults satisfy requirements', async () => {
119-
sessionStore.setDefaults({ scheme: 'MyScheme', deviceId: 'test-device-123' });
122+
sessionStore.setDefaults({
123+
scheme: 'MyScheme',
124+
deviceId: 'test-device-123',
125+
});
120126

121127
const result = await handler({
122128
projectPath: '/path/to/project.xcodeproj',
@@ -168,9 +174,9 @@ describe('test_device plugin', () => {
168174
);
169175

170176
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');
177+
expect(result.content[0].text).toContain('Test Results Summary:');
178+
expect(result.content[0].text).toContain('MyScheme Tests');
179+
expect(result.content[1].text).toContain('✅');
174180
});
175181

176182
it('should handle test failure scenarios', async () => {
@@ -214,8 +220,8 @@ describe('test_device plugin', () => {
214220
);
215221

216222
expect(result.content).toHaveLength(2);
217-
expect(result.content[1].text).toContain('Test Failures:');
218-
expect(result.content[1].text).toContain('testExample');
223+
expect(result.content[0].text).toContain('Test Failures:');
224+
expect(result.content[0].text).toContain('testExample');
219225
});
220226

221227
it('should handle xcresult parsing failures gracefully', async () => {
@@ -232,11 +238,17 @@ describe('test_device plugin', () => {
232238

233239
// First call is for xcodebuild test (successful)
234240
if (callCount === 1) {
235-
return createMockCommandResponse({ success: true, output: 'BUILD SUCCEEDED' });
241+
return createMockCommandResponse({
242+
success: true,
243+
output: 'BUILD SUCCEEDED',
244+
});
236245
}
237246

238247
// Second call is for xcresulttool (fails)
239-
return createMockCommandResponse({ success: false, error: 'xcresulttool failed' });
248+
return createMockCommandResponse({
249+
success: false,
250+
error: 'xcresulttool failed',
251+
});
240252
};
241253

242254
const result = await testDeviceLogic(
@@ -298,7 +310,7 @@ describe('test_device plugin', () => {
298310
);
299311

300312
expect(result.content).toHaveLength(2);
301-
expect(result.content[1].text).toContain('WatchApp Tests');
313+
expect(result.content[0].text).toContain('WatchApp Tests');
302314
});
303315

304316
it('should handle optional parameters', async () => {
@@ -337,7 +349,7 @@ describe('test_device plugin', () => {
337349
);
338350

339351
expect(result.content).toHaveLength(2);
340-
expect(result.content[0].text).toContain('✅');
352+
expect(result.content[1].text).toContain('✅');
341353
});
342354

343355
it('should handle workspace testing successfully', async () => {
@@ -374,9 +386,9 @@ describe('test_device plugin', () => {
374386
);
375387

376388
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');
389+
expect(result.content[0].text).toContain('Test Results Summary:');
390+
expect(result.content[0].text).toContain('WorkspaceScheme Tests');
391+
expect(result.content[1].text).toContain('✅');
380392
});
381393
});
382394
});

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ import { executeXcodeBuildCommand } from '../../../utils/build/index.ts';
1414
import { createTextResponse } from '../../../utils/responses/index.ts';
1515
import { normalizeTestRunnerEnv } from '../../../utils/environment.ts';
1616
import type {
17+
CommandExecOptions,
1718
CommandExecutor,
1819
FileSystemExecutor,
19-
CommandExecOptions,
2020
} from '../../../utils/execution/index.ts';
2121
import {
2222
getDefaultCommandExecutor,
@@ -138,7 +138,9 @@ function formatTestSummary(summary: Record<string, unknown>): string {
138138
const device = deviceConfig.device as Record<string, unknown> | undefined;
139139
if (device) {
140140
lines.push(
141-
`Device: ${device.deviceName ?? 'Unknown'} (${device.platform ?? 'Unknown'} ${device.osVersion ?? 'Unknown'})`,
141+
`Device: ${device.deviceName ?? 'Unknown'} (${
142+
device.platform ?? 'Unknown'
143+
} ${device.osVersion ?? 'Unknown'})`,
142144
);
143145
lines.push('');
144146
}
@@ -153,7 +155,9 @@ function formatTestSummary(summary: Record<string, unknown>): string {
153155
summary.testFailures.forEach((failureItem, index) => {
154156
const failure = failureItem as Record<string, unknown>;
155157
lines.push(
156-
` ${index + 1}. ${failure.testName ?? 'Unknown Test'} (${failure.targetName ?? 'Unknown Target'})`,
158+
` ${index + 1}. ${failure.testName ?? 'Unknown Test'} (${
159+
failure.targetName ?? 'Unknown Target'
160+
})`,
157161
);
158162
if (failure.failureText) {
159163
lines.push(` ${failure.failureText}`);
@@ -186,7 +190,9 @@ export async function testDeviceLogic(
186190
): Promise<ToolResponse> {
187191
log(
188192
'info',
189-
`Starting test run for scheme ${params.scheme} on platform ${params.platform ?? 'iOS'} (internal)`,
193+
`Starting test run for scheme ${params.scheme} on platform ${
194+
params.platform ?? 'iOS'
195+
} (internal)`,
190196
);
191197

192198
let tempDir: string | undefined;
@@ -259,13 +265,14 @@ export async function testDeviceLogic(
259265
await cleanup();
260266

261267
// Return combined result - preserve isError from testResult (test failures should be marked as errors)
268+
// Prioritize xcresult summary over raw stderr output so test failures are visible first
262269
return {
263270
content: [
264-
...(testResult.content || []),
265271
{
266272
type: 'text',
267273
text: '\nTest Results Summary:\n' + testSummary,
268274
},
275+
...(testResult.content || []),
269276
],
270277
isError: testResult.isError,
271278
};
@@ -305,7 +312,10 @@ export const handler = createSessionAwareTool<TestDeviceParams>({
305312
getExecutor: getDefaultCommandExecutor,
306313
requirements: [
307314
{ allOf: ['scheme', 'deviceId'], message: 'Provide scheme and deviceId' },
308-
{ oneOf: ['projectPath', 'workspacePath'], message: 'Provide a project or workspace' },
315+
{
316+
oneOf: ['projectPath', 'workspacePath'],
317+
message: 'Provide a project or workspace',
318+
},
309319
],
310320
exclusivePairs: [['projectPath', 'workspacePath']],
311321
});

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

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Following CLAUDE.md testing standards with literal validation
44
* Using dependency injection for deterministic testing
55
*/
6-
import { describe, it, expect, beforeEach } from 'vitest';
6+
import { beforeEach, describe, expect, it } from 'vitest';
77
import * as z from 'zod';
88
import {
99
createMockCommandResponse,
@@ -12,7 +12,7 @@ import {
1212
type FileSystemExecutor,
1313
} from '../../../../test-utils/mock-executors.ts';
1414
import { sessionStore } from '../../../../utils/session-store.ts';
15-
import { schema, handler } from '../test_macos.ts';
15+
import { handler, schema } from '../test_macos.ts';
1616
import { testMacosLogic } from '../test_macos.ts';
1717

1818
const createTestFileSystemExecutor = (overrides: Partial<FileSystemExecutor> = {}) =>
@@ -412,7 +412,11 @@ describe('test_macos plugin (unified)', () => {
412412
});
413413
}
414414

415-
return createMockCommandResponse({ success: true, output: '', error: undefined });
415+
return createMockCommandResponse({
416+
success: true,
417+
output: '',
418+
error: undefined,
419+
});
416420
};
417421

418422
// Mock file system dependencies
@@ -440,6 +444,89 @@ describe('test_macos plugin (unified)', () => {
440444
expect(result.isError).toBe(true);
441445
});
442446

447+
it('should place xcresult summary before build output on test failure', async () => {
448+
let callCount = 0;
449+
const mockExecutor = async (
450+
command: string[],
451+
logPrefix?: string,
452+
useShell?: boolean,
453+
opts?: { env?: Record<string, string> },
454+
detached?: boolean,
455+
) => {
456+
callCount++;
457+
void logPrefix;
458+
void useShell;
459+
void opts;
460+
void detached;
461+
462+
// First call is xcodebuild test - fails with a WARNING in stderr
463+
if (callCount === 1) {
464+
return createMockCommandResponse({
465+
success: false,
466+
output: '',
467+
error:
468+
'--- xcodebuild: WARNING: Using the first of multiple matching destinations:\n** TEST FAILED **',
469+
});
470+
}
471+
472+
// Second call is xcresulttool - returns failure details
473+
if (command.includes('xcresulttool')) {
474+
return createMockCommandResponse({
475+
success: true,
476+
output: JSON.stringify({
477+
title: 'Test Results',
478+
result: 'FAILED',
479+
totalTestCount: 3,
480+
passedTests: 1,
481+
failedTests: 2,
482+
skippedTests: 0,
483+
expectedFailures: 0,
484+
testFailures: [
485+
{
486+
testName: 'testFoo',
487+
targetName: 'MyTests',
488+
failureText: 'XCTAssertEqual failed',
489+
},
490+
],
491+
}),
492+
error: undefined,
493+
});
494+
}
495+
496+
return createMockCommandResponse({
497+
success: true,
498+
output: '',
499+
error: undefined,
500+
});
501+
};
502+
503+
const mockFileSystemExecutor = createTestFileSystemExecutor({
504+
mkdtemp: async () => '/tmp/xcodebuild-test-abc123',
505+
});
506+
507+
const result = await testMacosLogic(
508+
{
509+
workspacePath: '/path/to/MyProject.xcworkspace',
510+
scheme: 'MyScheme',
511+
},
512+
mockExecutor,
513+
mockFileSystemExecutor,
514+
);
515+
516+
expect(result.isError).toBe(true);
517+
518+
// xcresult summary must appear first
519+
const firstContent = result.content[0].text;
520+
expect(firstContent).toContain('Test Results Summary');
521+
expect(firstContent).toContain('Failed: 2');
522+
523+
// Build output should come after
524+
const allText = result.content.map((c) => c.text).join('\n');
525+
const summaryIndex = allText.indexOf('Test Results Summary');
526+
const stderrIndex = allText.indexOf('[stderr]');
527+
expect(summaryIndex).toBeLessThan(stderrIndex);
528+
});
529+
443530
it('should return exact successful test response with optional parameters', async () => {
444531
// Track command execution calls
445532
const commandCalls: any[] = [];

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ import { executeXcodeBuildCommand } from '../../../utils/build/index.ts';
1414
import { createTextResponse } from '../../../utils/responses/index.ts';
1515
import { normalizeTestRunnerEnv } from '../../../utils/environment.ts';
1616
import type {
17+
CommandExecOptions,
1718
CommandExecutor,
1819
FileSystemExecutor,
19-
CommandExecOptions,
2020
} from '../../../utils/execution/index.ts';
2121
import {
2222
getDefaultCommandExecutor,
@@ -294,13 +294,14 @@ export async function testMacosLogic(
294294
await fileSystemExecutor.rm(tempDir, { recursive: true, force: true });
295295

296296
// Return combined result - preserve isError from testResult (test failures should be marked as errors)
297+
// Prioritize xcresult summary over raw stderr output so test failures are visible first
297298
return {
298299
content: [
299-
...(testResult.content ?? []),
300300
{
301301
type: 'text',
302302
text: '\nTest Results Summary:\n' + testSummary,
303303
},
304+
...(testResult.content ?? []),
304305
],
305306
isError: testResult.isError,
306307
};
@@ -336,7 +337,10 @@ export const handler = createSessionAwareTool<TestMacosParams>({
336337
getExecutor: getDefaultCommandExecutor,
337338
requirements: [
338339
{ allOf: ['scheme'], message: 'scheme is required' },
339-
{ oneOf: ['projectPath', 'workspacePath'], message: 'Provide a project or workspace' },
340+
{
341+
oneOf: ['projectPath', 'workspacePath'],
342+
message: 'Provide a project or workspace',
343+
},
340344
],
341345
exclusivePairs: [['projectPath', 'workspacePath']],
342346
});

0 commit comments

Comments
 (0)