ci: scheduled Xcode/Swift toolchain drift detector (#392) - #442
Conversation
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR centralizes iOS simulator selection, aligns fixture cache keys with fixture generation, and adds a scheduled workflow that validates the latest stable Xcode and reports toolchain drift through GitHub issues. ChangesSimulator Selection and Toolchain Drift
Estimated code review effort: 4 (Complex) | ~60 minutes Mergeability Score: 🔵 Low · up to The workflow adds scheduled toolchain drift detection, but a failure in the version probe can produce an incomplete or misleading report about the legacy command surface, making breakage harder to diagnose. This is a bounded CI observability risk that is mergeable with explicit owner follow-up. Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant prepareTestResults
participant select_simulator
participant simctl
participant xchtmlreport
participant GitHubIssues
GitHubActions->>prepareTestResults: generate fresh fixtures
prepareTestResults->>select_simulator: request simulator metadata
select_simulator->>simctl: list devices and runtimes
simctl-->>select_simulator: return simulator JSON
select_simulator-->>prepareTestResults: return device, OS, UDID, and runtime build
GitHubActions->>xchtmlreport: validate generated result bundle
GitHubActions->>GitHubIssues: report drift outcomes
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
A scheduled workflow (Mon/Thu + workflow_dispatch) that exercises the newest Xcode on the runners end to end: checks that xcresulttool still advertises legacy command support (the early-warning signal for the #391 migration), builds the package, regenerates fixtures from scratch via prepareTestResults.sh, runs the test suite, and asserts xchtmlreport exits clean against a bundle produced by that exact toolchain. Any fault files or updates a single open `drift`-labelled issue, so a break cannot rot silently in the Actions tab. Fixtures are deliberately regenerated (never cache-restored) because the job's purpose is to exercise the current toolchain — but a successful run saves them under the #436 cache key, pre-warming PR jobs after a toolchain bump. Also folds in the #436 fast-follow: the fixture cache key derived its runtime from `simctl list runtimes` (newest installed) while prepareTestResults.sh selects the newest runtime that offers an iPhone. The selection now lives in scripts/select_simulator.py and both consume it, so the key can never disagree with the runtime the script boots. Closes #392 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5515660 to
f5337f1
Compare
| id: build | ||
| continue-on-error: true | ||
| timeout-minutes: 30 | ||
| run: swift build -v |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/toolchain-drift.yml:
- Around line 45-53: Remove the xcrun xcresulttool version probe from the
required Record toolchain step so Xcode outputs are always written. Move the
probe into the legacy step and export its successful version there, ensuring
Check legacy xcresulttool support and Report drift correctly reflect probe
failures.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 713ebd63-3203-4865-86f9-c65819d5fa9b
📒 Files selected for processing (4)
.github/workflows/test.yml.github/workflows/toolchain-drift.ymlprepareTestResults.shscripts/select_simulator.py
Review follow-up (CodeRabbit): the toolchain recording step invoked `xcrun xcresulttool version` bare, so the tool disappearing entirely would fail recording, skip the legacy probe, and file a generic setup failure instead of the one alarm the workflow exists to raise. Recording now tolerates the failure and the probe swallows the command error so the missing marker fires the explicit ::error either way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review of #442 found the drift job's pre-warm dead on arrival: its save key rendered hashFiles('XCTestHTMLReportSampleApp/**') at save time, after prepareTestResults.sh had left .derivedData inside the hashed tree (actions/glob matches dot-dirs), so the saved key could never equal test.yml's clean-tree restore key — ~95 MB of unreachable cache churned per run. Both workflows now derive the key from one composite action (.github/actions/fixture-cache-key) that runs before generation. The hash inputs and key format are unchanged, so existing cache entries still hit; sharing the action also removes the duplicated key logic the two workflows could silently de-sync on. Also from the review: - run the legacy probe under !cancelled() so a hard-failed record step (xcodebuild/swift dying — the alarm's most important case) cannot skip it, and have the report call out a probe that somehow still did not run - make the record step's xcresulttool tolerance survive pipefail (|| true inside the pipeline) instead of relying on the default shell leaving pipefail off Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes #392.
What this adds
A new scheduled workflow,
.github/workflows/toolchain-drift.yml(Mon/Thu 07:23 UTC +workflow_dispatch), that exercises the newest stable Xcode on the runners end to end and fails loudly. It collects five independent signals per run, eachcontinue-on-errorso one failure cannot mask another:xcresulttoolmarker —xcrun xcresulttool versioncurrently prints(legacy commands format version: 3.56); that parenthetical is Apple's marker for the legacy command surface the pre-migration backend depends on. When it vanishes on a new Xcode, this step fails — the early-warning alarm that Apple pulled--legacywhile the Migrate off xcresulttool --legacy before Apple removes it #391 migration is in flight.swift buildon the new toolchain../prepareTestResults.shfrom scratch, plus the sameInfo.plistcompleteness checktest.ymluses. Runs even if the package build failed (it only needs xcodebuild + a simulator).swift testagainst the freshly generated fixtures.xchtmlreportclean exit against a bundle produced by that exact toolchain — since 3.0 the tool exits non-zero on report degradation, so the exit code is the whole assertion (per the issue).Loud failure
A final report step turns step outcomes into faults and files a GitHub issue labelled
drift(creating the label if needed) containing the toolchain version, the fault list, and a per-step outcome table. De-duplication per the issue spec: while onedriftissue is open, later failing runs append comments instead of filing duplicates, and a later green run also comments so a resolved break is distinguishable from a persistent one. The job itself also exits non-zero, so the run is red in Actions regardless. Manual cancellations file nothing (!cancelled(), notalways()).Cache interplay (#436)
Fixtures are deliberately regenerated, never cache-restored — the job's purpose is to prove the current toolchain can still produce them. But since a successful run has just paid for fresh fixtures on the newest toolchain, it saves them under the same key
test.ymlcomputes: after a toolchain bump on the runners, PR jobs start warm instead of each paying for regeneration.#436 fast-follow: cache key ↔ script alignment
test.ymlderived the cache-key runtime fromsimctl list runtimes(newest installed), whileprepareTestResults.shselects the newest runtime that offers an iPhone — the two could disagree whenever the newest runtime ships without iPhone devices. The selection logic now lives inscripts/select_simulator.py(verbatim extraction of the script's python, extended to emit the runtime build version), and both the script and the cache-key steps consume it, so the key can never disagree with the runtime actually booted. The new file is included in the cache key'shashFiles(...).Notes
main; until then (and for testing)workflow_dispatchworks frommainonly. Noted in the workflow header.timeout-minutes(rather than a job timeout) so a hung simulator still produces a failed step and a filed issue instead of a silently cancelled run.shellcheckandactionlintpass on everything changed; the report step's four outcome paths (create/comment × fault/green) were smoke-tested with a stubbedgh.🤖 Generated with Claude Code
Summary by CodeRabbit
Testing
Chores