Skip to content

feat(testing): Support reusable prepared test artifacts#475

Open
cameroncooke wants to merge 11 commits into
mainfrom
codex/feat/issue-450-build-for-testing
Open

feat(testing): Support reusable prepared test artifacts#475
cameroncooke wants to merge 11 commits into
mainfrom
codex/feat/issue-450-build-for-testing

Conversation

@cameroncooke

Copy link
Copy Markdown
Collaborator

Why

Agents and CI workflows need to prepare tests once and execute them later without falling back to raw xcodebuild. The existing build tools always finished with a normal build action, so they could not expose the reusable products produced by build-for-testing.

What Changed

  • Extend the existing simulator, device, and macOS build tools with buildForTesting and optional testProductsPath inputs.
  • Return managed .xctestproducts bundles and discovered .xctestrun files as structured build artifacts.
  • Let test tools execute either prepared .xctestproducts bundles or advanced .xctestrun inputs without rebuilding, while returning the new .xcresult produced by the run.
  • Move conditional next-step selection into tool manifests so normal builds and prepared-test builds advertise the appropriate follow-up without changing existing output behavior.
  • Add unit, schema, lifecycle, and MCP/CLI snapshot coverage for successful and failing prepared-test workflows.

Fixes #450

Extend existing build and test tools to prepare, return, and consume reusable test artifacts while preserving normal build behavior.

Fixes #450
Move conditional next-step selection into tool manifests and preserve the behavior across MCP, CLI, daemon, and snapshot outputs.

Refs #450
@pkg-pr-new

pkg-pr-new Bot commented Jul 16, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: ae6d5ba

@cameroncooke
cameroncooke marked this pull request as ready for review July 16, 2026 16:19
Comment thread src/snapshot-tests/__fixtures__/cli/text/macos/test--failure.txt
Comment thread src/utils/test-common.ts
Keep source-only xcodebuild arguments out of the generated test-without-building phase and sort final test failures independently of emission order.
Comment thread src/mcp/tools/device/build_device.ts
Comment thread src/mcp/tools/device/build_device.ts
Comment thread src/utils/test-common.ts Outdated
Comment thread src/utils/test-common.ts
Share per-test environment setup and cleanup across snapshot suites, cache suite-level simulator builds, and normalize volatile output so fixtures remain stable. Update non-environmental fixtures and add coverage for cleanup and normalization behavior.
Comment thread src/snapshot-tests/preflight/simulator.ts Fixed
Comment thread src/utils/test-source.ts
Comment thread src/utils/test-common.ts
Comment thread src/snapshot-tests/__fixtures__/cli/json/swift-package/run--success.json Outdated
Comment thread src/snapshot-tests/__fixtures__/cli/text/macos/stop--success.txt
Prevent session defaults and selectors from leaking into prepared test runs, remove a useless simulator state initializer, and align volatile snapshot fixtures across CLI and MCP.
Comment thread src/utils/test-source.ts
Comment thread src/mcp/tools/macos/build_macos.ts
Keep only-testing and skip-testing arguments in the prepared test phase while retaining session-default conflict filtering.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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 is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ae6d5ba. Configure here.

platform,
logPrefix: `${platform} Device Build`,
logPrefix: `${platform} Device ${resolved.logLabel}`,
deviceId: params.buildForTesting ? params.deviceId : undefined,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Session device blocks generic builds

Medium Severity

Adding deviceId to the build_device schema makes session deviceId merge into every buildForTesting call. A session device then forces a device-specific destination, so the supported generic build-for-testing path becomes unreachable without clearing session defaults, and portable products can be built for the wrong device.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ae6d5ba. Configure here.

"name": "Apple Watch Series 11 (46mm)",
"simulatorId": "<UUID>",
"state": "<SIM_STATE>",
"isAvailable": true,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

watchOS runtime versions are baked into the fixture instead of normalized

These runtime values retain literal watchOS versions (watchOS 27.0, watchOS 26.5, watchOS 26.4) while iOS runtimes are normalized to iOS <VERSION>, so the fixture will break whenever the installed watchOS simulators differ.

Evidence
  • json-normalize.ts line 94-96 only rewrites the runtime key when isIosRuntimeLabel(result) matches /^iOS \d+(?:\.\d+)*$/.
  • watchOS labels do not match that regex, so watchOS 27.0/26.5/26.4 pass through unnormalized.
  • The hunk adds watchOS entries with concrete version numbers while iOS entries use the iOS <VERSION> placeholder, mixing normalized and ad-hoc volatile values in the same fixture.
  • simulatorId and state are normalized to <UUID>/<SIM_STATE>, confirming volatile fields are expected to be placeholders.

Identified by Warden xcodebuildmcp-snapshot-fixture-review · YMN-XD9

