Skip to content

Swift tests: name the suite that leaves windows open, and a .closesItsWindows check that closes them and fails that suite - #27003

Merged
teamleaderleo merged 3 commits into
feat-cmux-nextfrom
window-leak-hook
Oct 28, 2026
Merged

teamleaderleo merged 3 commits into
feat-cmux-nextfrom
window-leak-hook

Conversation

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Leo, on Swift suites that leave windows open for the suites after them: "But why do you guys not just clean up after yourselves, or at least do your best to?"

What this adds

The broad Swift pass grew from 3 to 428 open windows in one test process, and since every Core Animation commit lays out each open window that needs it, throughput fell from about 2,000 tests a minute to about 100 and the pass was cut short. This PR is the shared hook and the check; the suite fixes follow in their own PRs.

  1. Every window is tagged with the suite and test that opened it. Tests/Support/CmuxNextTestExitTrace/CmuxNextTestWindowCensus.c (a new file in the existing test-only C target; the app links none of it) swaps NSWindow's two designated inits to record swift-testing's Test.current (as Module.Suite and the test), and -close / -orderWindow:relativeTo: to record whether the window was closed. Windows from suites running in parallel are told apart by their tag, not by timing.
  2. A census at the test process's exit names, per suite, the windows still open: cmux-next window leak: <Module.Suite> left N of the M windows it created open (K on screen): <class> "<title>", ..., after one summary line. It needs no change to any suite, so a suite that opens windows without the check below is still named.
  3. @Suite(.closesItsWindows) (CmuxNextTestSupport/WindowLeakCheck.swift): when the suite ends, it closes every window its tests opened and left open (isReleasedWhenClosed = false, then close()), logs them on stderr, and records an issue on that suite, naming the suite, each window's class and title, and the test that opened it. It is a normal test failure on the suite that leaked, not a crash and not a later suite's failure. A window its test closed is not a leak, even while something still holds it. One main-actor turn passes before a window counts, so a window released without a close is not reported when its autorelease pool drains.

The check fails only suites that carry the trait, so it cannot turn base red on its own. Suites get the trait in the follow-up PRs together with their own teardown fixes; the last of them adds a test that every suite in a file that opens a window carries it.

Tests

WindowLeakCheckTests (CmuxNextAppTests): a window and a panel are tagged with this suite and test; the check closes and names a window the suite left open; a closed window that is still held is not a leak; ordering a closed window back in reopens it.

Measurement

Same harness at this head (the trait is on no suite yet, so this is today's leaking): scripts/ci/package-test-lane.sh suite Packages/macOS/CmuxNext through the fleet, one test process per filter (CmuxNextAppTests by first letter, every other target alone), 32 processes that reached their end.

metric before (this head, no suite fixed) after delta harness
windows still open at process exit, summed 251 (42 on screen) follow-up PRs fleet steps 173f76a44862a5c04019cbd6, 9f91aca811bb76a732202fef, 448e6672289e25f8f20c6316
suites named as leaking 79 suites, 233 windows (18 more opened outside any test) follow-up PRs same
closed windows something still holds 1,027 same

Worst first: SidebarRevealSelectionTests 12, SidebarPinnedRowWidthTests 12, SidebarRowsMoveOnRenderServerTests 9, MultiWindowInputRoutingTests 9, SidebarRowHoverInsetTests 8, SidebarMenuAnchorTests 8, PageFullscreenWholeScreenTests 8, BrowserToolbarPressTargetTests 8, PaletteNavigationTests 7 (all 7 on screen), UpdateReadyCardLayoutTests 7. The usual cause: a test's teardown calls orderOut(nil), which hides the window but keeps it in the app's window list, or the test never closes it.

Two processes gave no census: CmuxNextAppTests.C (the Cloud broker hang #26809 tracks) and CmuxNextAppTests.G (signal 11; the same filter crashed with signal 11 at the base commit in fleet step a0898dde41e141132554161e, and passed at this head in step 6afe330519763d2a4bd29aca).

Risk: test harness only. The swaps only add a tag and a flag around AppKit's own implementations, in the test bundle; no product code changes.

Agent session: agent session

🤖 Generated with Claude Code

teamleaderleo and others added 2 commits October 28, 2026 13:25
…t exit, and a .closesItsWindows suite check

Leo: "But why do you guys not just clean up after yourselves, or at least do your best to?"

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-85ca59f74f02d81734c3
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · session

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Bead cx-453b4x created for this PR by the cmux coordinator.

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review dd28757: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr27003 -f ref=window-leak-hook -f platform=macos -f backend_mode=local

@github-actions github-actions Bot added the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 28, 2026
…t a suite's leak

Batch fixes found TUINSWindow and SPRoundedWindow tagged to whichever suite had a focused field.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-85ca59f74f02d81734c3
@github-actions

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/40790954827 at 61f3b82 failed: cmux-next checks (god files, concurrency, crash safety, l10n).
Those jobs last passed at bd8d4ac. This pull request is one of 1 merged since: #27003.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

This branch was previously deployed

1 inactive deployment
ci-routing-agent — dd28757c Deployed Oct 28, 2026 by teamleaderleo via wait for same-key tree publisher #73845
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:low Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant