ci(ios): run the full XCTest suite nightly and check the PR test list (#1781 A7) - #1789
Conversation
|
Size Report
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. |
|
The new selection checker itself is valid: it passes at the exact head and fails with the correct workflow line after a planted renamed |
…te both test flags
|
Thanks — all three addressed in Blocker found before dispatch: the unfiltered run would have hung. (1) The dispatch — GitHub refuses it pre-merge.
I didn't do the second unprompted — it spends a shared macOS slot and makes the merged trigger block differ from the measured one. Meanwhile I've disclosed in the body what I expect the first run to hit, rather than leaving it to be discovered: reachable set is 151, not 154 ( (2) Ownership test added — (3) Waiting on required checks before undrafting, and not undrafting at all until the dispatch question above is settled. Two more corrections worth flagging, both mine:
Also picked up from the adversarial pass: On your optional suggestion of explicit Re-validated: |
|
CI note so the red marks aren't misread: GitHub had a platform incident while this branch was building, and every failure on it so far is that incident, not this change. Same
The two CodeQL jobs cannot be re-run from the API (
Separately, I filed the size-report papercut as its own task rather than fixing it here: a transient 503 on a cosmetic comment write should not fail a lane whose real work already succeeded. |
|
Follow-up review at |
That specific run is not obtainable pre-merge — not by me, and not by you either. GitHub only exposes So the blocker as written cannot be cleared. Flagging that rather than quietly substituting a different mechanism, because the substitute costs a shared macOS slot and changes what you're reviewing. Two paths actually produce a run that demonstrates all four things you listed (macOS build, A — temporary branch-scoped One honesty caveat, since you asked for this exact head: the measured run would be at the head with the temporary trigger, and removing it changes the SHA. The delta between what ran and what merges is exactly that trigger block — nothing in the build, the skip, the summary path, or the test selection. No pre-merge scheme avoids this; a dispatch would have had the same property in reverse. B — merge, then dispatch on My read: A if you want the evidence before this merges, B if you'd rather not spend the slot on a branch. I lean B — the lane is cheap to run once it's on On the two red CodeQL checks: agreed they're the 503s, and worth knowing they are stuck. Branch is |
|
Merged current The counts drifted within a day, and the check caught itAgainst merged Was 154 / 37 / 1 / 116 yesterday. #1798 and its siblings added three methods ( So I removed them. Numbers still appear in this PR description, which is fine — it is a point-in-time artifact, and I have updated it to the current figures. Gate manifest still models correctly after #1794You asked me to re-check this if #1794 landed first. It did, and it changed the manifest's vocabulary —
Re-validated on the merged tree: Unrelated papercut, filed separately
Still blocked on the same questionThe A-or-B decision is unchanged and unanswered: A temporary branch-scoped |
|
Re-reviewed exact head |
|
Status update at Exact-head CI is now fully greenThe push cleared the stuck CodeQL jobs, as expected — they were unrerunnable only because the previous run was So the only outstanding item is the run evidence. On authorizationTo be precise about where this stands, since I don't want to act on an ambiguity: "until the documented temporary branch trigger is authorized and run" states the condition but doesn't grant it. I've read it as not an authorization and have not touched the trigger. If you do want it, one word — "authorized" — and I'll add it, run it, attach the summary, tighten I'm not going to keep re-asking beyond this; if the answer is "merge first", that's fine too and I'll follow up with the numbers in a second PR. Meanwhile: running the suite locally, at this headRather than wait, I'm running the full suite on this exact tree on local hardware that matches the CI image — Xcode 26.2 / iOS 26.2 simulator runtime, the same versions This is not a substitute for the GitHub Actions run you asked for, and I won't present it as one — it proves nothing about the runner image, the derived-data cache restore, or the I'll post the numbers when it finishes, clearly labelled as local evidence: |
Full-suite evidence — and the lane found two real bugs on its first runRan the complete suite against this exact tree on Xcode 26.2 / iOS 26.2 Simulator — the same versions This is local evidence, not the Actions run you asked for. It says nothing about the runner image, the derived-data cache restore, or Rendered through Device: iPhone 16, iOS 26.2, arm64. (ios.yml prefers iPhone 17 Pro; these are geometry and policy tests, so the model is not load-bearing.) The count confirms the arithmetic exactly157 declared − 1 skipped ( The two failures were both in the dark setNeither appears in They are product defects, not test bugs. Both are the same root cause: the navigation helpers do not reject
There is already a correct precedent in the same target: I have not fixed them here — one issue per PR, and A7's "run" half is explicitly "get them running, no deleting or fixing yet". Filed separately with the repro. Worth noting the consequence for this PR: the nightly will land red on night one, which is the outcome I disclosed up front as the evidence the lane exists to produce.
|
|
Filed the two failures as #1812 — not fixed here, so this PR stays one issue. Digging into the helpers sharpened the diagnosis from "doesn't reject
I reproduced both arithmetic paths independently; they match the failure output byte for byte. The
Nothing changes for this PR: the nightly will report both until they're fixed, which is the disclosed outcome. |
…1795) * ci: keep Bundle Size job green on transient GitHub comment failures The size measurement and job summary had already succeeded on PR #1789 (run 32050847506) when the PR comment write got a 503 during a GitHub incident and failed the whole lane. --post-comment now retries 5xx / 429 / network errors (4 attempts, 1s/2s/4s backoff) on both the list and write calls. If it still fails, it prints a ::warning::, appends a note to $GITHUB_STEP_SUMMARY, and exits 0. Other 4xx (bad token, missing permissions) stay fatal. * refactor: split GitHub response classification to satisfy fallow complexity gate * fix: reconcile uncertain comment creates instead of re-POSTing; add regressions Retry now wraps the whole list -> write cycle rather than each request, so a create whose response was lost (network error / 5xx) is re-listed on the next attempt and turned into a PATCH of the marker comment instead of a duplicate POST. Splits the retry/classify helpers under the fallow complexity gate. Adds scripts/__tests__/size-report-post-comment.test.ts (unit-core): spawns the real script against a stubbed fetch and pins uncertain-create reconciliation, transient exhaustion (warn + exit 0), and fatal 4xx (nonzero, no retry). SIZE_REPORT_RETRY_BASE_MS lets the tests skip real backoff.
Post-merge dispatch evidence — run 32124687738
xcodebuild's own line: Timings
The cache sharing works as designed58 seconds means the derived-data cache hit — ios.yml's Both new steps behaved on the real runner
The arithmetic holds at a different countMerged
Failures: exactly the two expected, nothing newBoth are #1812, both in the dark set, both reproducing identically to my local run — same assertion text, same
|
1 participant
Wave 1 of #1781 A7 — iOS XCTest, the "run" half only. No tests are deleted, moved, or classified here; that is A7's wave-3 work.
The gap
.github/workflows/ios.ymlrunsxcodebuild test-without-buildingwith a hand-written list of-only-testing:AgentDeviceRunnerUITests/RunnerTests/…entries naming a subset of the target's methods. Everything outside that list ran nowhere — not on PRs, not nightly, not in any gate.Figures at this head (
pnpm check:xctest-selectionderives them; nothing hand-copies them any more): 157 declared, 42 selected on every PR, 1 skipped by the nightly, 114 reached only by the nightly. They were 154 / 37 / 1 / 116 one day earlier — see the drift note below.The list is also unverifiable by construction:
xcodebuildtreats a test identifier that matches nothing as an empty set, not an error. A renamed or deleted test silently stops being tested and the lane stays green.1. Nightly full-suite lane
New
.github/workflows/xctest-nightly.yml— schedule30 4 * * *+workflow_dispatch, onemacos-26job.Reuses
./.github/actions/setup-apple-runner-buildand./.github/actions/boot-ios-test-simulatorwith byte-identical inputs to ios.yml (sameIOS_RUNTIME_VERSION: '26.2', samecache-key-prefix/cache-key-suffix, same gate/platform/destination, sameAGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS: '1'), then the samexcodebuild test-without-buildingminus every-only-testing:flag, with-resultBundlePath. Result bundle +xcresulttoolsummary uploadedif: always().-skip-testing:AgentDeviceRunnerUITests/RunnerTests/testCommandis the one filter it keeps, and it is load-bearing.testCommandis not a test: it is the runner's server entry point (RunnerTests.swift:245) — it opens anNWListenerand blocks inXCTWaiter.wait(timeout: 24 * 60 * 60), which is exactly how production drives it (runner-session.ts:219always passes it as the sole-only-testing:). It compiles unconditionally; the#if AGENT_DEVICE_RUNNER_UNIT_TESTSblock ends at line 167, well above it. Alphabetically it is test #16, so the first draft of this lane would have run ~16 tests and then hung totimeout-minutes, red every night, measuring nothing. ItsAGENT_DEVICE_RUNNER_NOOP_STARTUPescape hatch is an env var, and the comment atRunnerTests.swift:126-129records that env plumbing into a simulator test process is not dependable —-skip-testing:is the lever that works from CI.AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTSis load-bearing twice over: it feeds the derived-data cache's build-variant hash and it is the-D AGENT_DEVICE_RUNNER_UNIT_TESTScompile flag that makes the RunnerTests methods exist. Dropping it builds a bundle with no tests in it, whichxcodebuildreports as success. The lane therefore asserts the run executed at least one test.Under the #1781 lane rule (also in the workflow header):
push: mainrun having populated the cache at the same source hash — a miss means the lane also pays a full runner build; (b)concurrencyis keyed ongithub.ref, so a manual dispatch on the same ref cancels an in-flight scheduled run rather than queueing behind it. It does not affect PRs: schedule + manual only, nopull_requesttrigger, no new required check, zero minutes added to any PR.Expected hazards on the first run (disclosed, not yet measured)
The reachable set on this destination is 154, not 157:
testCommandis skipped, and two tests (testResolveBlockingSystemModalIsAbsentWithoutSpringBoardOnTvOS,testBlockingSystemAlertSnapshotIsNilOnTvOS) sit behind#if os(tvOS)and cannot execute on an iOS Simulator. Of the rest, these have never executed anywhere and may well be red on first contact:testTapPointPolicyMatchesGoldenParityTable— resolves its fixture through#filePathfrom inside the simulator, i.e. a host path the test process may not be able to read.testExecuteDispatchedReturnsBusyBeforeBlockingSystemModalProbeDrains— does realapp.launch/terminateagainst 15s deadlines.timeout-minutesis currently 120. That is the hang's ceiling, not an estimate; it should be tightened to ~2× the measured duration once a run exists.2. Discovery check, so neither list can silently drift
pnpm check:xctest-selection(scripts/check-xctest-selection.ts, parse-only — no Xcode, no simulator) enumerates everyTarget/Class/methodthe Swift sources declare, parses both-only-testing:and-skip-testing:identifiers out of both workflows, and fails if any names a method no source declares. Both directions fail silently in their own way, and the skip direction is the expensive one — a typo there re-arms the 24-hour hang with no signal at all.It does not fail on tests missing from the PR list; that is the nightly's job. It does fail if either scan comes back empty, or if a guarded workflow no longer exists, so a broken parser cannot report a healthy list forever.
Output today:
Drift note. Those were 154 / 37 / 1 / 116 when this PR opened; merging
maina day later moved them to 157 / 42 / 1 / 114 (three methods added, five PR entries added). Every entry still resolves, so the check passes — but the counts I had hand-copied into comments were already wrong.8e54e29etherefore removes hardcoded counts from the workflow header, the ci.yml comment, and the docs row, leaving them to describe the relationship and pointing at this command for the live split. A count in a comment is the same class of defect this PR exists to close.Registered the way the repo registers checks:
CheckId+ALL_CHECKS+CHECK_CATALOG(xctest-selection), aBUILD_OWNERSHIPrule so a change underAgentDeviceRunnerUITests/selects it, arun-gatestep inCI / Affected-check Selector(next to the gate manifest — same family of failure: a CI selection that stopped selecting what it claims), andcheck:tooling.pnpm check:gate-manifest: 49 checks wired across 34 lanes.Tests — 28 cases across two files, both in
unit-core:scripts/__tests__/xctest-selection.test.ts(18): the real tree passes; a typo planted into the real ios.yml text fails with the right line; a typo planted into the real nightly-skip-testing:entry fails too; the declared-set count is derived independently of the check's own file filter; plusclass func, afinal class ResultBoxnested in a test body, comment lines, and both blind-parse guards.scripts/__tests__/xctest-run-summary.test.ts(10): the lane's reporter/liveness check, which otherwise only ever executes at 04:30 on a macOS runner — zero-test detection, the failure-list cap, message truncation, and newline flattening so a stack trace cannot forge markdown headings in the job summary.Review fixes in
353c8276testCommandwould have hung the lane. Added-skip-testing:, and extended the check to validate skip identifiers with the same rigour as only identifiers (+ a planted-typo test for it).RunnerTests*.swiftmissedRunnerTapPointPolicy.swift, which declares a real addressable test — the Xcode project uses aPBXFileSystemSynchronizedRootGroup, so membership is the directory, not the name. Now globs*.swift. That moved the declared count from 153 to 154 at the time;8e54e29elater removed the hardcoded counts from the durable files entirely, for the reason in the drift note above.fs.existsSync; a missing one is a named failure rather than a claim nothing backs.[ ! -s ]not[ ! -f ], so a truncated summary hits the crafted message;--compactonxcresulttoolwith a plain-form fallback; and the job-summary rendering moved out ofnode -einto a tested script with an explicit failure-list cap, for the 1 MiB limit.scripts/check-affected/model.test.ts): a change underAgentDeviceRunnerUITests/**selects exactlyswift-runner-ios,swift-runner-macos,xctest-selection;RunnerTapPointPolicy.swiftselects it too;apple/runner/Sources/**does not.Two defects the review process surfaced in my own work, both now regression-tested: the scan counted its own explanatory comments as flags (inflating 37→39 and inventing a second skip), and the per-lane counts were aggregated by flag rather than by workflow.
On the optional explicit
.github/workflows/ios.ymlownership: deliberately not added.selectChecksclassifies workflow paths as fail-open, so a change toios.ymlalready selects the full check set includingxctest-selection(verified). An explicit rule would be dead code.Validation
pnpm check:tooling— green (includes the newcheck:xctest-selection, plus format/lint/typecheck/layering/depgraph/gate-manifest/production-exports/build/package)pnpm check:gate-manifest→ok — 49 checks wired across 34 lanes, 1 declared unprovablepnpm check:gate-manifest:test(28),pnpm check:affected:test(36, incl. the new ownership case)pnpm exec vitest run --project unit-core scripts/__tests__/xctest-selection.test.ts scripts/__tests__/xctest-run-summary.test.ts(28)pnpm check:fallow --base origin/main— cleanactionlint -shellcheck=shellcheck .github/workflows/xctest-nightly.yml .github/workflows/ci.yml— cleanNot validated: the live macOS run.
workflow_dispatchis refused pre-merge because GitHub only exposes it for workflows present on the default branch. Two ways forward, your call:gh workflow run xctest-nightly.yml, and tightentimeout-minutesfrom 120 to ~2× measured in a follow-up. Risk is bounded: worst case is one red night with a result bundle attached, and the hang that made this dangerous is now skipped.push:trigger scoped to this branch (push triggers have no default-branch requirement), get the numbers, then strip it before merge. Costs one macOS slot now and means the merged trigger block differs from the measured one.I have not done (2) unprompted — it spends a shared macOS slot and changes the artifact under review.