Skip to content

fix(datagrid): report a query tab's first run in the status bar, and put the six quarantined UI cases back on the gate - #3130

Merged
datlechin merged 5 commits into
mainfrom
fix/ui-quarantine-burn-down
Sep 25, 2026
Merged

datlechin merged 5 commits into
mainfrom
fix/ui-quarantine-burn-down

Conversation

@datlechin

@datlechin datlechin commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Empties .github/macos-ui-test-quarantine.txt. Each of its six cases is either back on the gate or replaced by a unit test that makes the same assertion. One of them was hiding a real bug, fixed here.

Inspector toggle placement (1 case, replaced)

Cause. The case pins a 1512x861 window and the runner's screen is 1024x768. Measured in the test host: a window pinned wider than its screen keeps its width while it is off screen (3000pt on a 2560pt screen stayed 3000pt), and is clamped to the screen the moment it is ordered front (it came back 2560pt). On the runner the window shrank, the toolbar sent its trailing items to the overflow menu, and the case skipped itself on every run.

Change. InspectorToolbarPlacementTests builds the real connection window through TabWindowController at 1512x861, attaches the real MainWindowToolbar with the default item set, and never orders it front. It measures the inspector toggle's distance from the window's trailing edge with the inspector closed, open, and closed again. It also checks that the Refresh button moves by more than 100pt on each transition, which proves AppKit laid the toolbar out again for the new pane width rather than the test reading a frozen layout.

Two things differ from the UI case, both measured:

  • The toolbar gets a private identifier with autosave off. The test host shares the app's defaults, and a saved arrangement (mine had Back and Forward added) would otherwise decide the item set.
  • The pane is opened and closed through inspectorSplitItem.isCollapsed, not toggleInspector(). The command goes through animator(), and in the test host that animation never completes: its completion handler did not fire within 3 seconds, off screen or on, and the pane geometry only caught up when the next toggle started. The command itself is covered by TrailingPaneRevealTests.

Deleting the .flexibleSpace before the toggle in MainWindowToolbar.defaultItemIdentifiers turns the test red: the toggle moves 217pt when the inspector opens and ends 225pt in from the edge. The UI case is deleted.

Execution indicator (1 case, app bug)

Cause. Two layers.

  1. The case polled window.toolbars for execution-indicator, and the indicator left the toolbar in refactor(toolbar): rebuild the main window toolbar around native grouped items #2668 for the results status bar. Its sibling, which asserts the indicator is gone after each table loads, was scoped the same way and could not fail.
  2. With the queries pointed at the window, round 1 (run 36126826071) still failed: 176 polls on the first attempt and 175 on the retry, about 85ms apart, never found Stop. The screen recording shows why. While a new query tab runs its first query, the editor shows Stop and the tab shows a spinner, but the status bar is empty. The query took 2.95s and the bar only filled in once it finished. ResultStatusBar draws the execution readout inside the rows readout, and a query tab shows no rows readout until it has columns, so the spinner and Stop never appear on a tab's first run, or after a statement that returned no columns.

Change. ResultStatusModel gains showsExecutionWithoutReadout: a result mode, nothing described yet, and a fetch running. The bar then shows the execution indicator on its own in the readout's place. A table tab is unchanged, since it always has a readout. Four ResultStatusModelTests cases pin the rule, and forcing it to false turns the first-run case red. CHANGELOG has a Fixed entry.

The UI cases now query the window with types: Stop as a button, the spinner as an activity indicator (a SwiftUI ProgressView is backed by a spinning NSProgressIndicator, whose role is AXBusyIndicator). The running query is typed through typeQuery, which retypes it if keystrokes are lost. Round 2 carried a one-off diagnostic that would have named the element type had the typed indicator query missed; it did not miss, and the diagnostic is gone.

