test: repair test target after datagrid refactors#920
Closed
test: repair test target after datagrid refactors#920
Conversation
This was referenced Apr 28, 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.
Summary
Repairs the
TableProTeststarget so it compiles and runs again. The target had been broken across PRs #914, #915, #917, #918 (datagrid refactors), and surfacing those compile errors also exposed a pre-existing dialect-quoting issue inSQLStatementGenerator. Three focused commits, scoped narrowly to unblock other PRs (including the upcoming raycast-integration work).In scope (this PR)
1. Compile fix for 4 test files —
7868fce6. The datagrid refactors renamed APIs but missed:MainStatusBarLayoutTests.swift—tab:arg replaced withsnapshot: StatusBarSnapshot(tab: nil)(refactor(datagrid): unify cell edit path, class-based RowProvider cache #918).CommandActionsDispatchTests.swift— removedselectedRowIndices:binding param, passselectionState:(refactor: move selectedRowIndices to @Observable GridSelectionState on coordinator #914).SaveCompletionTests.swift— coordinator row ops no longer takeselectedRowIndices: inout, usecoordinator.selectionState.indices(refactor: move selectedRowIndices to @Observable GridSelectionState on coordinator #914).AnyChangeManagerTests.swift—AnyChangeManager(dataManager:)andAnyChangeManager(structureManager:)collapsed intoAnyChangeManager(_ manager: any ChangeManaging)(refactor: replace AnyChangeManager closure erasure with ChangeManaging protocol #915);wrapper.changesis nowwrapper.rowChanges.2. Source-side dialect derivation —
27e63c47. Once the test target compiled,SQLStatementGenerator*Tests(51 cases) ran for the first time and 100% failed because tests construct the generator withdialect: niland assert on identifier quoting.quoteIdentifierFromDialect(nil)returns identity. AddedresolveSQLDialect(for:explicit:)inDialectQuoteHelper.swiftthat falls back toPluginMetadataRegistry.shared.snapshot(forTypeId:)?.editor.sqlDialectwhen no explicit dialect is supplied, and wiredSQLStatementGenerator.initthrough it. Production behavior is unchanged for callers that already pass a dialect; production code that previously had to look up the dialect manually still can. Result:SQLStatementGeneratorTests49/51 green (the 2 remaining are LIMIT 1 / TOP 1 — see follow-ups).3.
FakeTableOperationProvidertest seam —57a397fb. ThreeSaveCompletionTestsexited via the empty-statements error path becausecurrentPluginDriverAdapteris nil in tests (no live driver session). Extracted aTableOperationStatementProviderprotocol covering the four methodsMainContentCoordinator+TableOperations.swiftcalls (truncateTableStatements,dropObjectStatement,foreignKeyDisableStatements,foreignKeyEnableStatements);PluginDriverAdapteradopts it via empty extension. AddedtableOperationOverride: TableOperationStatementProvider?onMainContentCoordinator(test-only seam, never assigned in production).FakeTableOperationProviderreturns ANSI-style SQL. Result:SaveCompletionTests13/13 green.Verification
All 100% green except the 2 LIMIT-1 cases (see follow-ups). swiftlint --strict clean on all touched files.
Known pre-existing failures (NOT in this PR)
After the test target started compiling and the dialect fix landed, the full target run reports approximately 280 pre-existing failures previously masked by compile errors. These are out of scope; each warrants its own ticket. Buckets:
LIMIT 1/TOP (1)/ALTER TABLE ... DELETE WHEREcodegen. No such codegen exists inTablePro/Core/ChangeTracking/SQLStatementGenerator.swift. Either the safety-limit feature was removed and tests weren't deleted, or it was never implemented. Affected:SQLStatementGeneratorTests(2),SQLStatementGeneratorNoPKTests,SQLStatementGeneratorMSSQLTests,DataChangeManagerClickHouseTests.<external symbol>under parallel test execution. Tests touchDatabaseManager.shared/PluginManager.shared/ global state and race when xcodebuild spawns multiple test host processes. Fix is either-parallel-testing-enabled NOor removing shared-state coupling. Affected:RowProviderSyncTests,SidebarFieldTests,JSONEditorHighlightTests,EvictionTests,BlobFormattingTests,ColumnTypeBadgeLabelTests, others.DatabaseTypeCassandraTests/scylladbIconName(scylladb-iconvscassandra-icon),SafeModeLevelTests/iconNames(lock.open.fillvslock.open),ConnectionURLFormatterTestsMariaDB scheme,ConnectionSharingTestsPrivate KeyvsprivateKey. Tests didn't track UI/serialization renames.SSHConfigParserTests/testSSHTokensInIdentityFile,SSHConfigurationTests/testTildeExpansionWithSubpath. Real product bug: tilde expansion produces double-slashes ($HOME//.ssh/...).GroupStorageTests,EtcdQueryBuilderTests/emptyPrefix,DataChangeManagerExtendedTests/Full undo/redo chain. Need individual investigation per test.TableQueryBuilderMSSQLTestsproducesLIMIT 200 OFFSET 0(PostgreSQL-style) instead ofOFFSET ... FETCH NEXT. Plugin not registered in tests, so dialect lookup falls back to defaults.I have not opened separate tickets for these — happy to do so if the team wants, but they would need product-owner triage first (especially bucket 1, where the right answer might be deleting the tests rather than building features).
Test plan
SaveCompletionTests— 13/13 greenCommandActionsDispatchTests— greenMainStatusBarLayoutTests— greenAnyChangeManagerTests— greenSQLStatementGeneratorTests— 49/51 green (2 LIMIT-1 are out-of-scope)PluginManager.shared.sqlDialect(for:))