ci(cmux-next): the swift test job fails when fewer tests report than ran

Merged
TE
8d ago
+738-46

Ownership: taken over by another agent session under the 10-minute rule (the previous owner had not pushed or replied for over 15 minutes), to add the budget change below and land this.

A truncated swift test run could read as a pass. This makes the job fail when fewer tests report than ran, which is the half of Leo's ask that the job could not do before.

"Then make the job fail when the number of tests that reported is below the number discovered, so it can never read as a pass again."

The hole

The gate only required that a Test run with N tests summary line exist. That line counts the tests that reported, so a run that ends while tests are still going still prints one — and when the tests that did report were green, the job went green with thousands of tests unreported. A missing summary was caught; a lying summary was not.

Two conditions hold on a whole run and fail on a truncated one, and both are now checked:

  1. Every test the log shows starting also finishes.
  2. The summaries count at least as many tests as started.

Not equality: a skipped test prints no started. line, so the summary legitimately runs ahead of the starts (2 starts against 3 tests on the green fixture; a healthy CmuxNextAppTests run measured 399 started, 399 finished, 399 tests in 80 suites).

Red, then green

The behavioural regression is committed with the fix and fails without it. Measured, not asserted — on base's wrapper with this PR's test file:

a truncated run with a summary line exited 0
◇ Test run started.
◇ Test aPasses() started.
✔ Test aPasses() passed after 0.001 seconds.
◇ Test stillGoing() started.
◇ Test alsoStillGoing() started.
✔ Test run with 1 test in 1 suite passed after 0.002 seconds.
RED EXIT: 1

Two tests started, neither finished, Test run with 1 test ... passed, and base exits 0. With this change the same fixture fails the job.

treescripts/cmux-next/tests/swift-test-summary.test.sh
base 3fa409e885c + this test fileexit 1 — a truncated run with a summary line exited 0
this PRexit 0 — ok swift-test-summary

Three parse bugs that would each flip the verdict

These are the ones that make a gate like this dangerous rather than useful, and each has a test:

  • Anchored, not matched anywhere. A test whose own output contains ✔ Test ... passed after could otherwise mark itself finished — the single failure this gate exists to prevent. Anchoring trades that for the opposite risk (a test that writes without a trailing newline runs its output into the next reporter line, and that finish is missed), which produces a red naming the exact test rather than silence. Measured before choosing: of ~12,000 marker lines across three real job logs, every one was anchored.
  • Per run, not per name. One log can hold the same test name twice — the isolated-suite step appends each suite's process to one log — and keying finishes by name let run 1 finishing cover run 2 starting, so a truncation that lost only names an earlier block had already run read as a pass.
  • Every finish shape the reporter emits. Both verbs on all four markers, ➜ … skipped., ➜ … was cancelled after (swift-testing 6.3 Test.cancel()) and the ━ known-issue lines. An earlier revision of this gate missed the cancellation shape, which would have failed every healthy run — a base red across every swift test job, from the change meant to clear base reds. That is why the vocabulary is written out in the script with the reporter's own wording rather than inferred.

The markers are matched as anchored alternations and never a bracket set: awk reads [✔✘➜━] as the set of bytes those characters are built from, and each ends in a byte another contains, so a set matches every line.

The watchdog stops killing healthy runs

It measured the age of open tests, but swift-testing prints every test started. as it enters the run while SWT_EXPERIMENTAL_MAXIMUM_PARALLELIZATION_WIDTH (6 in cmux-next.yml) lets only a few execute, so thousands are legitimately open and waiting. Measured on run 40313902783: 7,159 started, 2,003 finished, 3,927 open past 300 s, and the watchdog killed the step at exit 143 with nothing wrong — the same 3,925 under the merge-base parser, so this is the premise of the check rather than the accounting under it.

Lack of progress is the signal that survives any width. The age of an open test now only decides which tests to name; whether the run is stuck is whether any test finished in the window.

