From 944b24c37e8a592b21a0053430bac374d4fdf209 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 3 Sep 2026 19:13:28 +0700 Subject: [PATCH 1/3] test(coordinator): give a trigger edit its execution gate so the drop test cannot raise a modal alert Claude-Session: https://claude.ai/code/session_01NdXqgRevM8nxhW8HXhN7aU --- TablePro/Core/Database/TriggerEditing.swift | 15 +++++--- .../Compare/CompareSyncExecutorTests.swift | 18 ---------- .../Database/TriggerInfoMappingTests.swift | 16 +++++++-- .../Helpers/StubExecutionGates.swift | 36 +++++++++++++++++++ 4 files changed, 61 insertions(+), 24 deletions(-) create mode 100644 TableProTests/Helpers/StubExecutionGates.swift diff --git a/TablePro/Core/Database/TriggerEditing.swift b/TablePro/Core/Database/TriggerEditing.swift index 58454d9af..7aee965ca 100644 --- a/TablePro/Core/Database/TriggerEditing.swift +++ b/TablePro/Core/Database/TriggerEditing.swift @@ -53,9 +53,10 @@ enum TriggerEditing { sql: String, isEdit: Bool, originalName: String?, - originalDefinition: String? + originalDefinition: String?, + gate: any ExecutionGate = ExecutionGateProvider.shared ) async throws { - let decision = await ExecutionGateProvider.shared.authorize( + let decision = await gate.authorize( OperationRequest( connectionId: connection.id, databaseType: connection.type, @@ -95,7 +96,13 @@ enum TriggerEditing { AppCommands.shared.refreshData.send(DataRefreshRequest(connectionId: connection.id, scope: scope)) } - static func drop(scope: DatabaseScope, connection: DatabaseConnection, tableName: String, name: String) async throws { + static func drop( + scope: DatabaseScope, + connection: DatabaseConnection, + tableName: String, + name: String, + gate: any ExecutionGate = ExecutionGateProvider.shared + ) async throws { guard let driver = DatabaseManager.shared.driver(for: connection.id) else { throw TriggerEditingError.notConnected } @@ -103,7 +110,7 @@ enum TriggerEditing { throw TriggerEditingError.dropUnavailable } - let decision = await ExecutionGateProvider.shared.authorize( + let decision = await gate.authorize( OperationRequest( connectionId: connection.id, databaseType: connection.type, diff --git a/TableProTests/Core/Compare/CompareSyncExecutorTests.swift b/TableProTests/Core/Compare/CompareSyncExecutorTests.swift index 0b81d289f..7e0335dfc 100644 --- a/TableProTests/Core/Compare/CompareSyncExecutorTests.swift +++ b/TableProTests/Core/Compare/CompareSyncExecutorTests.swift @@ -52,24 +52,6 @@ private final class RecordingDriver: PluginDatabaseDriver, @unchecked Sendable { } } -private struct AlwaysAllowGate: ExecutionGate { - func authorize(_ request: OperationRequest) async -> OperationDecision { - .authorized(OperationReceipt( - connectionId: request.connectionId, - kind: request.kind, - effectiveWrite: true, - grantedAt: Date(), - token: UUID() - )) - } -} - -private struct AlwaysDenyGate: ExecutionGate { - func authorize(_ request: OperationRequest) async -> OperationDecision { - .denied(reason: "Read-Only connection") - } -} - final class CompareSyncExecutorTests: XCTestCase { private func endpoint() -> DatabaseEndpoint { DatabaseEndpoint( diff --git a/TableProTests/Core/Database/TriggerInfoMappingTests.swift b/TableProTests/Core/Database/TriggerInfoMappingTests.swift index 8fec17011..9a516e8a4 100644 --- a/TableProTests/Core/Database/TriggerInfoMappingTests.swift +++ b/TableProTests/Core/Database/TriggerInfoMappingTests.swift @@ -244,6 +244,11 @@ struct TriggerApplyExecutionTests { /// The session driver holds whatever transaction a query tab left open, so a trigger edit /// with a BEGIN of its own ran inside it and committed or rolled back the tab's work. + /// + /// The gate is supplied rather than taken from `ExecutionGateProvider`. A drop is a + /// `.destructiveQuery`, which the real gate confirms with an `NSAlert`, and an alert raised + /// with no window runs application-modal: on a CI runner nobody answers it, so the whole + /// unit job stops there and is killed by its timeout rather than failing. @Test("Apply and drop run on the pooled connection and leave the session driver alone") func applyAndDropRunOnThePooledConnection() async throws { let connection = TestFixtures.makeConnection(database: "app", type: .postgresql) @@ -272,9 +277,16 @@ struct TriggerApplyExecutionTests { sql: "CREATE TRIGGER t", isEdit: false, originalName: nil, - originalDefinition: nil + originalDefinition: nil, + gate: AlwaysAllowGate() + ) + try await TriggerEditing.drop( + scope: scope, + connection: connection, + tableName: "orders", + name: "t", + gate: AlwaysAllowGate() ) - try await TriggerEditing.drop(scope: scope, connection: connection, tableName: "orders", name: "t") #expect(pooledStub.executedQueries == ["BEGIN", "CREATE TRIGGER t", "COMMIT", "DROP TRIGGER t"]) #expect(sessionStub.executedQueries.isEmpty) diff --git a/TableProTests/Helpers/StubExecutionGates.swift b/TableProTests/Helpers/StubExecutionGates.swift new file mode 100644 index 000000000..a2d310423 --- /dev/null +++ b/TableProTests/Helpers/StubExecutionGates.swift @@ -0,0 +1,36 @@ +// +// StubExecutionGates.swift +// TableProTests +// +// The real gate confirms a destructive statement with an NSAlert, and an alert raised with no +// window runs application-modal. Nothing answers it on a CI runner, so a test that reaches the +// real gate stops the whole job there. Every test that drives a gated operation supplies one of +// these instead and asserts on what ran, not on who was asked. +// + +import Foundation +@testable import TablePro + +internal struct AlwaysAllowGate: ExecutionGate { + internal func authorize(_ request: OperationRequest) async -> OperationDecision { + .authorized(OperationReceipt( + connectionId: request.connectionId, + kind: request.kind, + effectiveWrite: true, + grantedAt: Date(), + token: UUID() + )) + } +} + +internal struct AlwaysDenyGate: ExecutionGate { + internal let reason: String + + internal init(reason: String = "Read-Only connection") { + self.reason = reason + } + + internal func authorize(_ request: OperationRequest) async -> OperationDecision { + .denied(reason: reason) + } +} From f9c51bb751ba215a95fc01c06fee99105de80678 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 3 Sep 2026 19:13:35 +0700 Subject: [PATCH 2/3] test(structure): apply a session's staged edits against the pooled connection the save now runs on Claude-Session: https://claude.ai/code/session_01NdXqgRevM8nxhW8HXhN7aU --- .../StructureEditingSessionTests.swift | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/TableProTests/Views/Structure/StructureEditingSessionTests.swift b/TableProTests/Views/Structure/StructureEditingSessionTests.swift index 53273a799..8ab87838c 100644 --- a/TableProTests/Views/Structure/StructureEditingSessionTests.swift +++ b/TableProTests/Views/Structure/StructureEditingSessionTests.swift @@ -156,17 +156,29 @@ struct StructureEditingSessionTests { /// The save no longer needs a mounted structure view. This is the whole point: `hasUnsavedWork` /// reads the session, so the prompt offering Save can be raised by a tab showing its Data view /// or by a background tab in a batch close, and the save it offers has to reach the work. + /// + /// The ALTER lands on a pooled connection, not the session driver, because a save runs on the + /// schema change route. The pool is seeded here for the same reason it is in + /// `DatabaseManagerSchemaChangeRoutingTests`: no plugin loads under XCTest, so a pool left to + /// open its own connection reports the driver missing and nothing runs. @Test("A session applies its staged edits with no view mounted") func applyRunsWithoutAView() async throws { let connection = TestFixtures.makeConnection(database: "testdb") - let driver = StructureSessionDriver() - let adapter = PluginDriverAdapter(connection: connection, pluginDriver: driver) - var connectionSession = ConnectionSession(connection: connection, driver: adapter) + let sessionDriver = StructureSessionDriver() + var connectionSession = ConnectionSession( + connection: connection, + driver: PluginDriverAdapter(connection: connection, pluginDriver: sessionDriver) + ) connectionSession.browseDatabase = "testdb" DatabaseManager.shared.injectSession(connectionSession, for: connection.id) - defer { DatabaseManager.shared.removeSession(for: connection.id) } let session = Self.makeSession(connection: connection) + let pooledDriver = try await Self.seedPooledDriver(connection, scope: session.scope) + defer { + MetadataConnectionPool.shared.closeAll(connectionId: connection.id) + DatabaseManager.shared.removeSession(for: connection.id) + } + Self.stageAColumn(on: session) #expect(session.changeManager.hasChanges) @@ -174,9 +186,22 @@ struct StructureEditingSessionTests { #expect(outcome == .applied) #expect(outcome.allowsClose) - #expect(driver.executedQueries.contains { $0.contains("ADD COLUMN") }) + #expect(pooledDriver.executedQueries.contains { $0.contains("ADD COLUMN") }) + #expect(sessionDriver.executedQueries.isEmpty) #expect(!session.changeManager.hasChanges) #expect(session.appliedVersion == 1) #expect(!session.hasLoaded) } + + /// Stands in for the connection the pool would open on the scope. + private static func seedPooledDriver( + _ connection: DatabaseConnection, + scope: DatabaseScope + ) async throws -> StructureSessionDriver { + let driver = StructureSessionDriver() + let adapter = PluginDriverAdapter(connection: connection, pluginDriver: driver) + try await adapter.connect() + MetadataConnectionPool.shared.injectEntry(adapter, scope: scope) + return driver + } } From 06d49ff505f923a514168f0220f21648daa0e4c2 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 3 Sep 2026 19:13:35 +0700 Subject: [PATCH 3/3] test(ui): switch the result view through the menu so an occluded status bar cannot swallow the click Claude-Session: https://claude.ai/code/session_01NdXqgRevM8nxhW8HXhN7aU --- .../StructureColumnMoveUITests.swift | 14 +----- .../StructureConstraintsTabUITests.swift | 10 +---- .../StructureRowMenuParityUITests.swift | 14 +----- .../StructureTabIdentityUITests.swift | 12 +---- TableProUITests/Support/UITestCase.swift | 44 +++++++++++++++++++ 5 files changed, 51 insertions(+), 43 deletions(-) diff --git a/TableProUITests/StructureColumnMoveUITests.swift b/TableProUITests/StructureColumnMoveUITests.swift index f5b7cbfcd..71064650d 100644 --- a/TableProUITests/StructureColumnMoveUITests.swift +++ b/TableProUITests/StructureColumnMoveUITests.swift @@ -20,7 +20,7 @@ final class StructureColumnMoveUITests: UITestCase { XCTAssertTrue(row.waitToExist(timeout: 20), "The object browser must list Album") clickAtCenter(row) - showStructure(in: window) + showStructure(in: app, window: window) let grid = window.tables.matching(identifier: "data-grid").firstMatch XCTAssertTrue(grid.waitToExist(timeout: 30), "The structure editor must draw its column grid") XCTAssertTrue( @@ -35,9 +35,7 @@ final class StructureColumnMoveUITests: UITestCase { "The grid must be laid out before a coordinate is taken off it" ) - /// A point offset from the grid, never a row or cell element: the grid's columns are - /// siblings of its rows and later in the tree, so XCUITest reads both as obscured. - let target = grid.coordinate(withNormalizedOffset: .zero).withOffset(CGVector(dx: 80, dy: 40)) + let target = gridPoint(in: grid, of: window, dy: 40) /// Right-clicking an unselected row falls through to the row view's own `menu(for:)`. target.rightClick() @@ -66,12 +64,4 @@ final class StructureColumnMoveUITests: UITestCase { "SQLite reorders by rebuilding, so a column has at least one direction it can move" ) } - - private func showStructure(in window: XCUIElement) { - let modePicker = window.radioGroups["results-view-mode-picker"].firstMatch - XCTAssertTrue(modePicker.waitToExist(timeout: 20), "The result must expose its view modes") - let structure = modePicker.radioButtons["Structure"].firstMatch - XCTAssertTrue(structure.waitToExist(timeout: 20), "Structure must be one of them") - structure.click() - } } diff --git a/TableProUITests/StructureConstraintsTabUITests.swift b/TableProUITests/StructureConstraintsTabUITests.swift index 1aecf1d6b..69e917888 100644 --- a/TableProUITests/StructureConstraintsTabUITests.swift +++ b/TableProUITests/StructureConstraintsTabUITests.swift @@ -17,7 +17,7 @@ final class StructureConstraintsTabUITests: UITestCase { XCTAssertTrue(row.waitToExist(timeout: 20), "The object browser must list Album") clickAtCenter(row) - showStructure(in: window) + showStructure(in: app, window: window) let constraints = subTab(named: "Constraints", in: window) XCTAssertTrue( @@ -38,14 +38,6 @@ final class StructureConstraintsTabUITests: UITestCase { (segment.value as? NSNumber)?.intValue == 1 } - private func showStructure(in window: XCUIElement) { - let modePicker = window.radioGroups["results-view-mode-picker"].firstMatch - XCTAssertTrue(modePicker.waitToExist(timeout: 20), "The result must expose its view modes") - let structure = modePicker.radioButtons["Structure"].firstMatch - XCTAssertTrue(structure.waitToExist(timeout: 20), "Structure must be one of them") - structure.click() - } - /// The sub-tab labels carry item counts, so they are matched by prefix rather than exactly. private func subTab(named name: String, in window: XCUIElement) -> XCUIElement { window.radioGroups["structure-tab-picker"].firstMatch diff --git a/TableProUITests/StructureRowMenuParityUITests.swift b/TableProUITests/StructureRowMenuParityUITests.swift index 8209af8c4..1c2b2ee73 100644 --- a/TableProUITests/StructureRowMenuParityUITests.swift +++ b/TableProUITests/StructureRowMenuParityUITests.swift @@ -28,7 +28,7 @@ final class StructureRowMenuParityUITests: UITestCase { XCTAssertTrue(row.waitToExist(timeout: 20), "The object browser must list Album") clickAtCenter(row) - showStructure(in: window) + showStructure(in: app, window: window) let grid = window.tables.matching(identifier: "data-grid").firstMatch XCTAssertTrue(grid.waitToExist(timeout: 30), "The structure editor must draw its column grid") XCTAssertTrue( @@ -43,9 +43,7 @@ final class StructureRowMenuParityUITests: UITestCase { "The grid must be laid out before a coordinate is taken off it" ) - /// A point offset from the grid, never a row or cell element: the grid's columns are - /// siblings of its rows and later in the tree, so XCUITest reads both as obscured. - let target = grid.coordinate(withNormalizedOffset: .zero).withOffset(CGVector(dx: 80, dy: 40)) + let target = gridPoint(in: grid, of: window, dy: 40) target.rightClick() assertStructureMenu(in: app, path: "an unselected column row") @@ -68,12 +66,4 @@ final class StructureRowMenuParityUITests: UITestCase { "\(path) must offer \(structureOnlyItem), which only the structure menu builds" ) } - - private func showStructure(in window: XCUIElement) { - let modePicker = window.radioGroups["results-view-mode-picker"].firstMatch - XCTAssertTrue(modePicker.waitToExist(timeout: 20), "The result must expose its view modes") - let structure = modePicker.radioButtons["Structure"].firstMatch - XCTAssertTrue(structure.waitToExist(timeout: 20), "Structure must be one of them") - structure.click() - } } diff --git a/TableProUITests/StructureTabIdentityUITests.swift b/TableProUITests/StructureTabIdentityUITests.swift index b299bdd2c..0fc3732c6 100644 --- a/TableProUITests/StructureTabIdentityUITests.swift +++ b/TableProUITests/StructureTabIdentityUITests.swift @@ -18,7 +18,7 @@ final class StructureTabIdentityUITests: UITestCase { XCTAssertTrue(row.waitToExist(timeout: 20), "The object browser must list Album") clickAtCenter(row) - showStructure(in: window) + showStructure(in: app, window: window) let indexes = subTab(named: "Indexes", in: window) XCTAssertTrue(indexes.waitToExist(timeout: 20), "The structure editor must offer Indexes") indexes.click() @@ -32,7 +32,7 @@ final class StructureTabIdentityUITests: UITestCase { XCTAssertTrue(openInNewTab.waitToExist(timeout: 15), "The sidebar must offer Open in New Tab") openInNewTab.click() - showStructure(in: window) + showStructure(in: app, window: window) let columns = subTab(named: "Columns", in: window) XCTAssertTrue(columns.waitToExist(timeout: 20), "The second tab must have its own structure editor") XCTAssertTrue( @@ -47,14 +47,6 @@ final class StructureTabIdentityUITests: UITestCase { (segment.value as? NSNumber)?.intValue == 1 } - private func showStructure(in window: XCUIElement) { - let modePicker = window.radioGroups["results-view-mode-picker"].firstMatch - XCTAssertTrue(modePicker.waitToExist(timeout: 20), "The result must expose its view modes") - let structure = modePicker.radioButtons["Structure"].firstMatch - XCTAssertTrue(structure.waitToExist(timeout: 20), "Structure must be one of them") - structure.click() - } - /// The sub-tab labels carry item counts, so they are matched by prefix rather than exactly. private func subTab(named name: String, in window: XCUIElement) -> XCUIElement { window.radioGroups["structure-tab-picker"].firstMatch diff --git a/TableProUITests/Support/UITestCase.swift b/TableProUITests/Support/UITestCase.swift index afc4861e7..e171eed92 100644 --- a/TableProUITests/Support/UITestCase.swift +++ b/TableProUITests/Support/UITestCase.swift @@ -182,6 +182,50 @@ internal class UITestCase: XCTestCase { waitForPredicate(timeout: timeout) { element.exists && element.isHittable } } + /// Switches the result to its Structure editor, through **View > Result View > Structure** + /// rather than the `Structure` segment of the results status bar. + /// + /// The segment cannot be clicked on the runner. Its screen is 1024x768, and a window that + /// wide cannot hold the sidebar, the detail pane at its minimum width and the row inspector + /// at once: the detail pane keeps its minimum and is drawn under the sidebar, taking the + /// leading half of the status bar with it. XCUITest still reports the segment as existing and + /// hittable, because its accessibility frame is where the layout says it is, so the click is + /// posted at (314, 691) and lands on the sidebar. Nothing fails there. The result stays on + /// Data, and the suite's next assertion reads the data grid as though it were the structure + /// grid, or waits out its timeout for a structure tab picker that was never going to appear. + /// (Run 33734073855, where the element tree captured the mode picker still reporting + /// `Data` selected after the click.) + /// + /// The menu item carries no geometry, so it is reachable whatever the window is doing. + /// Nothing probes for it first: XCUITest resolves a menu item by opening its parent, and a + /// probe that resolves it leaves that menu open, so the click's own traversal then fails with + /// "open menu during menu traversal" and waits out a ten second watchdog. Waiting on the menu + /// bar costs nothing and waiting for the tab picker afterwards is what makes the switch + /// observed rather than assumed. + internal func showStructure(in app: XCUIApplication, window: XCUIElement) { + let menuBar = app.menuBars.firstMatch + XCTAssertTrue(menuBar.waitToExist(timeout: 20), "The app must publish its menu bar") + menuBar.menuItems["Structure"].click() + XCTAssertTrue( + window.radioGroups["structure-tab-picker"].firstMatch.waitToExist(timeout: 30), + "The structure editor must open on the Structure result view" + ) + } + + /// A point inside the data grid that an overlapping pane cannot steal. + /// + /// A coordinate is the only way to click a row at all: the grid's columns are siblings of its + /// rows and later in the tree, so XCUITest reads every row and every cell as obscured and + /// refuses to click either. The grid's leading edge is not safe to measure from, though. On + /// the runner the detail pane is drawn under the sidebar, so a point 80pt in from that edge + /// lands on the object browser and a right-click raises its menu rather than the grid's. + /// Starting from whichever edge is further right keeps the point on the grid at any width. + internal func gridPoint(in grid: XCUIElement, of window: XCUIElement, dy: CGFloat) -> XCUICoordinate { + let clearOfBrowser = window.outlines.firstMatch.frame.maxX + 40 - grid.frame.minX + return grid.coordinate(withNormalizedOffset: .zero) + .withOffset(CGVector(dx: max(80, clearOfBrowser), dy: dy)) + } + /// The object browser draws its rows as hosted cells, so a row's name arrives as the static /// text's `value`, carrying the object kind the row reads out to VoiceOver, rather than as a /// label or an identifier. Matching on `value` is what finds them.