Skip to content

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

Merged
datlechin merged 1 commit into
mainfrom
fix/3051-test-gaps
Sep 23, 2026
Merged

datlechin merged 1 commit into
mainfrom
fix/3051-test-gaps

Conversation

@datlechin

Copy link
Copy Markdown
Member

Follow-up to #3056, which fixed #3051. The fix is correct. Its tests were not.

An independent review of the merged diff found three defects, all in the tests, and all of them meaning the suite could not fail if the fix were reverted.

The regression suite asserted a tautology

JsonFieldEditingModelTests.settle() drove the editor model against a real MultiRowEditState "until nothing moves" and asserted rounds == 1.

JsonFieldEditingModel keeps displayText and lastSynced byte-identical at every exit of both mutating entry points: init assigns the same string to each, typed either returns nil at its guard or sets lastSynced = text, and received ends lastSynced = displayText. So model.typed(model.displayText) always hit the guard and returned nil, the loop always returned on its first pass, Issue.record for non-convergence was unreachable, and #expect(rounds == 1) held for any model, including one with the bug.

Replaced with adoptsRestatement, which asserts the invariant the fix actually rests on: MultiRowEditState stages a compacted form of the text the editor shows, so the store always answers with a different string, and adopting that answer is the bug. That fails if received stops normalizing or loses its guard. A comment records why the loop shape was wrong so it does not come back.

The UI test could never find the JSON editor

.accessibilityIdentifier("inspector-json-field") was applied to JSONCodeEditor, whose body is a SourceEditor, an NSViewControllerRepresentable. That names the representable, not the NSTextView underneath it. The repo had already measured this and written it down: UITestCase.editorTextView keeps a firstMatch fallback for sql-editor-textview and says the identified lookup "should start working the day the identifier reaches the text view".

This makes that day today. EditorAccessibilityIdentifier is a TextViewCoordinator that sets the identifier on the text view from prepareCoordinator, which TextViewController.init calls immediately after building it. JSONCodeEditor takes an optional identifier and holds the coordinator in @State so the same instance is handed over on every update.

The UI test could never find the fixture table, and clicked the header

browser.staticTexts["json_fixture"] matched nothing: object browser rows are hosted cells that publish the name as the static text's value behind the object kind, so the element reads Table: json_fixture. UITestCase.objectBrowserRow encodes that and five suites use it; this one hand-rolled a different lookup. The grid click also used dy: 12, which lands on the 28pt header and selects no row.

Now uses objectBrowserRow, a coordinate double-click, waitForClickableRows (which carries the non-empty-frame guard), and dy: 40.

Proof that the test now constrains the fix

I reintroduced both measured root causes into the working tree and re-ran the UI test:

tree result
onChange re-reading the captured context, and the binding getter back to the per-render snapshot FAIL at InspectorJsonFieldEditUITests.swift:48, "Every typed character must survive", with the editor holding the document without the typed marker (31.4s)
fix in place PASS (12.6s)

Both defects had to go back together: with the binding getter already reading the store, re-reading the context returns the current value and the loop cannot form. That is the #3051 symptom reproduced under the test and caught by it.

Verification

build PASS. test 126/126 PASS. uitest InspectorJsonFieldEditUITests 2/2 PASS, confirmed from the log rather than the count. swiftlint --strict clean on every changed file.

Separately, and unrelated to this PR: the AllPlugins aggregate passes on the merged fix, and a security review of it found nothing at confidence 8 or above, including on UITestJsonFixture, which ships in the release binary and runs sqlite3_exec on interpolated SQL. Its isIsolated gate is reachable only through TABLEPRO_UI_TEST_SANDBOX, the SQL carries only compile-time constants, and sqlite3_open_v2 is called without SQLITE_OPEN_CREATE.

@datlechin
datlechin merged commit 3f827b0 into main Sep 23, 2026
8 checks passed
@datlechin
datlechin deleted the fix/3051-test-gaps branch September 23, 2026 02:57
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.

Main thread spins in a SwiftUI layout/update loop while typing in the row inspector's JSON field editor

1 participant