The finish definition is shared by the watchdog and the reporter rather than duplicated, because a copy each is how one came to handle a shape the other did not: the watchdog's copy still read a known-issue pass as unfinished after the reporter's was fixed, so a healthy run would have been sampled and killed.

Budget for a whole broad pass

With stall detection as the hang guard, the swift-test "Run package tests" step gets room for a whole broad pass:

settingbeforeafter
CMUX_NEXT_TEST_HANG_SECONDS (whole-run limit per pass)9001500
step timeout-minutes2030
job timeout-minutes6090

Why: a full broad pass on a slow mini is projected at about 860 s, over the old 900 s cap once the priority pass runs too. Admission (up to 20 min), cache restore (~7 min) and the build (~6 min) already used most of a 60-minute job before tests ran. A real hang now ends on the stall rule (no test finished in 300 s), so a generous whole-run limit costs nothing on a real hang; it only stops a slow healthy pass from being cut off. Five broad passes on the earlier rule were killed at ~300 s with 3,018 to 5,415 tests finished.

The priority and broad passes share one deadline (step start + 28 min): the broad pass gets min(1500, time left), so a pass that hangs before any test starts (nothing for the stall rule to see) still reaches the sampler's own limit and is named and sampled before the 30-minute step timeout.

Each test process held to its own summary

From the independent review: totals alone let one truncated process hide behind another. A log holds a run per test process (◇ Test run started.), and an earlier summary that runs ahead of its starts (skipped tests print none) could cover a later process that was cut off after all its own starts had finished. Each run block must now print its own summary counting at least its own starts; otherwise the job fails and names the block (test run 2 of 2 printed no summary after 1 starts). Checked on the isolated-suite section of a real job log (17 runs, 17 summaries): no error. Further review rounds tightened the same accounting: starts and finishes now match within one test process only (a finish in a later process cannot close an earlier one's lost start), and a skip closes no start at all (swift-testing skips a test instead of starting it, and two suites can share a bare name; one such collision appears in a real job log). The watchdog also fires on silence when no test is open once a test has started (a helper hung after its summary); before the first test line it does not, because a real broad pass printed nothing for 356 s and then 2,000 lines at once, so there the whole-run limit is the guard, and a report that errors out fails the job instead of passing on swift's exit 0. Each has a fixture that fails on the earlier head.

Also narrowed: only the generated ◇ Test case passing N argument(s) ... start is skipped, so a test whose name begins "case passing" is still counted. The new tests fail on the previous head's wrapper.

Also in this push: the sampler's header comment now describes the stall rule, and three wc -l comparisons in swift-test-summary.test.sh strip BSD wc's padding so the test passes on macOS as well as Linux.

Changelog

none

No capture: CI wrapper and its tests; no app surface and nothing to show.

Scope

Split out of #25391 under the ~800-line rule. The other two pieces are separate PRs:

  • the exit(0) cause — HomeOpenFocusesComposerTests' click harness drives AppKit's event queue, AppKit's CFRunLoopStop lands on the process's outermost run loop (which is swift_task_asyncMainDrainQueue's CFRunLoopRun), and main + 132 falls into exit(0)
  • the crash-report diagnosis (SWIFT_BACKTRACE, both report directories)

This PR is the gate alone, so it is reviewable as a parser rather than as three changes at once. It touches the same wrapper as the crash PR, so whichever lands second takes a rebase.

Observed / Not run

Observed: both rows of the table above, run locally in isolated trees extracted from each commit; prepush-gate.py --base --head reporting no new violations for this exact commit; and the split verified loss-free by recombining the two halves of the wrapper diff and diffing against #25391's file (identical).

Not run: a full cmux-next swift test job at this head reporting every test. It will not yet, because the exit(0) cause is a separate PR — this PR makes that truncation fail instead of passing, which is the point, so the first job on a head carrying both is where the complete-run claim gets made.

🤖 Generated with Claude Code

Agent session: agent session


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.