Skip to content

Commit c6d1a9d

Browse files
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. Part of #231
1 parent db9d7d0 commit c6d1a9d

5 files changed

Lines changed: 190 additions & 22 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/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
});

src/utils/__tests__/build-utils.test.ts

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* Tests for build-utils Sentry classification logic
33
*/
44

5-
import { describe, it, expect } from 'vitest';
5+
import { describe, expect, it } from 'vitest';
66
import path from 'node:path';
77
import { createMockExecutor } from '../../test-utils/mock-executors.ts';
88
import { executeXcodeBuildCommand } from '../build-utils.ts';
@@ -241,6 +241,71 @@ describe('build-utils Sentry Classification', () => {
241241
});
242242
});
243243

244+
describe('Stderr Warning vs Error Classification', () => {
245+
it('should classify stderr lines containing WARNING with warning prefix', async () => {
246+
const mockExecutor = createMockExecutor({
247+
success: false,
248+
error: '--- xcodebuild: WARNING: Using the first of multiple matching destinations:',
249+
exitCode: 1,
250+
});
251+
252+
const result = await executeXcodeBuildCommand(
253+
mockParams,
254+
mockPlatformOptions,
255+
false,
256+
'build',
257+
mockExecutor,
258+
);
259+
260+
expect(result.isError).toBe(true);
261+
expect(result.content[0].text).toContain('⚠️ [stderr]');
262+
expect(result.content[0].text).toContain('WARNING');
263+
expect(result.content[0].text).not.toMatch(/^/);
264+
});
265+
266+
it('should classify stderr lines without WARNING with error prefix', async () => {
267+
const mockExecutor = createMockExecutor({
268+
success: false,
269+
error: 'Build failed with errors',
270+
exitCode: 1,
271+
});
272+
273+
const result = await executeXcodeBuildCommand(
274+
mockParams,
275+
mockPlatformOptions,
276+
false,
277+
'build',
278+
mockExecutor,
279+
);
280+
281+
expect(result.isError).toBe(true);
282+
expect(result.content[0].text).toContain('❌ [stderr]');
283+
});
284+
285+
it('should classify mixed stderr lines independently', async () => {
286+
const mockExecutor = createMockExecutor({
287+
success: false,
288+
error:
289+
'--- xcodebuild: WARNING: Using the first of multiple matching destinations:\nerror: Build input file not found',
290+
exitCode: 1,
291+
});
292+
293+
const result = await executeXcodeBuildCommand(
294+
mockParams,
295+
mockPlatformOptions,
296+
false,
297+
'build',
298+
mockExecutor,
299+
);
300+
301+
expect(result.isError).toBe(true);
302+
// First line is a warning
303+
expect(result.content[0].text).toContain('⚠️ [stderr]');
304+
// Second line is an error
305+
expect(result.content[1].text).toContain('❌ [stderr]');
306+
});
307+
});
308+
244309
describe('Exit Code Undefined Cases', () => {
245310
it('should not trigger Sentry logging when exitCode is undefined', async () => {
246311
const mockExecutor = createMockExecutor({
@@ -385,7 +450,9 @@ describe('build-utils Sentry Classification', () => {
385450
expect(capturedCommand).toBeDefined();
386451
expect(capturedCommand).toContain(expectedProjectPath);
387452
expect(capturedCommand).toContain(expectedDerivedDataPath);
388-
expect(capturedOptions).toEqual({ cwd: path.dirname(expectedProjectPath) });
453+
expect(capturedOptions).toEqual({
454+
cwd: path.dirname(expectedProjectPath),
455+
});
389456
});
390457
});
391458
});

src/utils/build-utils.ts

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,17 @@
1818
*/
1919

2020
import { log } from './logger.ts';
21-
import { XcodePlatform, constructDestinationString } from './xcode.ts';
22-
import type { CommandExecutor, CommandExecOptions } from './command.ts';
23-
import type { ToolResponse, SharedBuildParams, PlatformBuildOptions } from '../types/common.ts';
24-
import { createTextResponse, consolidateContentForClaudeCode } from './validation.ts';
21+
import { constructDestinationString, XcodePlatform } from './xcode.ts';
22+
import type { CommandExecOptions, CommandExecutor } from './command.ts';
23+
import type { PlatformBuildOptions, SharedBuildParams, ToolResponse } from '../types/common.ts';
24+
import { consolidateContentForClaudeCode, createTextResponse } from './validation.ts';
2525
import {
26-
isXcodemakeEnabled,
27-
isXcodemakeAvailable,
28-
executeXcodemakeCommand,
29-
executeMakeCommand,
3026
doesMakefileExist,
3127
doesMakeLogFileExist,
28+
executeMakeCommand,
29+
executeXcodemakeCommand,
30+
isXcodemakeAvailable,
31+
isXcodemakeEnabled,
3232
} from './xcodemake.ts';
3333
import { sessionStore } from './session-store.ts';
3434
import path from 'path';
@@ -66,10 +66,12 @@ export async function executeXcodeBuildCommand(
6666
return text
6767
.split('\n')
6868
.map((content) => {
69-
if (/(?:^(?:[\w-]+:\s+)?|:\d+:\s+)warning:\s/i.test(content))
69+
if (/(?:^(?:[\w-]+:\s+)?|:\d+:\s+)warning:\s/i.test(content)) {
7070
return { type: 'warning', content };
71-
if (/(?:^(?:[\w-]+:\s+)?|:\d+:\s+)(?:fatal )?error:\s/i.test(content))
71+
}
72+
if (/(?:^(?:[\w-]+:\s+)?|:\d+:\s+)(?:fatal )?error:\s/i.test(content)) {
7273
return { type: 'error', content };
74+
}
7375
return null;
7476
})
7577
.filter(Boolean) as { type: 'warning' | 'error'; content: string }[];
@@ -266,12 +268,14 @@ export async function executeXcodeBuildCommand(
266268
});
267269
});
268270

269-
// Include all stderr lines as errors
271+
// Include stderr lines, classifying warnings vs errors
270272
if (result.error) {
271273
result.error.split('\n').forEach((content) => {
272-
if (content.trim()) {
273-
buildMessages.push({ type: 'text', text: `❌ [stderr] ${content}` });
274-
}
274+
const trimmed = content.trim();
275+
if (!trimmed) return;
276+
const isWarning = /\bWARNING\b/i.test(trimmed);
277+
const prefix = isWarning ? '⚠️ [stderr]' : '❌ [stderr]';
278+
buildMessages.push({ type: 'text', text: `${prefix} ${content}` });
275279
});
276280
}
277281

0 commit comments

Comments
 (0)