CI. testTheExecutingIndicatorAppearsWhileAQueryRunsAndClearsAfterIt passed on the first attempt in shard 2 in round 2 (run 36135765290) and again in round 4 (run 36150062875, diagnostic removed, merged with main). testTheExecutingIndicatorClearsOnceEachTableHasLoaded, now able to fail, passed in shard 0 both times.

Editor tab reorder (4 cases, back on the gate)

Cause. These were quarantined on 2026-08-27 on the suspicion that AppKit's titlebar window drag took the press, since the strip lives in a bottom titlebar accessory. #2571 landed two days later and settled it: EditorTabInteractionView owns the press and returns false from mouseDownCanMoveWindow. It added testDraggingATabNeverMovesTheWindow and testAFastDragReordersTheStrip, which drag through the same helper and pass on CI, and nobody took the four older cases off the list. That fix shipped in 0.70.0, so there is no app change for it here.

Change. The four cases are back on the gate. EditorTabStripWindowDragTests pins the contract without a display: it builds the real window with its toolbar, shows the band, and hit-tests the window's frame view at the leading edge, centre and trailing edge of every tab. Each press lands on EditorTabInteractionView, which never moves the window. A second case checks that the titlebar above the toolbar items still answers true, so window dragging and double-click-to-zoom there are untouched. Flipping mouseDownCanMoveWindow to true turns the first case red on all nine points.

CI. All four passed on the first attempt, no retry, in round 1 (run 36126826071), round 2 (run 36135765290) and round 4 (run 36150062875). Shard 0 ran testDraggingAnUnselectedTabReordersTheStrip, shard 1 testDraggingATabReordersAnOverflowingStrip and testDraggingTheSelectedTabReordersTheStrip, shard 2 testDraggingATabReordersTheStrip.

Test plan

  • verify.sh build, and build-for-testing of TableProUITests
  • verify.sh test InspectorToolbarPlacementTests EditorTabStripWindowDragTests ResultStatusModelTests ResultStatusBarLayoutTests QueryResultPresentationTests plus the neighbouring window suites (ConnectionWindowChromeTests, EditorWindowInitialFrameTests, TrailingPaneRevealTests, EditorTabStripAccessoryControllerTests, EditorTabMiddleClickTests): all pass
  • The new unit tests pass on CI (runs 36126826071 and 36135765290, macOS 26 on the 1024x768 runner)
  • Round 4 UI shards: 239 passed, 2 skipped, 0 retried. Round 3 was cancelled by the merge of main (test: drop display-name-only @Suite attributes that make the test module compile quadratically #3129).
  • Each new test turned red against the mutation named above
  • swiftlint lint --strict on every touched Swift file: 0 violations
  • UI cases are proved on CI only; local XCUITest is blocked by the UI Automation prompt

Found on the way, not changed here

  • ConnectionWindowChromeUITests has two more cases, testTheDefaultToolbarCarriesItsControlsAndNoModeControl and testRefreshLeavesTheToolbarOnACreateTableTab, that skip on every CI run for the same 1024pt screen reason. They are the two skips in round 2's 239 passed and 2 skipped. They are not in the quarantine file, so nothing shows that they gate nothing. The off-screen fixture here is how they could run.
  • EditorAutocompleteFocusUITests/testCommittedKeywordTakesTheTypedCase failed its first attempt and passed its retry in round 2. Unrelated to this change.
  • Round 4's unit job died on an uncaught NSFileHandleOperationException ("-[NSConcreteFileHandle availableData]: unknown error") raised inside ProcessNativeDumpRunner.start's stderr readabilityHandler, which calls availableData. XCTest pinned it on AppSettingsJsonExtractorTests/testNpgsql(), the case running when the process aborted. availableData raises rather than returning an error, so the same handler can take the app down during a real dump. It is outside this change and is not fixed here; the job was re-run.

@datlechin datlechin changed the title test(ci): put the six quarantined UI cases back on the gate fix(datagrid): report a query tab's first run in the status bar, and put the six quarantined UI cases back on the gate Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant