test(datagrid): make the JSON field's regression tests able to fail

TablePro/Views/Results/EditorAccessibilityIdentifier.swift

@@ -0,0 +1,30 @@
+//
+//  EditorAccessibilityIdentifier.swift
+//  TablePro
+//
+
+import AppKit
+import TableProEditorKit
+
+/// Names the editor's `NSTextView` for accessibility, and for the UI tests that reach it that way.
+///
+/// `.accessibilityIdentifier(...)` on the SwiftUI side names the representable, not the text view
+/// underneath it, so a query for that identifier matches nothing. `UITestCase.editorTextView` says
+/// so in as many words and keeps a `firstMatch` fallback because of it, which is only unambiguous
+/// while exactly one text view is on screen. The row inspector has several.
+///
+/// A coordinator is the documented way in: `prepareCoordinator` runs from `TextViewController`
+/// with the text view already built, which is what an identifier has to be set on.
+internal final class EditorAccessibilityIdentifier: TextViewCoordinator {
+    private let identifier: String
+
+    internal init(_ identifier: String) {
+        self.identifier = identifier
+    }
+
+    internal func prepareCoordinator(controller: TextViewController) {
+        controller.textView?.setAccessibilityIdentifier(identifier)
+    }
+
+    internal func destroy() {}
+}

TablePro/Views/Results/JSONCodeEditor.swift

