test(connections): cover reaching a saved connection without opening a window - #2624
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
#2620 shipped the grouped switcher with no UI automation, on the grounds that seeding a fixture
needed a launch hook the app does not have. That was half wrong, so here is the half that works.
Connections are seedable:
AppStorageEnvironmentredirects the Application Support root into thesandbox
UITestCaseprepares, andConnectionStorageadopts a store with no integrity tag besideit rather than refusing it, which is the path an install predating the tag takes. Writing
connections.jsoninto the sandbox before launch is enough.The test opens Switch Connection, activates a saved connection, and asserts the window count did
not change. That is what #1311 asked for and what #2097 promised, and nothing was covering it.
What this test does not cover, and why it cannot from here
The group the connections belong to is not seeded. Groups live in a
UserDefaultssuite, and thesuite name resolves per process container: the runner is sandboxed, the app is not, so the same
name is
<runner container>/Data/Library/Preferences/com.TablePro.uitest.plistfor the test and~/Library/Preferences/com.TablePro.uitest.plistfor the app. Both files exist on this machine.Anything the runner writes there, the app never reads.
ConnectionSwitcherSectionsTestscovers the grouping itself, and it covers it better than a UItest would: section order, the path title of a nested group, the accent colour, the collapse under
a filter, and a group whose connections are all open.
Two things found while writing it
Every search field in the app answers to
sidebar-filter.NativeSearchField.accessibilityIdentifierdefaults to that string, and 18 of its 30 call sitestake the default, including the switcher's own field, the database switcher, the welcome window and
several Settings panes. The captured element tree shows the switcher's field as
identifier: 'sidebar-filter', placeholderValue: 'Search connections'.This is not theoretical. The first version of this test waited on
app.searchFields.firstMatch,matched the object browser's filter field, and passed while asserting nothing: the panel it
meant to drive never had to open. It only came to light because the other assertion in the same
test failed for a real reason. This test now matches on the placeholder, the way
SwitcherEscapeUITestsalready did.UI test isolation for
UserDefaultsdoes not work.UITestCase.setUpWithErrorclearscom.TablePro.uitestso that "one test cannot read what thelast one wrote", but it clears the runner's copy while the app reads and writes its own. The app's
copy currently holds
com.TablePro.sample.openedCount => 229along with per-connection historypanel state, accumulated across every UI test run this machine has ever done. Every UI test shares
one growing defaults domain, and any test that depends on a clean one is silently reading the
previous run's.
Neither is fixed here. The first is a shared component and its 18 adopters; the second can only be
repaired from the app side, since a sandboxed runner cannot reach the domain the app uses, and that
means production code changed for a test. Both are worth their own change.
Verification
verify.sh generate: PASSverify.sh uitest GroupedConnectionSwitcherUITests: PASSswiftlint --stricton the new file: cleanNo CHANGELOG entry: nothing here changes what the app does.
https://claude.ai/code/session_01L81TaoPWxkLd15CGw2riPq