Comment on lines 29 to +40
"testFailures": [
{
"suite": "CalculatorAppTests",
"test": "testCalculatorServiceFailure",
"message": "XCTAssertEqual failed: (\"0\") is not equal to (\"999\") - This test should fail - display should be 0, not 999",
"location": "<ROOT>/example_projects/spm/Tests/TestLibTests/SimpleTests.swift:49"
},
{
"suite": "(Unknown Suite)",
"test": "test",
"message": "Expectation failed: Bool(false)\nTest failed",
"location": "SimpleTests.swift:57"
},
{
"suite": "CalculatorAppTests",
"test": "testCalculatorServiceFailure",
"message": "XCTAssertEqual failed: (\"0\") is not equal to (\"999\") - This test should fail - display should be 0, not 999",
"location": "<TMPDIR>/spm/Tests/TestLibTests/SimpleTests.swift:49"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

testFailures entries reordered in fixture instead of sorted in normalizer

The two testFailures entries were swapped in this fixture, but testFailures is normalized only by normalizeDiagnosticTestFailures (a plain map) with no stable sort, unlike testCases which is sorted in code — so reordering here patches volatile ordering ad hoc and risks a flaky snapshot.

Evidence
  • In json-normalize.ts, normalizeValue handles key === 'testCases' via normalizeTestCases, which sorts with testCaseSortKey for stability.
  • The key === 'testFailures' branch calls normalizeDiagnosticTestFailures, which only items.map(normalizeDiagnosticTestFailure) and never sorts, so emitted order is preserved verbatim.
  • The hunk swaps the CalculatorAppTests/testCalculatorServiceFailure and (Unknown Suite)/test entries, matching the sorted testCases order, indicating the underlying failure order is volatile.
  • No code-level sort was added for testFailures, so the reorder is a fixture-only patch.
Also found at 2 additional locations
  • src/snapshot-tests/__fixtures__/cli/text/macos/test--failure.txt:19-28
  • src/snapshot-tests/__fixtures__/mcp/json/macos/test--failure.json:36-48

Identified by Warden xcodebuildmcp-snapshot-fixture-review · KAK-HHB

Comment on lines +14 to +15
"x": 61,
"y": 490

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unexplained long-press coordinate change unrelated to prepared-test feature

This UI-automation long-press fixture's x/y coordinates change (201,437 → 61,490) but the PR only adds prepared-test build/test artifacts; confirm this reflects an intentional behavior change and is not a coordinate drift patched to make the snapshot pass.

Evidence
  • PR scope is buildForTesting/testProductsPath build+test artifacts, no UI-automation behavior changes described.
  • normalize.ts normalizes elementRef, UUIDs, hashes, etc., but has no rule for raw x/y action coordinates, so these values are hardcoded into the fixture rather than normalized in code.
  • The changed x/y are the only diff in this file and have no connection to prepared-test artifacts, suggesting an ad-hoc value update.
Also found at 1 additional location
  • src/snapshot-tests/__fixtures__/cli/json/ui-automation/touch--success.json:14-15

Identified by Warden xcodebuildmcp-snapshot-fixture-review · THM-PL3

"<REF>|tap|button|Dictate||Dictate",
"<REF>|tap|button|Dictate||Dictate"
],
"count": 99999,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sibling UI-automation capture fixtures store literal count 28 that cannot match the unconditional count→99999 normalizer

The swipe--success.json capture fixture records the normalized 99999 count placeholder (line 31), but eight other UI-automation capture fixtures (tap, gesture, key-press, long-press, touch, wait-for-ui, key-sequence, snapshot-ui) still record literal count: 28. normalizeNumber in json-normalize.ts forces capture.count to 99999 unconditionally, and snapshot comparison is an exact string match against the raw stored fixture. A stored count: 28 therefore cannot be the output of the current normalizer, so those sibling fixtures are stale/misaligned — they were not regenerated when the count-normalization rule was applied (as swipe was), meaning npm run test:snapshots will not stay aligned for those scenarios.

Evidence
  • normalizeNumber (json-normalize.ts:109-111) returns 99999 whenever key === 'count' && path.includes('capture'), with no other condition.
  • json-normalize.test.ts confirms this: capture counts of 240 and 7 both normalize to 99999.
  • expectMatchesFixture in fixture-io.ts compares actual !== expected against the raw fixture file (no re-normalization on read), so the stored value must already equal the normalizer output.
  • swipe--success.json:31 stores "count": 99999 (matching the rule), but grep under __fixtures__/cli/json/ui-automation shows tap, gesture, key-press, long-press, touch, wait-for-ui, key-sequence, and snapshot-ui all still store "count": 28, which the current normalizer would never emit.

Identified by Warden xcodebuildmcp-snapshot-fixture-review · Q6L-SQB

Comment on lines +611 to 617
"label": "0"
},
{
"action": "touch",
"elementRef": "e8",
"label": "7"
"label": "0"
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

UI-automation fixture label change unrelated to prepared-test-artifacts feature

This tap--success-verbose.json label flip from "7" to "0" is unrelated to the PR's prepared-test-artifacts changes and looks like a fixture patched to match new output rather than an intentional, in-scope behavior change.

Evidence
  • PR scope is buildForTesting/testProductsPath build and test workflows; nothing touches UI-automation tap output.
  • The hunk changes accessibility label values for elementRef "e8" from "7" to "0" on both longPress and touch actions.
  • Surrounding elements (e10 "C", e11 "±", e12 "%") keep stable labels, so this is a targeted value change, not a structural normalization.
  • No corresponding code normalization for these labels is present, so the changed value is baked directly into the fixture.

Identified by Warden xcodebuildmcp-snapshot-fixture-review · WJX-V2J

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.

Add explicit build-for-testing action

1 participant