ci: parametrize apple-tests matrix/timeouts, add crash-watch with grace period - #105
Open
o-nnerb wants to merge 5 commits into
Open
ci: parametrize apple-tests matrix/timeouts, add crash-watch with grace period#105o-nnerb wants to merge 5 commits into
o-nnerb wants to merge 5 commits into
Conversation
Lets callers customize which Apple platforms run (or restrict to a subset, e.g. iOS only) via the new apple-tests-matrix JSON input, which defaults to the exact 7-destination matrix already in use so existing callers see no behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a crash-report watcher around the Step 4 Apple Tests xcodebuild run: it snapshots ~/Library/Logs/DiagnosticReports (and the system-wide directory) before testing, then polls for a *new* report while xcodebuild runs. If a crash shows up mid-run, it kills the xcodebuild process early instead of waiting out the full job timeout — the working theory being that a crashed simulator/xctest process sometimes leaves xcodebuild hanging rather than exiting. Whatever crash report is found (whether or not it triggered an early kill) is uploaded as a crash-reports-<name> artifact. Toggleable via the new apple-tests-crash-watch-enabled input (default: true). Also turns every job's previously hardcoded timeout-minutes into a workflow input (one per job, defaulting to its current value), so a caller can raise or lower timeouts per stage without forking the workflow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-watch kill
Two fixes surfaced by reviewing a real watchOS run:
- The "🚨 Identify Test Running at Crash Time" parser paired each
swift-testing "Test case passing N argument(s) to X() started/passed"
line by treating the free-form argument description as part of the
matching key. That description isn't guaranteed to render identically
between a case's started and passed/failed lines, so every argument of
every parameterized test could leak as a permanent false "still
running" entry even though the test (and its outer Test entry) had
already finished — drowning any real signal in noise. These per-case
lines are now skipped entirely; the outer Test-level pair already
reflects whether the test actually hung.
- The apple-tests-crash-watch-enabled kill added previously assumed a
crash report appearing meant xcodebuild was permanently hung. In
practice Xcode's own post-crash diagnostics collection is expected to
run for a while and self-limits at ~10 minutes
("IDETestOperationsObserverDebug: Failure collecting diagnostics ...
Timed out after 600.0 seconds") without affecting test results that
already finished. Killing immediately on the first sign of a crash
report risked tearing xcodebuild down mid-.xcresult write, losing
results/coverage for the whole run over a collection that would have
resolved on its own. The watcher now only kills once xcodebuild has
ALSO gone apple-tests-crash-watch-grace-minutes (default 12, safely
above Xcode's own timeout) with no new line written to
raw-test-output.log — i.e. once it looks genuinely stuck rather than
just running that collection out.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Each apple-tests-matrix entry can now set its own optional `timeout` (minutes), overriding apple-tests-timeout-minutes for just that destination — e.g. giving a known-slow simulator (or one hit by the crash-watch grace period) more headroom without raising the timeout for every other destination in the matrix. Entries that omit `timeout` keep falling back to apple-tests-timeout-minutes, so the default matrix is unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
TEST_PID=$! after backgrounding a `cmd1 | cmd2 &` pipeline captures the PID of the *last* pipeline stage (xcbeautify, or tee in debug mode), not xcodebuild's — confirmed with an isolated `sleep 30 | cat &` repro: `$!` resolves to `cat`, which has no children, so `pkill -P $TEST_PID` finds nothing and `kill $TEST_PID` only kills `cat`, leaving xcodebuild running as an orphan still holding the simulator. The grace-period `exit 1` still failed the step either way, so this wasn't blocking, but the kill never actually touched the process it was meant to stop. Fixed by redirecting xcodebuild's output into a `tee`/`xcbeautify` process substitution (`> >(...)`) instead of piping into it with `|`, so `$!` right after backgrounding is xcodebuild's own PID — verified against the same repro (`$!` now resolves to the actual `sleep`/ stand-in xcodebuild process, and a plain `kill` on it succeeds) and an end-to-end test with a stand-in "xcodebuild" script covering both the kill path (process is confirmed gone, not orphaned) and the normal path (real exit code and full log output still come through). Also sends TERM to the process itself (not just to its children via pkill -P) before escalating to KILL, and guards the post-kill `wait` so set -e (default for run steps without an explicit `shell:`) can't swallow the intended exit code before it's reported. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
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.
Summary
apple-testsjob's simulator matrix into a workflow input (apple-tests-matrix), defaulting to the exact 7 destinations already in use, so a caller can restrict to a subset (e.g. iOS only) or swap in different simulators without forking the workflow.timeout-minutesinto its own input (one per job, defaulting to its current value), so a caller can raise/lower timeouts per stage.xcodebuild test-without-buildingrun: it snapshots~/Library/Logs/DiagnosticReports(+ the system-wide dir) before testing, polls for a new report while xcodebuild runs, and only kills the run if xcodebuild ALSO produces no new test output forapple-tests-crash-watch-grace-minutes(default 12) after that — since Xcode's own post-crash diagnostics collection self-limits at ~10 minutes without affecting already-finished results, and killing on sight risked corrupting/losing the.xcresultfor the whole leg. Whatever crash report is found is uploaded as acrash-reports-<name>artifact regardless. Toggleable viaapple-tests-crash-watch-enabled(default true).Test case passing N argument(s) to X() started/passed) were paired by a free-form argument description that isn't guaranteed to render identically between the started/passed lines, so every argument of every parameterized test could leak as a permanent false "still running" entry. These per-case lines are now skipped; the outerTest X() started/passedpair already reflects a genuine hang.coverage-upload's expected-platforms check from the sameapple-tests-matrixinput instead of a separate hardcoded list, keeping it consistent with a customized matrix.Test plan
actionlint(with shellcheck) on.github/workflows/swift-ci.yaml— cleanapple-tests-matrixJSON round-trips and matches the original 7-destination matrix exactly (no behavior change for existing callers)pipefailstill propagates the real xcodebuild exit code through the backgrounded pipeline, and that the grace-period logic does NOT kill while the log keeps growing but DOES kill once the log genuinely stalls past the grace window🤖 Generated with Claude Code