Skip to content

base red: NativePopupSliderTests makes the NSApplication its arrow keys need - #33094

Merged
lawrencecchen merged 2 commits into
feat-cmux-nextfrom
base-red-popup-slider
Nov 3, 2026
Merged

lawrencecchen merged 2 commits into
feat-cmux-nextfrom
base-red-popup-slider

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Base red on feat-cmux-next: CmuxNextDesignTests.NativePopupSliderTests.namedStopsRestWithoutCommittingAndEnterCommitsOnce fails at NativePopupSliderTests.swift:41/42 (values ["1"], expected ["1", "0"]; label High, expected Low).

Cause: NativePopupDiscreteSlider's arrow keys set doubleValue and then call sendAction(action, to: target), which AppKit routes through NSApp. The package-test lane runs each suite in its own process, and this suite made no NSApplication before its first key, so the action went nowhere and the view never published the new stop. It passed only after another suite in the same process had made NSApplication.shared.

Fix: the test's key helper makes the application first, as the other design suites do (HoverCardNoActivateTests, NativeContextMenuTests). No product change: the app always has NSApp.

Culprit: bc7d6bd (#27142) added the suite.

Owner: no OWNERS.md row covers CmuxNextDesign/Overlay/NativePopup*.

Testing

Command: cmux-ci run --class light --display --script scripts/ci/package-test-lane.sh --ref SHA --arg=suite --arg=Packages/macOS/CmuxNext --arg=CmuxNextDesignTests.NativePopupSliderTests (5 runs in one step).

  • Base (feat 73eaec7 on cmuxs-Mac-mini and cmux11s): fails at lines 41 and 42.
  • Tip: job a554671b75a6b853405d5c5d at feat 11dfe91 (cmux8s) is red on the first two of five rounds for this suite.
  • Head: job 1f8b796bb8f159ec80165214 (cmux9s) at 62a37b8: passed 5 of 5. An earlier job, 87da12d1eddb21067d9445e8 (cmux11s), passed 4 of 4 before its host lost the lease.

Each job's lane summary is red for a reason outside this change: on these hosts the target's XCTest pass aborts (signal 6, after Contacts/AddressBook CoreData store errors) or hangs ~680-900 s with no test in flight, after the Swift Testing pass. The Swift Testing results below are the suite's own.

Observed / Not run

Observed: the fleet runs above. python3 scripts/cmux-next/prepush-gate.py passes.
Not run: no app build; test-only change.

Changelog

none

… keys need

namedStopsRestWithoutCommittingAndEnterCommitsOnce failed at
NativePopupSliderTests.swift:41/42 (values ["1"], label High): the left
arrow moved nothing that the test could see. NativePopupDiscreteSlider's
arrow keys set doubleValue and then call sendAction(action, to: target);
NSControl.sendAction routes through NSApp. The package-test lane runs each
suite in its own process, and this suite made no NSApplication before its
first key, so NSApp was nil, the action went nowhere and the view never
published the new stop. The test passed only after another suite in the
same process had made NSApplication.shared.

The key helper now makes the application first, as the other design
suites do (HoverCardNoActivateTests, NativeContextMenuTests). No product
change: the app always has NSApp.

Culprit: bc7d6bd (#27142) added the suite.
Red: fleet job 2bf90b6c18ca136f3fb6c15d at feat 11dfe91.
@lawrencecchen lawrencecchen added the risk:low Risk tier (docs/ci/risk-tiers.md) label Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 62a37b8: 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=pr33094 -f ref=base-red-popup-slider -f platform=macos -f backend_mode=local

web and size checks: green

Shipped in:

@blacksmith-sh

blacksmith-sh Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmp2g9un6_2/summary-fail.json (live):/summary-fail.json (live): View Logs
/tmp/tmppbz40fi1/summary-missing.json (live):/summary-missing.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs

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

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent review subagent, Opus 5.5 claude-opus-5-5, read-only): SHIP at 62a37b8

  • The cause is correct. NativePopupDiscreteSlider.keyDown (NativePopupSliderView.swift:160-164) sets the value, then calls sendAction(action, to: target), which goes through NSApp. In a test process with no NSApp, the action never runs. In the app, key events reach the slider only while NSApp exists.
  • The test is not weakened. The only change is _ = NSApplication.shared in the key() helper. Every assertion stays the same.
  • Global state: NSApplication.shared stays for the life of the process. Nine other design suites do the same. The suite keeps .closesItsWindows.
  • Fleet: NativePopupSliderTests passed 5 of 5 in job 1f8b796bb8f1. The lane is red overall only because of the known fleet XCTest-stage AddressBook crash.

Nit: the helper makes the app object on the first key, so numericSliderReceivesInitialKeyboardFocus runs with or without NSApp depending on test order. Making it in the suite's init() would remove that order effect.

@lawrencecchen
lawrencecchen marked this pull request as ready for review November 3, 2026 07:37
@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 3, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Correction to my review receipt above: the lanes were red overall because of the test-bundle run-loop guard, which held the main thread in xctest. It was not the macOS AddressBook store; that was only a symptom. #33023 and #33013 fixed it on feat (ecfbb44). This head is older than that fix, so its lane still shows the XCTest stage red. The suite's own result (passes at this head, fails at the tip) is unchanged. No action is needed on this PR.

@cursor

cursor Bot commented Nov 3, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 3, 2026 08:04 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 3, 2026 08:04 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 3, 2026 08:08 Inactive
@lawrencecchen
lawrencecchen merged commit bb27df2 into feat-cmux-next Nov 3, 2026
149 of 151 checks passed
@lawrencecchen
lawrencecchen deleted the base-red-popup-slider branch November 3, 2026 08:22

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 3a84f162 Deployed Nov 3, 2026 by cmux-bot[bot] via wait for same-key tree publisher #104801
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:low Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant