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
Conversation
datlechin
marked this pull request as ready for review
September 25, 2026 18:49
This was referenced Sep 25, 2026
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
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.
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.
InspectorToolbarPlacementTestsbuilds the real connection window throughTabWindowControllerat 1512x861, attaches the realMainWindowToolbarwith 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:
inspectorSplitItem.isCollapsed, nottoggleInspector(). The command goes throughanimator(), 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 byTrailingPaneRevealTests.Deleting the
.flexibleSpacebefore the toggle inMainWindowToolbar.defaultItemIdentifiersturns 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.
window.toolbarsforexecution-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.ResultStatusBardraws 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.
ResultStatusModelgainsshowsExecutionWithoutReadout: 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. FourResultStatusModelTestscases pin the rule, and forcing it tofalseturns 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
ProgressViewis backed by a spinningNSProgressIndicator, whose role isAXBusyIndicator). The running query is typed throughtypeQuery, 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.
testTheExecutingIndicatorAppearsWhileAQueryRunsAndClearsAfterItpassed 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:
EditorTabInteractionViewowns the press and returnsfalsefrommouseDownCanMoveWindow. It addedtestDraggingATabNeverMovesTheWindowandtestAFastDragReordersTheStrip, 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.
EditorTabStripWindowDragTestspins 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 onEditorTabInteractionView, which never moves the window. A second case checks that the titlebar above the toolbar items still answerstrue, so window dragging and double-click-to-zoom there are untouched. FlippingmouseDownCanMoveWindowtotrueturns 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 1testDraggingATabReordersAnOverflowingStripandtestDraggingTheSelectedTabReordersTheStrip, shard 2testDraggingATabReordersTheStrip.Test plan
verify.sh build, andbuild-for-testingofTableProUITestsverify.sh test InspectorToolbarPlacementTests EditorTabStripWindowDragTests ResultStatusModelTests ResultStatusBarLayoutTests QueryResultPresentationTestsplus the neighbouring window suites (ConnectionWindowChromeTests,EditorWindowInitialFrameTests,TrailingPaneRevealTests,EditorTabStripAccessoryControllerTests,EditorTabMiddleClickTests): all passswiftlint lint --stricton every touched Swift file: 0 violationsFound on the way, not changed here
ConnectionWindowChromeUITestshas two more cases,testTheDefaultToolbarCarriesItsControlsAndNoModeControlandtestRefreshLeavesTheToolbarOnACreateTableTab, 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/testCommittedKeywordTakesTheTypedCasefailed its first attempt and passed its retry in round 2. Unrelated to this change.NSFileHandleOperationException("-[NSConcreteFileHandle availableData]: unknown error") raised insideProcessNativeDumpRunner.start's stderrreadabilityHandler, which callsavailableData. XCTest pinned it onAppSettingsJsonExtractorTests/testNpgsql(), the case running when the process aborted.availableDataraises 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.