Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixed

- Fixed `swift_package_build`, `swift_package_test`, and `swift_package_clean` swallowing compiler diagnostics on failure by treating empty stderr as falsy, so stdout diagnostics are included in the error response ([#243](https://github.com/getsentry/XcodeBuildMCP/issues/243)).
- 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))

## [2.1.0]

Expand Down
85 changes: 75 additions & 10 deletions src/mcp/tools/device/__tests__/test_device.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@ describe('test_device plugin', () => {
);

expect(result.content).toHaveLength(2);
expect(result.content[0].text).toContain('');
expect(result.content[1].text).toContain('Test Results Summary:');
expect(result.content[1].text).toContain('MyScheme Tests');
expect(result.content[0].text).toContain('Test Results Summary:');
expect(result.content[0].text).toContain('MyScheme Tests');
expect(result.content[1].text).toContain('');
});

it('should handle test failure scenarios', async () => {
Expand Down Expand Up @@ -214,8 +214,8 @@ describe('test_device plugin', () => {
);

expect(result.content).toHaveLength(2);
expect(result.content[1].text).toContain('Test Failures:');
expect(result.content[1].text).toContain('testExample');
expect(result.content[0].text).toContain('Test Failures:');
expect(result.content[0].text).toContain('testExample');
});

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

it('should preserve stderr when xcresult reports zero tests (build failure)', async () => {
// When the build fails, xcresult exists but has totalTestCount: 0.
// stderr contains the actual compilation errors and must be preserved.
let callCount = 0;
const mockExecutor = async (
_args: string[],
_description?: string,
_useShell?: boolean,
_opts?: { cwd?: string },
_detached?: boolean,
) => {
callCount++;

// First call: xcodebuild test fails with compilation error
if (callCount === 1) {
return createMockCommandResponse({
success: false,
output: '',
error: 'error: missing argument for parameter in call',
});
}

// Second call: xcresulttool succeeds but reports 0 tests
return createMockCommandResponse({
success: true,
output: JSON.stringify({
title: 'Test Results',
result: 'unknown',
totalTestCount: 0,
passedTests: 0,
failedTests: 0,
skippedTests: 0,
expectedFailures: 0,
}),
});
};

const result = await testDeviceLogic(
{
projectPath: '/path/to/project.xcodeproj',
scheme: 'MyScheme',
deviceId: 'test-device-123',
configuration: 'Debug',
preferXcodebuild: false,
platform: 'iOS',
},
mockExecutor,
createMockFileSystemExecutor({
mkdtemp: async () => '/tmp/xcodebuild-test-buildfail',
tmpdir: () => '/tmp',
stat: async () => ({ isDirectory: () => false, mtimeMs: 0 }),
rm: async () => {},
}),
);

// stderr with compilation error must be preserved
const allText = result.content.map((c) => c.text).join('\n');
expect(allText).toContain('[stderr]');
expect(allText).toContain('missing argument');

// xcresult summary should NOT be present
expect(allText).not.toContain('Test Results Summary:');
});

it('should support different platforms', async () => {
// Mock xcresulttool output
const mockExecutor = createMockExecutor({
Expand Down Expand Up @@ -298,7 +362,7 @@ describe('test_device plugin', () => {
);

expect(result.content).toHaveLength(2);
expect(result.content[1].text).toContain('WatchApp Tests');
expect(result.content[0].text).toContain('WatchApp Tests');
});

it('should handle optional parameters', async () => {
Expand Down Expand Up @@ -337,7 +401,8 @@ describe('test_device plugin', () => {
);

expect(result.content).toHaveLength(2);
expect(result.content[0].text).toContain('✅');
expect(result.content[0].text).toContain('Test Results Summary:');
expect(result.content[1].text).toContain('✅');
});

it('should handle workspace testing successfully', async () => {
Expand Down Expand Up @@ -374,9 +439,9 @@ describe('test_device plugin', () => {
);

expect(result.content).toHaveLength(2);
expect(result.content[0].text).toContain('');
expect(result.content[1].text).toContain('Test Results Summary:');
expect(result.content[1].text).toContain('WorkspaceScheme Tests');
expect(result.content[0].text).toContain('Test Results Summary:');
expect(result.content[0].text).toContain('WorkspaceScheme Tests');
expect(result.content[1].text).toContain('');
});
});
});
30 changes: 24 additions & 6 deletions src/mcp/tools/device/test_device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
getSessionAwareToolSchemaShape,
} from '../../../utils/typed-tool-factory.ts';
import { nullifyEmptyStrings } from '../../../utils/schema-helpers.ts';
import { filterStderrContent } from '../../../utils/test-result-content.ts';

// Unified schema: XOR between projectPath and workspacePath
const baseSchemaObject = z.object({
Expand Down Expand Up @@ -76,13 +77,18 @@ const publicSchemaObject = baseSchemaObject.omit({
* (JavaScript implementation - no actual interface, this is just documentation)
*/

interface XcresultSummary {
formatted: string;
totalTestCount: number;
}

/**
* Parse xcresult bundle using xcrun xcresulttool
*/
async function parseXcresultBundle(
resultBundlePath: string,
executor: CommandExecutor = getDefaultCommandExecutor(),
): Promise<string> {
): Promise<XcresultSummary> {
try {
// Use injected executor for testing
const result = await executor(
Expand All @@ -98,7 +104,11 @@ async function parseXcresultBundle(

// Parse JSON response and format as human-readable
const summaryData = JSON.parse(result.output) as Record<string, unknown>;
return formatTestSummary(summaryData);
return {
formatted: formatTestSummary(summaryData),
totalTestCount:
typeof summaryData.totalTestCount === 'number' ? summaryData.totalTestCount : 0,
};
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error);
log('error', `Error parsing xcresult bundle: ${errorMessage}`);
Expand Down Expand Up @@ -252,20 +262,28 @@ export async function testDeviceLogic(
throw new Error(`xcresult bundle not found at ${resultBundlePath}`);
}

const testSummary = await parseXcresultBundle(resultBundlePath, executor);
const xcresult = await parseXcresultBundle(resultBundlePath, executor);
log('info', 'Successfully parsed xcresult bundle');

// Clean up temporary directory
await cleanup();

// Return combined result - preserve isError from testResult (test failures should be marked as errors)
// If no tests ran (for example build/setup failed), xcresult summary is not useful.
// Return raw output so the original diagnostics stay visible.
if (xcresult.totalTestCount === 0) {
log('info', 'xcresult reports 0 tests — returning raw build output');
return testResult;
}

// xcresult summary should be first. Drop stderr-only noise while preserving non-stderr lines.
const filteredContent = filterStderrContent(testResult.content);
return {
content: [
...(testResult.content || []),
{
type: 'text',
text: '\nTest Results Summary:\n' + testSummary,
text: '\nTest Results Summary:\n' + xcresult.formatted,
},
...filteredContent,
],
isError: testResult.isError,
};
Expand Down
137 changes: 137 additions & 0 deletions src/mcp/tools/macos/__tests__/test_macos.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,143 @@ describe('test_macos plugin (unified)', () => {
);
});

it('should filter out stderr lines when xcresult data is available', async () => {
// Regression test for #231: stderr warnings (e.g. "multiple matching destinations")
// should be dropped when xcresult parsing succeeds, since xcresult is authoritative.
let callCount = 0;
const mockExecutor = async (
command: string[],
logPrefix?: string,
useShell?: boolean,
opts?: { env?: Record<string, string> },
detached?: boolean,
) => {
callCount++;
void logPrefix;
void useShell;
void opts;
void detached;

// First call: xcodebuild test fails with stderr warning
if (callCount === 1) {
return createMockCommandResponse({
success: false,
output: '',
error:
'WARNING: multiple matching destinations, using first match\n' + 'error: Test failed',
});
}

// Second call: xcresulttool succeeds
if (command.includes('xcresulttool')) {
return createMockCommandResponse({
success: true,
output: JSON.stringify({
title: 'Test Results',
result: 'FAILED',
totalTestCount: 5,
passedTests: 3,
failedTests: 2,
skippedTests: 0,
expectedFailures: 0,
}),
});
}

return createMockCommandResponse({ success: true, output: '' });
};

const mockFileSystemExecutor = createTestFileSystemExecutor({
mkdtemp: async () => '/tmp/xcodebuild-test-stderr',
});

const result = await testMacosLogic(
{
workspacePath: '/path/to/MyProject.xcworkspace',
scheme: 'MyScheme',
},
mockExecutor,
mockFileSystemExecutor,
);

// stderr lines should be filtered out
const allText = result.content.map((c) => c.text).join('\n');
expect(allText).not.toContain('[stderr]');

// xcresult summary should be present and first
expect(result.content[0].text).toContain('Test Results Summary:');

// Build status line should still be present
expect(allText).toContain('Test Run test failed for scheme MyScheme');
});

it('should preserve stderr when xcresult reports zero tests (build failure)', async () => {
// When the build fails, xcresult exists but has totalTestCount: 0.
// In that case stderr contains the actual compilation errors and must be preserved.
let callCount = 0;
const mockExecutor = async (
command: string[],
logPrefix?: string,
useShell?: boolean,
opts?: { env?: Record<string, string> },
detached?: boolean,
) => {
callCount++;
void logPrefix;
void useShell;
void opts;
void detached;

// First call: xcodebuild test fails with compilation error on stderr
if (callCount === 1) {
return createMockCommandResponse({
success: false,
output: '',
error: 'error: missing argument for parameter in call',
});
}

// Second call: xcresulttool succeeds but reports 0 tests
if (command.includes('xcresulttool')) {
return createMockCommandResponse({
success: true,
output: JSON.stringify({
title: 'Test Results',
result: 'unknown',
totalTestCount: 0,
passedTests: 0,
failedTests: 0,
skippedTests: 0,
expectedFailures: 0,
}),
});
}

return createMockCommandResponse({ success: true, output: '' });
};

const mockFileSystemExecutor = createTestFileSystemExecutor({
mkdtemp: async () => '/tmp/xcodebuild-test-buildfail',
});

const result = await testMacosLogic(
{
workspacePath: '/path/to/MyProject.xcworkspace',
scheme: 'MyScheme',
},
mockExecutor,
mockFileSystemExecutor,
);

// stderr with compilation error must be preserved (not filtered)
const allText = result.content.map((c) => c.text).join('\n');
expect(allText).toContain('[stderr]');
expect(allText).toContain('missing argument');

// xcresult summary should NOT be present (it's meaningless with 0 tests)
expect(allText).not.toContain('Test Results Summary:');
});

it('should return exact exception handling response', async () => {
// Mock executor (won't be called due to mkdtemp failure)
const mockExecutor = createMockExecutor({
Expand Down
Loading
Loading