Skip to content

feat(test): Expose xcresult bundle paths#397

Merged
cameroncooke merged 4 commits intomainfrom
xcresult
May 6, 2026
Merged

feat(test): Expose xcresult bundle paths#397
cameroncooke merged 4 commits intomainfrom
xcresult

Conversation

@cameroncooke
Copy link
Copy Markdown
Collaborator

Expose xcresult bundle paths in test results so clients can jump from MCP structured output or text output to the generated result bundle for deeper analysis.

The test tools now carry a real .xcresult path when xcodebuild reports one or when callers pass -resultBundlePath. This updates the shared test-result schema, domain result artifacts, parsers, renderers, and normalization/tests to keep JSON and text output aligned.

swift_package_test intentionally does not synthesize an xcresult path because SwiftPM test output does not produce the same xcodebuild result bundle signal.

Fixes #392

Include xcresult bundle paths in test result artifacts when xcodebuild
reports or receives a result bundle path. This lets MCP clients open the
result bundle directly from structured output and text renderers.

Fixes #392
Co-Authored-By: Codex <noreply@openai.com>
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 6, 2026

Open in StackBlitz

npm i https://pkg.pr.new/xcodebuildmcp@397

commit: 66608a0

Comment thread src/utils/xcodebuild-domain-results.ts
Comment thread src/utils/xcresult-test-failures.ts
@cameroncooke cameroncooke marked this pull request as ready for review May 6, 2026 19:16
Return null when xcresult summary parsing receives malformed JSON so the
exported parser matches its nullable contract and callers can safely fall
back to streamed counts.

Refs #397
Co-Authored-By: Codex <noreply@openai.com>

