Share one build and one simulator across fixture generation - #425
Merged
Conversation
prepareTestResults.sh ran three `xcodebuild test` invocations, each paying a full build and its own simulator boot. Per-invocation timing extracted from CI logs (#412) showed the two single-purpose invocations cost 43-45% of fixture generation while producing one test and one retry suite between them -- a ratio that held across three runs on runners differing 1.8x in speed. Boot the simulator once, `build-for-testing` once, then run each fixture with `test-without-building` against the shared products. Also: - Pin the destination by UDID instead of name+OS. Unambiguous when several runtimes offer the same device name, and it guarantees the simulator booted here is the one xcodebuild uses. - Drop `|| true` from the build. Tests are allowed to fail; a build failure is not. Previously a broken sample app surfaced as a confusing `mv` error. Verified locally: script exits 0, all three bundles produced, and `swift test` passes 23 tests / 1 skipped / 0 failures against them. Whether this is faster on CI is measured separately -- local timing is not evidence for this repository. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cross-run wall clock cannot measure a change here: identical code has varied 1.8x between runners. This job runs both generators on the same runner, back to back, counterbalanced (old, new, new, old), resetting DerivedData and shutting down simulators between variants so neither benefits from the other's warmth. It also records how many .xcresult bundles each variant produced -- a run that generated nothing is not a fast run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe test preparation script targets a resolved simulator by UDID, builds test artifacts once into stable derived data, and reuses them for functional, sanity, and retry fixtures. The sample app’s derived-data directory is ignored. ChangesTest preparation workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant prepareTestResults.sh
participant Simulator
participant xcodebuild
prepareTestResults.sh->>Simulator: Resolve and boot simulator by UDID
prepareTestResults.sh->>xcodebuild: Build test artifacts once
prepareTestResults.sh->>xcodebuild: Run functional fixture without building
prepareTestResults.sh->>xcodebuild: Run sanity fixture without building
prepareTestResults.sh->>xcodebuild: Run retry fixture without building
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
It has served its purpose. Result, same runner, counterbalanced:
run1 old 498s 3 bundles
run2 new 308s 3 bundles
run3 new 296s 3 bundles
run4 old 434s 3 bundles
old mean 466s, new mean 302s -- a 35% reduction, with both new runs below
both old runs and no overlap between the groups.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tylervick
marked this pull request as ready for review
August 11, 2026 03:17
This was referenced Aug 11, 2026
tylervick
added a commit
that referenced
this pull request
Aug 11, 2026
Generating a report from TestResults.xcresult on main produced 16 .mp4 references, 0 .png/.jpeg, and 0 rendered `img class="screenshot"` elements. The screenshot templates, the `screenshot` CSS class and the `-z` downsizing branch were executed by no test at all -- that path could have been entirely broken and `swift test` would still have been green. #357 (screenshots missing from reports, open since June 2024) has been uninvestigable for the same reason. Adds FirstSuite.testAttachScreenshot, which attaches XCUIScreen.main.screenshot() with .keepAlways. That yields a real public.png payload, which Attachment.swift maps to isImage -> cssClass "screenshot". It deliberately does NOT launch the app: XCUIScreen captures the simulator screen without it, and #423 removed the launches from these suites as pure flake risk. Restores the image assertion in CliTests, commented out when Xcode 15 began attaching videos by default. Widened to include `img.screenshot-tail`, which the original selector missed, and given failure messages that say what is wrong. Proved non-vacuous rather than assumed: removing the screenshot test and regenerating makes it fail on BOTH the plain and the -z variants ("No image attachments rendered"). Restoring it returns the suite to green. Also: - CoreTests expected 16 rows; there are now 17 (14 XCTest + 3 Swift Testing). Its comment justified not asserting the pass/fail split on the grounds that the suites launch the app in setUp -- false since #423. Rewritten to say what is actually true. - Excludes XCTestHTMLReportSampleApp/.derivedData from SwiftLint. #425 moved DerivedData inside the repo; SwiftLint does not read .gitignore and reported errors from Xcode-generated sources. CI never saw it because the lint job does not generate fixtures, so it only bit locally. swift test: 23 tests, 1 skipped, 0 failures. Closes #393. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
tylervick
added a commit
that referenced
this pull request
Aug 12, 2026
Fixture generation boots a simulator and runs the sample app's UI tests on every CI run; even after #425 it dominates the test job's ~10-minute wall time (#412). The bundles are deterministic for a given toolchain and sample app (#423/#430), and every assertion in the suite compares within one run, so identical bytes are safe to serve across runs. Cache the three .xcresult bundles keyed on the exact Xcode build, the newest installed iOS simulator runtime, prepareTestResults.sh itself, and the sample-app sources. No restore-keys: an inexact match would serve fixtures from a different toolchain, which the drift detector (#392) and the legacy-vs-modern differential test cannot tolerate. A verification step asserts all three bundles exist whether restored or generated, so a cache hit that restores nothing fails instead of passing as a suspiciously fast green run. Local flow is untouched: prepareTestResults.sh is unchanged and swift test still reads the same paths. Fixes #412 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Toward #412. Draft — it carries a temporary measurement workflow that must be deleted before merge.
What #412 actually is
Per-invocation timing pulled from CI logs (
prepareTestResults.shruns underset -ex, so everyxcodebuild testis timestamped):Rows 2 and 3 are identical code, 1.8× apart — that is runner speed, and it is why #422 was closed and why #423's apparent speedup evaporated. What survives is the within-run ratio:
(Sanity + Retry) / totalis 45% / 44% / 43% across those three runs. Nearly half of fixture generation goes to two invocations that produce one test and one retry suite, almost all of it fixed per-invocation overhead.The change
Boot the simulator once,
build-for-testingonce, then run each fixture withtest-without-buildingagainst the shared products. Three invocations become one build + one boot + three cheap test runs.Also in here:
|| truefrom the build. The tests are allowed to fail; a build failure is not. Underxcodebuild test || truea broken sample app surfaced later as a confusingmverror.Verified
swift test— 23 tests, 1 skipped, 0 failures against the newly generated fixturesshellcheckcleantest-without-buildingaccepts-test-iterations/-retry-tests-on-failure, rather than trusting the help textLocal wall clock moved 50.4s → 48.0s, which is not evidence of anything — locally the simulator is already booted and builds are cached, so the overhead this targets barely exists.
How the claim gets tested
.github/workflows/ab-412.ymlruns both generators on the same runner, back to back, counterbalanced (old, new, new, old), resetting DerivedData and shutting down simulators between variants. It also records how many.xcresultbundles each variant produced — a run that generated nothing is not a fast run.If the A/B shows no improvement, this PR gets closed like #422 did. The measurement decides, not the reasoning.
🤖 Generated with Claude Code
Summary by CodeRabbit
Improvements
Chores