@@ -19,20 +19,28 @@ internal struct JSONCodeEditor: View {
 
     @State private var editorState = SourceEditorState()
     @State private var configuration: SourceEditorConfiguration
+    /// Held in `@State` so the editor is handed the same coordinator on every update. It names the
+    /// text view itself, which is where an accessibility identifier has to land: the SwiftUI
+    /// modifier names the representable and never reaches it.
+    @State private var coordinators: [any TextViewCoordinator]
     @Environment(\.colorScheme) private var colorScheme
 
-    init(text: Binding<String>, isEditable: Bool) {
+    init(text: Binding<String>, isEditable: Bool, accessibilityIdentifier: String? = nil) {
         self._text = text
         self.isEditable = isEditable
         self._configuration = State(wrappedValue: Self.makeConfiguration(isEditable: isEditable))
+        self._coordinators = State(
+            wrappedValue: accessibilityIdentifier.map { [EditorAccessibilityIdentifier($0)] } ?? []
+        )
     }
 
     var body: some View {
         SourceEditor(
             $text,
             language: .json,
             configuration: configuration,
-            state: $editorState
+            state: $editorState,
+            coordinators: coordinators
         )
         .frame(maxWidth: .infinity, maxHeight: .infinity)
         .onChange(of: colorScheme) { _ in

TablePro/Views/RowInspector/FieldEditors/JsonEditorView.swift

@@ -40,11 +40,14 @@ internal struct JsonEditorView: View {
             range: ResizableFieldMetrics.jsonHeightRange,
             expandedHeight: isExpanded ? ResizableFieldMetrics.expandedHeight : nil
         ) {
-            JSONCodeEditor(text: editorText, isEditable: !context.isReadOnly)
+            JSONCodeEditor(
+                text: editorText,
+                isEditable: !context.isReadOnly,
+                accessibilityIdentifier: "inspector-json-field"
+            )
                 .clipShape(RoundedRectangle(cornerRadius: 5))
                 .overlay(RoundedRectangle(cornerRadius: 5).strokeBorder(Color(nsColor: .separatorColor)))
                 .overlay(alignment: .bottomTrailing) { actionButtons }
-                .accessibilityIdentifier("inspector-json-field")
         }
         /// `newValue`, never a re-read of `context.value.wrappedValue`. An `onChange` action
         /// closure belongs to the render that registered it, so its captured context is a render

TableProTests/Models/UI/JsonFieldEditingModelTests.swift

@@ -13,8 +13,8 @@ import Testing
 ///
 /// The editor and the store are two mirrors of one value, and the hang was a fixpoint between
 /// them: one keystroke produced 6,944 body evaluations in 30 seconds at 100% CPU, and the typed
-/// character was thrown away. `settle` is the shape that catches it: drive the pair until nothing
-/// moves, and fail if that takes more than a round.
+/// character was thrown away. What catches it is ``adoptsRestatement``: the store always answers
+/// with a different string than the editor holds, and adopting that answer is the bug.
 @MainActor
 @Suite("JSON field editing model")
 struct JsonFieldEditingModelTests {
@@ -29,26 +29,25 @@ struct JsonFieldEditingModelTests {
         return state
     }
 
-    /// Runs the editor and the store against each other the way SwiftUI does: the editor publishes,
-    /// the store restates, the editor is told what the store now holds, and round again. Returns
-    /// how many rounds it took to stop moving.
-    @discardableResult
-    private static func settle(
+    /// Whether the editor adopts the store's restatement of what the editor itself just published.
+    ///
+    /// This is the whole of #3051 in one question. `MultiRowEditState` stages a compacted form of
+    /// the text the editor shows, so the store always answers with a *different string* than the
+    /// editor holds. Adopting that answer is what overwrote the keystroke and pushed the overwrite
+    /// back, at about 75 rounds a second.
+    ///
+    /// Asserted directly rather than by looping the pair until it settles: `displayText` and
+    /// `lastSynced` are equal at every exit of both entry points, so a loop that re-publishes
+    /// `model.displayText` can never take a second turn and would report convergence for any model
+    /// at all, including one with the bug.
+    private static func adoptsRestatement(
         _ model: inout JsonFieldEditingModel,
-        against state: MultiRowEditState,
-        limit: Int = 8
-    ) -> Int {
-        for round in 1...limit {
-            let delivered = state.currentText(at: 0)
-            guard model.received(delivered) else { return round }
-            guard let published = model.typed(model.displayText) else { return round }
-            state.updateField(at: 0, value: published)
-        }
-        Issue.record("The editor and the store never agreed within \(limit) rounds")
-        return limit
+        from state: MultiRowEditState
+    ) -> Bool {
+        model.received(state.currentText(at: 0))
     }
 
-    @Test("A keystroke survives the round trip and settles in one round")
+    @Test("A keystroke survives the round trip and the store's echo is refused")
     func keystrokeSettles() {
         let original = "{\"a\":1}"
         let state = Self.makeState(value: original, type: .json(rawType: "jsonb"))
@@ -60,15 +59,15 @@ struct JsonFieldEditingModelTests {
         #expect(published == typed)
         state.updateField(at: 0, value: published)
 
-        let rounds = Self.settle(&model, against: state)
-        #expect(rounds == 1)
+        let adopted = Self.adoptsRestatement(&model, from: state)
+        #expect(!adopted, "the store answers with the compact form of what was just typed")
         #expect(model.displayText == typed, "the editor keeps its layout, not the compact echo")
         #expect(state.fields[0].pendingValue == "{\"a\":2}")
     }
 
     /// The document is invalid for as long as the user is halfway through typing a value, which is
     /// where `JsonReindenter`'s parse-failure fallback used to make the two sides disagree.
-    @Test("A transiently invalid document settles at every step")
+    @Test("A transiently invalid document keeps every keystroke")
     func invalidIntermediateSettles() {
         let original = "{\"a\": 1}"
         let state = Self.makeState(value: original, type: .text(rawType: "text"))
@@ -78,15 +77,15 @@ struct JsonFieldEditingModelTests {
             if let published = model.typed(typed) {
                 state.updateField(at: 0, value: published)
             }
-            let rounds = Self.settle(&model, against: state)
-            #expect(rounds == 1, "stalled on \(typed)")
+            let adopted = Self.adoptsRestatement(&model, from: state)
+            #expect(!adopted, "the store's restatement was adopted on \(typed)")
             #expect(model.displayText == typed, "the keystroke must survive")
         }
     }
 
     /// The store drops a pending value that matches the stored one again, so the binding answers
     /// with the original. That is still this editor's own echo and must not be adopted.
-    @Test("Editing back to the stored value settles rather than bouncing")
+    @Test("Editing back to the stored value does not bounce")
     func revertToOriginalSettles() {
         let original = "{\"a\":1}"
         let state = Self.makeState(value: original, type: .json(rawType: "jsonb"))
@@ -97,8 +96,10 @@ struct JsonFieldEditingModelTests {
 
         state.updateField(at: 0, value: model.typed(JsonReindenter.reindent(original)))
         #expect(!state.fields[0].hasEdit)
-        let rounds = Self.settle(&model, against: state)
-        #expect(rounds == 1)
+        /// The store dropped the pending value, so it now answers with `originalValue`. That is
+        /// still this editor's own write coming back and must not be adopted.
+        let adopted = Self.adoptsRestatement(&model, from: state)
+        #expect(!adopted)
     }
 
     @Test("The editor's own write is never adopted back")
@@ -112,12 +113,16 @@ struct JsonFieldEditingModelTests {
         #expect(model.displayText == typed, "the caret's layout must not be rewritten")
     }
 
-    @Test("A value the editor did not write is adopted")
+    @Test("A value the editor did not write is adopted, and is not published back")
     func externalChangeIsAdopted() {
         var model = JsonFieldEditingModel(storedValue: "{\"a\":1}")
         let adopted = model.received("{\"b\":2}")
         #expect(adopted)
         #expect(model.displayText == JsonReindenter.reindent("{\"b\":2}"))
+        /// The adopted value arrived compact and is displayed laid out, so the two differ by more
+        /// than nothing. Republishing it would stage an edit the user never made.
+        let republished = model.typed(model.displayText)
+        #expect(republished == nil, "adopting is not an edit")
     }
 
     /// Set NULL, Set DEFAULT and Set EMPTY all empty the field's editable text. Adopting that and

TableProUITests/RowInspector/InspectorJsonFieldEditUITests.swift

@@ -2,11 +2,11 @@
 //  InspectorJsonFieldEditUITests.swift
 //  TableProUITests
 //
-//  The row inspector's JSON field kept two mirrors of one value in two textual forms and
-//  reconciled them from an `onChange` action that re-read a context a render out of date. Typing
-//  one character made the editor put the previous value back and push it into the store again, at
-//  about 75 rounds a second: one keystroke produced 6,944 body evaluations in 30 seconds at 100%
-//  CPU, and the character was thrown away (#3051).
+//  The row inspector's JSON field kept two mirrors of one value and reconciled them from an
+//  `onChange` action that re-read a context a render out of date. Typing one character made the
+//  editor put the previous value back and push it into the store again, at about 75 rounds a
+//  second: one keystroke produced 6,944 body evaluations in 30 seconds at 100% CPU, and the
+//  character was thrown away (#3051).
 //
 //  So the assertion is not "the app is responsive", which is hard to state and easy to pass by
 //  accident. It is that the character is still there afterwards, which the loop could not manage.
@@ -26,16 +26,15 @@ final class InspectorJsonFieldEditUITests: UITestCase {
         let app = try launchWithSampleDatabase(environment: [jsonFixtureVariable: "1"])
         let window = app.windows.firstMatch
 
-        try openFixtureTable(in: app)
+        openFixtureTable(in: app)
         showInspector(in: app)
 
         let grid = window.tables.matching(identifier: "data-grid").firstMatch
         XCTAssertTrue(grid.waitToExist(timeout: 30), "The fixture table must produce a result grid")
-        XCTAssertTrue(
-            waitForPredicate(timeout: 30) { grid.tableRows.firstMatch.exists },
-            "The fixture table must return a row"
-        )
-        gridPoint(in: grid, of: window, dy: 12).click()
+        XCTAssertTrue(waitForClickableRows(in: grid), "The fixture table must return a row")
+        /// Past the header, which is 28pt with no column comments, and inside the first row at
+        /// every row height the setting offers.
+        gridPoint(in: grid, of: window, dy: 40).click()
 
         let field = window.textViews.matching(identifier: "inspector-json-field").firstMatch
         XCTAssertTrue(field.waitToExist(timeout: 20), "The JSON column must open the JSON editor")
@@ -47,7 +46,7 @@ final class InspectorJsonFieldEditUITests: UITestCase {
         app.typeText(typedMarker)
 
         XCTAssertTrue(
-            waitForPredicate(timeout: 20) { (field.value as? String)?.contains(self.typedMarker) == true },
+            waitForPredicate(timeout: 20) { (field.value as? String)?.contains(self.typedMarker) ?? false },
             "Every typed character must survive; the editor holds '\(field.value as? String ?? "nil")'"
         )
 
@@ -64,16 +63,22 @@ final class InspectorJsonFieldEditUITests: UITestCase {
 
     // MARK: - Helpers
 
-    private func openFixtureTable(in app: XCUIApplication) throws {
-        let browser = app.windows.firstMatch.outlines.firstMatch
-        XCTAssertTrue(browser.waitToExist(timeout: 30), "The object browser must list the sample's tables")
+    /// The object browser draws its rows as hosted cells, so the name arrives as the static text's
+    /// `value` behind the object kind, and the row takes a coordinate click rather than an
+    /// element one. `objectBrowserRow` carries both facts.
+    private func openFixtureTable(in app: XCUIApplication) {
+        let window = app.windows.firstMatch
+        XCTAssertTrue(
+            window.outlines.firstMatch.waitToExist(timeout: 30),
+            "The object browser must list the sample's tables"
+        )
 
-        let table = browser.staticTexts[fixtureTable].firstMatch
+        let row = objectBrowserRow(fixtureTable, in: window)
         XCTAssertTrue(
-            table.waitToExist(timeout: 30),
+            row.waitToExist(timeout: 30),
             "The seeded fixture table must appear in the object browser"
         )
-        table.doubleClick()
+        row.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)).doubleClick()
     }
 
     /// The inspector remembers whether it was open, so the starting state is whatever the previous