if (succeeded) {
return `${statusEmoji} ${pluralize(passed, 'test', 'tests')} passed, ${skipped} skipped${durationPart}`;
return `${statusEmoji} ${pluralize(passed, 'test', 'tests')} passed, ${failed} failed, ${skipped} skipped${durationPart}`;
Copy link
Copy Markdown

@sentry-warden sentry-warden Bot May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Success summary reports non-zero failed count when status is SUCCEEDED

When event.status === 'SUCCEEDED', the branch now includes ${failed} failed in the summary line. If the status is SUCCEEDED, failed should be 0, but if the upstream data is ever inconsistent (e.g., parser sets SUCCEEDED while failedTests > 0), this will produce a confusing message like '✅ 5 tests passed, 2 failed, 0 skipped'. More importantly, even in the normal case, this always emits '0 failed' on success which is noisy and contradicts the success emoji. The user-visible consequence is misleading or noisy summary output on successful test runs.

Verification

Read the surrounding formatSummaryEvent function in the hunk and context. The succeeded branch is only entered when event.status === 'SUCCEEDED'. The change unconditionally injects the failed count into the success message. I did not find evidence in the provided context that callers guarantee failed===0 when SUCCEEDED, but even if they do, '0 failed' is always rendered.

Identified by Warden find-bugs, code-review · 2PT-YAZ

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this unchanged. The successful summary intentionally includes the full count tuple (passed, failed, skipped) and the updated renderer tests assert this output. If upstream data ever contradicts the success status, surfacing the count is preferable to hiding it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix attempt detected (commit 66608a0)

The commit changed line 514 to add '${failed} failed' to the success message, which directly worsens the reported issue by injecting the failed count into SUCCEEDED status summaries, producing noisy output like '✅ 5 tests passed, 0 failed, 0 skipped'.

The original issue appears unresolved. Please review and try again.

Evaluated by Warden

Comment thread src/snapshot-tests/__tests__/json-normalize.test.ts
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Duplicated isResultBundlePathValue helper across two files
    • Extracted isResultBundlePathValue and findResultBundlePathArg to a new shared utility file src/utils/result-bundle-path.ts, eliminating duplication between test-common.ts and simulator-test-execution.ts.

Create PR

Or push these changes by commenting:

@cursor push 5d543b503d
Preview (5d543b503d)
diff --git a/src/utils/result-bundle-path.ts b/src/utils/result-bundle-path.ts
new file mode 100644
--- /dev/null
+++ b/src/utils/result-bundle-path.ts
@@ -1,0 +1,34 @@
+/**
+ * Shared utilities for parsing and validating -resultBundlePath arguments
+ */
+
+export function isResultBundlePathValue(value: string | undefined): value is string {
+  return value !== undefined && value.length > 0 && !value.startsWith('-');
+}
+
+export function findResultBundlePathArg(extraArgs?: readonly string[]): string | undefined {
+  if (!extraArgs) {
+    return undefined;
+  }
+
+  let resultBundlePath: string | undefined;
+  for (let index = 0; index < extraArgs.length; index += 1) {
+    const argument = extraArgs[index];
+    if (argument === '-resultBundlePath') {
+      const value = extraArgs[index + 1];
+      if (isResultBundlePathValue(value)) {
+        resultBundlePath = value;
+        index += 1;
+      }
+      continue;
+    }
+    if (argument?.startsWith('-resultBundlePath=')) {
+      const value = argument.slice('-resultBundlePath='.length);
+      if (isResultBundlePathValue(value)) {
+        resultBundlePath = value;
+      }
+    }
+  }
+
+  return resultBundlePath;
+}

diff --git a/src/utils/simulator-test-execution.ts b/src/utils/simulator-test-execution.ts
--- a/src/utils/simulator-test-execution.ts
+++ b/src/utils/simulator-test-execution.ts
@@ -1,9 +1,6 @@
 import type { TestPreflightResult } from './test-preflight.ts';
+import { isResultBundlePathValue } from './result-bundle-path.ts';
 
-function isResultBundlePathValue(value: string | undefined): value is string {
-  return value !== undefined && value.length > 0 && !value.startsWith('-');
-}
-
 function parseTestSelectorArgs(extraArgs: string[] | undefined): {
   remainingArgs: string[];
   selectorArgs: string[];

diff --git a/src/utils/test-common.ts b/src/utils/test-common.ts
--- a/src/utils/test-common.ts
+++ b/src/utils/test-common.ts
@@ -16,6 +16,7 @@
 import { type TestPreflightResult } from './test-preflight.ts';
 
 import { createSimulatorTwoPhaseExecutionPlan } from './simulator-test-execution.ts';
+import { findResultBundlePathArg } from './result-bundle-path.ts';
 
 import type {
   BuildTarget,
@@ -59,37 +60,6 @@
   return [...streamedLines, ...(responseContent ?? []).map((item) => item.text)];
 }
 
-function isResultBundlePathValue(value: string | undefined): value is string {
-  return value !== undefined && value.length > 0 && !value.startsWith('-');
-}
-
-function findResultBundlePathArg(extraArgs?: readonly string[]): string | undefined {
-  if (!extraArgs) {
-    return undefined;
-  }
-
-  let resultBundlePath: string | undefined;
-  for (let index = 0; index < extraArgs.length; index += 1) {
-    const argument = extraArgs[index];
-    if (argument === '-resultBundlePath') {
-      const value = extraArgs[index + 1];
-      if (isResultBundlePathValue(value)) {
-        resultBundlePath = value;
-        index += 1;
-      }
-      continue;
-    }
-    if (argument?.startsWith('-resultBundlePath=')) {
-      const value = argument.slice('-resultBundlePath='.length);
-      if (isResultBundlePathValue(value)) {
-        resultBundlePath = value;
-      }
-    }
-  }
-
-  return resultBundlePath;
-}
-
 function createXcodebuildTestArtifacts(
   params: Pick<SharedTestExecutorParams, 'deviceId'>,
   started: ReturnType<typeof createDomainStreamingPipeline>,

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 899b5b7. Configure here.

Comment thread src/utils/test-common.ts Outdated
Move result bundle path parsing into a shared helper so single-phase and
two-phase test execution paths use the same validation and precedence rules.

Refs #397
Co-Authored-By: Codex <noreply@openai.com>
@cameroncooke cameroncooke merged commit 1ae1867 into main May 6, 2026
47 checks passed
@cameroncooke cameroncooke deleted the xcresult branch May 6, 2026 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Expose xcresult bundle path for further analysis

1 participant