Fix iOS UI test auth-screen flake (#276)#277
Merged
Merged
Conversation
Working hypothesis: while AppViewModel.isLoading == true, the outer ZStack in RootView only contained two static Text views (the brand lockup), and SwiftUI did not surface that ZStack as a queryable otherElements match. The UI tests query app.otherElements["root.currentView.auth"] with a 30s wait — when the auth-target tests ran (seedAuth=false), the accessibility node never appeared and every test except the very first seedAuth=true smoke test failed. Changes: - RootView: add .accessibilityElement(children: .contain) to the outer ZStack so the root.currentView.* identifier is always queryable as an otherElements match, regardless of which branch (loading/auth/home) is currently rendered. - APIClient.init: switch [E2E-DIAG] from print() to os_log via a nonisolated static Logger. print() from the app process is not captured by Xcode UI test xcresult bundles, which is why the diagnostic added in PR #261 never showed up in CI logs. - APIClient.init: also extract persistence into a nonisolated static helper Self.persist(store:key:) so init no longer calls the actor-isolated instance method, killing the two Swift 6 actor- isolation warnings on lines 52 and 60. - APIClient: clearSwiftDataPersistentStore() removes ~/Library/Application Support/default.store{,-shm,-wal} on SP_UI_TEST_RESET_STORE=1. SwiftData was the one piece of persistent state the previous reset path missed (issue #266 wiped UserDefaults, Keychain, cookies, URL credentials, AudioEngine prefs — but not the SwiftData container). - AppViewModel.checkAuth: same os_log conversion for the [E2E-DIAG] checkAuth.done line, gated on SP_UI_TEST_MODE=1 to avoid PII leakage in production logs. Refs #276, #266, PR #261. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces ad-hoc Changes
Sequence Diagram(s)sequenceDiagram
participant Test as UI Test Runner
participant App as StillPointApp (init)
participant API as APIClient
participant FS as FileSystem / UserDefaults
participant VM as AppViewModel
Test->>App: launch()
App->>FS: read SP_UI_TEST_MODE / SP_UI_TEST_RESET_STORE
alt reset enabled
App->>FS: remove SwiftData files (default.store, -shm, -wal) (best-effort)
end
App->>API: initialize APIClient (UI-test path)
API->>API: decode/create resolvedStore locally
API->>FS: persist(resolvedStore, key:) [nonisolated helper]
API->>VM: assign uiTestStore / finish init
VM->>VM: run auth-check -> publish currentView (auth/home)
Test->>App: query root.currentView.auth (accessibility marker present)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
codeant-ai
Bot
added
the
size:M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ios/StillPointShared/Sources/StillPointShared/APIClient.swift (1)
82-91: Avoid silently swallowing SwiftData store wipe failures.
try?on delete makes reset failures invisible, which can hide the exact CI flake this PR is targeting. Please log non-file-not-foundremoval errors.Suggested patch
private static func clearSwiftDataPersistentStore() { let fm = FileManager.default guard let appSupport = try? fm.url(for: .applicationSupportDirectory, in: .userDomainMask, appropriateFor: nil, create: false) else { return } for suffix in ["", "-shm", "-wal"] { let url = appSupport.appendingPathComponent("default.store\(suffix)") - try? fm.removeItem(at: url) + do { + try fm.removeItem(at: url) + } catch CocoaError.fileNoSuchFile { + continue + } catch { + Self.diagLog.error("[E2E-DIAG] failed to remove SwiftData file path=\(url.path, privacy: .public) error=\(error.localizedDescription, privacy: .public)") + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointShared/Sources/StillPointShared/APIClient.swift` around lines 82 - 91, The clearSwiftDataPersistentStore currently swallows all removal errors with try?, so replace that with a do-catch around FileManager.removeItem(at:) inside the loop in clearSwiftDataPersistentStore and log any errors that are not "file not found": catch the thrown error (from FileManager.default.removeItem) and if (error as NSError).code != NSFileNoSuchFileError then emit a diagnostic (e.g., print/processLogger/error reporting) including the URL and error; otherwise ignore the missing-file case. This preserves ignoring benign missing-store files but surfaces real removal failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ios/StillPointShared/Sources/StillPointShared/APIClient.swift`:
- Around line 82-91: The clearSwiftDataPersistentStore currently swallows all
removal errors with try?, so replace that with a do-catch around
FileManager.removeItem(at:) inside the loop in clearSwiftDataPersistentStore and
log any errors that are not "file not found": catch the thrown error (from
FileManager.default.removeItem) and if (error as NSError).code !=
NSFileNoSuchFileError then emit a diagnostic (e.g., print/processLogger/error
reporting) including the URL and error; otherwise ignore the missing-file case.
This preserves ignoring benign missing-store files but surfaces real removal
failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 548a517d-b614-4048-b1c6-546d00fcb54e
📒 Files selected for processing (3)
ios/StillPointApp/ViewModels/AppViewModel.swiftios/StillPointApp/Views/RootView.swiftios/StillPointShared/Sources/StillPointShared/APIClient.swift
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
CodeAnt AI finished reviewing your PR. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit de5e4e8. Configure here.
Owner
Author
|
@graphite-app re-review |
Round-1 didn't move the smoke needle and the SwiftData wipe ran too
late. Two corrections:
1. Move the SwiftData wipe to StillPointApp.init(), before the
.modelContainer(...) WindowGroup modifier opens SQLite. Both CodeAnt
and Cursor flagged that wiping from APIClient.init() (which fires
lazily from RootView.task { checkAuth() }) deletes files that the
ModelContainer has already mapped — Unix file handles keep the
stale rows alive in memory. Removing the now-dead
clearSwiftDataPersistentStore() helper from APIClient.
2. Restructure RootView to ALWAYS render the current-view branch and
show the brand-lockup as an overlay on top during loading, instead
of gating the branch behind `if isLoading`. Round-1's
.accessibilityElement(children: .contain) on the ZStack didn't
surface root.currentView.auth as an otherElements match (smoke test
still timed out at 30s) and would have flattened interactive
children — wrong on both counts. Always-rendering AuthView gives the
accessibility tree real interactive content from t=0, so
`app.otherElements["root.currentView.auth"]` resolves immediately
regardless of where checkAuth is in its lifecycle. The loading
overlay is .accessibilityHidden(true) so it doesn't compete in the
tree.
Refs #276, addresses CodeAnt Major + Cursor Medium on PR #277.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/StillPointApp/StillPointApp.swift`:
- Around line 31-34: The guard currently checks SP_UI_TEST_MODE only for "1"
while SP_UI_TEST_RESET_STORE accepts multiple truthy strings; update the check
so both keys are validated the same way by lowercasing env["SP_UI_TEST_MODE"]
and testing it against the same truthy set (e.g. ["1","true","yes","on"]) before
proceeding with the SwiftData reset; locate the check in StillPointApp.swift
that references ProcessInfo.processInfo.environment and replace the
single-string comparison with the contains(...) pattern used for
SP_UI_TEST_RESET_STORE.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5220fdbb-17cc-4f6f-9a09-b4ddf4ab7834
📒 Files selected for processing (3)
ios/StillPointApp/StillPointApp.swiftios/StillPointApp/Views/RootView.swiftios/StillPointShared/Sources/StillPointShared/APIClient.swift
Round 2 (always-render the current-view branch) didn't move the smoke needle: root.currentView.auth still timed out at 30s on iOS 26 even with AuthView mounted from t=0. The defect is upstream — applying .accessibilityIdentifier() to a SwiftUI ZStack does not reliably promote that node to an XCUIElementType.other match for app.otherElements[...] queries on iOS 26. Switching to an explicit, full-screen, invisible Color.clear overlay with .accessibilityElement() and the root.currentView.<slug> identifier. Color.clear with an explicit accessibility-element promo is purpose-built and surfaces predictably as a dedicated .other node, independent of child-coalescing heuristics in the SwiftUI->UIKit a11y bridge. The marker is full-screen, invisible, and .allowsHitTesting(false) so it does not interfere with the rest of the view tree (auth.emailField, home.beginButton, etc. remain queryable through their own AuthView/MainTabView accessibility trees). Also addresses CR Minor on PR #277: aligns SP_UI_TEST_MODE / SP_UI_TEST_RESET_STORE parsing with the rest of the UI-test gates by factoring out a shared `truthy(_:)` closure that accepts 1/true/yes/on (case-insensitive). Previously SP_UI_TEST_MODE only accepted "1" while UITestConfig.fromProcessInfo accepted any of those forms, which could quietly skip the SwiftData reset for valid runs. Refs #276, addresses CodeRabbit Minor on PR #277. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Owner
Author
|
@graphite-app re-review |
Round 3 fixed *existence* of root.currentView.auth — the test now
finds it at t=24s and finds auth.emailField at t=25s. But the tap
fails with "Computed hit point {-1, -1} after scrolling to visible —
Not hittable", because the always-mounted full-screen SPColor.bg
loading overlay participates in hit testing and swallows the tap
before it can reach AuthView's TextField.
`.accessibilityHidden(true)` removes the overlay from the a11y tree
but does NOT disable hit testing — those are independent in SwiftUI.
Adding `.allowsHitTesting(false)` to the loading overlay branch lets
taps fall through to the AuthView/MainTabView layer underneath, while
keeping the brand-lockup paint visible.
Critical-suite progress on round 3 (HEAD 21f72e5): 5/10 passing
(testHistoryAndSettingsNavigationSmoke, testLaunchOffline*,
testTokenExpiry*, testSessionsFailure*, testVoiceOver*). Remaining 5
all fail with the same {-1,-1} hit-point pattern. This change should
unblock them.
Refs #276, PR #277.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
CodeAnt AI is running Incremental review |
codeant-ai
Bot
added
size:L
|
CodeAnt AI Incremental review completed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/StillPointApp/StillPointApp.swift`:
- Around line 40-48: The current cleanup loop using try? swallows all removal
errors; replace the silent attempts in the loop that uses fm, appSupport and url
(the appSupport.appendingPathComponent("default.store\(suffix)") block) with a
do-catch around fm.removeItem(at: url), and in the catch check the error (cast
to NSError) to ignore only the "file not found" / NSFileNoSuchFileError case
while logging or rethrowing any other errors via your logger (or print/OSLog) so
unexpected failures are surfaced during UI-test resets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31404d27-d389-48a2-a1cc-f45765a9cf82
📒 Files selected for processing (2)
ios/StillPointApp/StillPointApp.swiftios/StillPointApp/Views/RootView.swift
Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
|
CodeAnt AI is running Incremental review |
codeant-ai
Bot
added
size:L
|
CodeAnt AI Incremental review completed. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-ios.yml:
- Around line 68-70: The current log export predicate is too broad and pulls all
logs for subsystem "com.brettonauerbach.stillpoint"; update the --predicate used
in the log show invocation so it filters to the e2e-diag category (e.g. require
category == "e2e-diag" in addition to or instead of the subsystem) while still
keeping the existing eventMessage CONTAINS "[E2E-DIAG]" fallback; modify the
--predicate string in the log show command in .github/workflows/e2e-ios.yml to
include category == "e2e-diag" (for example combine with AND) so only UI-test
diagnostic logs are exported.
In `@ios/StillPointApp/StillPointApp.swift`:
- Around line 44-47: The guard-let using try? on
FileManager.url(for:in:appropriateFor:create:) (the fm call that assigns
appSupport) swallows all errors; replace it with a do/catch so you can
distinguish "not found" from other errors, and when the lookup throws an
unexpected error call your logger (or print) with a clear message before
returning; specifically, in the catch inspect the error (as NSError) and if it's
NSFileNoSuchFileError/NSCocoaErrorDomain return silently, otherwise log the
unexpected error and then return.
In `@ios/StillPointApp/Views/RootView.swift`:
- Around line 102-109: The overlayed AccessibilityMarkerView (identifier
"root.currentView.\(viewAccessibilitySlug)") is always rendered which exposes a
test-only accessibility element to VoiceOver users; wrap the overlay so it only
renders when the SP_UI_TEST_MODE flag is enabled (e.g. check SP_UI_TEST_MODE
before creating AccessibilityMarkerView), preserving the existing
frame/allowsHitTesting modifiers and identifier/value usage in RootView.swift so
the marker remains available during UI test runs but absent for normal
accessibility users.
In `@ios/StillPointAppUITests/StillPointAppUITests.swift`:
- Around line 299-304: The helper waitForRoot currently calls
assertColdStartBound even when root.waitForExistence(times out), causing extra
waits and a second failure; change waitForRoot to short-circuit immediately when
root.waitForExistence(timeout:) returns false (e.g., call XCTFail or return
early) and skip calling assertColdStartBound so only the original missing-root
failure is reported; update references to root, waitForExistence, and
assertColdStartBound in waitForRoot to implement this early-return behavior.
- Around line 4-7: coldStartMaxMs was set to 45_000 which conflates XCTest's
launchTimeout with the app's internal auth-check SLA; change coldStartMaxMs to a
realistic auth-check budget (e.g., 4_500 or 5_000) so comparisons against
coldStartAuthCheckMs enforce a true auth latency SLA, and add a short comment
near launchTimeout and coldStartMaxMs clarifying that launchTimeout is an XCTest
wait budget while coldStartMaxMs is the app-measured auth-check SLA (referencing
launchTimeout, coldStartMaxMs, and coldStartAuthCheckMs).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ad17ea38-b62b-4787-8e59-a1ff607888b7
📒 Files selected for processing (5)
.github/workflows/e2e-ios.ymlios/StillPointApp/StillPointApp.swiftios/StillPointApp/Theme/DesignTokens.swiftios/StillPointApp/Views/RootView.swiftios/StillPointAppUITests/StillPointAppUITests.swift
Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
codeant-ai
Bot
added
the
size:L
|
CodeAnt AI Incremental review completed. |
3 tasks
This was referenced Apr 30, 2026
docs(ios-ui-tests): document terminate-after-navigation race + when to use direct tap vs helper
#320
Closed
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:L
Sequence DiagramThis diagram shows how the app now initializes in UI test mode, resets SwiftData before opening the model container, runs the auth check, and exposes a stable accessibility marker and cold-start metric that the UI tests wait on. sequenceDiagram
participant UITest
participant App
participant RootView
participant APIClient
UITest->>App: Launch with UI test environment flags
App->>App: Reset SwiftData store when reset flag is set
App->>RootView: Create RootView and start auth check task
RootView->>APIClient: Call auth check using me
APIClient-->>RootView: Return user result and log diagnostics
RootView-->>UITest: Expose root currentView marker with coldStart metric for waits and assertions
Generated by CodeAnt AI |
| } | ||
|
|
||
| private var controlsShouldBeVisible: Bool { | ||
| vm.controlsVisible || !vm.isActive || verticalSizeClass == .compact |
There was a problem hiding this comment.
Suggestion: This visibility rule makes the control panel show whenever the session is not active, including completed/abandoned states. That exposes controls like pause/end/abandon after completion and can drive invalid state transitions while completion save/navigation is in flight. Restrict visibility to valid session states (for example, active/paused or explicit pre-start cases) rather than all !isActive states. [possible bug]
Severity Level: Major ⚠️
- ❌ Session abandonment can still persist data via saveSession.
- ⚠️ Navigation may jump from Home to Completion unexpectedly.
- ⚠️ Control panel visible in invalid post-completion states.Steps of Reproduction ✅
1. From the home screen, tap the "Begin" button in `HomeView`
(`ios/StillPointApp/Views/HomeView.swift:15-21`), which calls `appVM.beginSession()`
(`ios/StillPointApp/ViewModels/AppViewModel.swift:151-153`) and switches `currentView` to
`.session`, causing `RootView` to present `SessionView`
(`ios/StillPointApp/Views/RootView.swift:21-28`).
2. In `SessionView`, `onAppear` starts the session timer by calling `vm.start()`
(`ios/StillPointApp/Views/SessionView.swift:107-109`), which marks `vm.isActive = true`
and schedules the control-hide timer (`SessionViewModel.start` and `scheduleControlHide`,
`ios/StillPointApp/ViewModels/SessionViewModel.swift:92-118,63-72`).
3. End the session either by letting the timer run out (natural completion via `tick()`,
which sets `isActive = false` and `isComplete = true`,
`ios/StillPointApp/ViewModels/SessionViewModel.swift:13-20,23-25`) or by tapping "End
Early" in the control panel (`SessionView.endEarlyButton` calls `vm.endEarly()`,
`ios/StillPointApp/Views/SessionView.swift:342-355`, implemented at
`ios/StillPointApp/ViewModels/SessionViewModel.swift:193-199`). This flip of
`vm.isComplete` triggers `onChange(of: vm.isComplete)`
(`ios/StillPointApp/Views/SessionView.swift:110-113`), which calls `handleCompletion()`
(`ios/StillPointApp/Views/SessionView.swift:28-44`) to asynchronously run
`vm.saveSession(...)` (`ios/StillPointApp/ViewModels/SessionViewModel.swift:211-256`) and,
on success, `appVM.completeSession(...)`
(`ios/StillPointApp/ViewModels/AppViewModel.swift:184-200`). During the network `await`,
`appVM.currentView` remains `.session`.
4. Immediately after completion, while `saveSession` is still in flight, note that
`vm.isActive` is now `false`, so `controlsShouldBeVisible`
(`ios/StillPointApp/Views/SessionView.swift:171-173`) evaluates to `true` purely because
of the `!vm.isActive` clause, keeping `controlPanel` visible with its "Abandon" button
(`ios/StillPointApp/Views/SessionView.swift:373-387`). Tapping "Abandon" now executes
`vm.abandon()` (`ios/StillPointApp/ViewModels/SessionViewModel.swift:202-209`, which sets
`isAbandoned = true`, `isComplete = true`) and then `appVM.returnHome()`
(`ios/StillPointApp/ViewModels/AppViewModel.swift:202-208`), navigating to `.home` even
though the earlier `handleCompletion` task will still eventually call
`appVM.completeSession(...)`. This produces a window where a supposedly abandoned,
post-completion session is still saved and may navigate to the completion screen after
briefly returning home, demonstrating that exposing the control panel whenever
`!vm.isActive` allows invalid post-completion actions.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ios/StillPointApp/Views/SessionView.swift
**Line:** 172:172
**Comment:**
*Possible Bug: This visibility rule makes the control panel show whenever the session is not active, including completed/abandoned states. That exposes controls like pause/end/abandon after completion and can drive invalid state transitions while completion save/navigation is in flight. Restrict visibility to valid session states (for example, active/paused or explicit pre-start cases) rather than all `!isActive` states.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| let returnButton = app.buttons["completion.returnButton"] | ||
| XCTAssertTrue(returnButton.waitForExistence(timeout: 5)) | ||
| returnButton.tap() | ||
| tapByStableCenter(returnButton, in: app) |
There was a problem hiding this comment.
Suggestion: Replacing the direct return-button tap with the stable-frame helper makes this step flaky because the helper rejects taps when the button's frame is not considered "stable" in the scroll/transition state right before app termination. In this specific flow, keep a direct tap followed by the explicit root wait before terminating. [possible bug]
Severity Level: Major ⚠️
- ⚠️ Flaky `testLaunchLoginCompleteSessionAndHistoryPersistence` in CI.
- ⚠️ Intermittent failures in `ios-e2e-critical` regression suite.
- ⚠️ Reduced confidence in history persistence coverage.
- ⚠️ Test-only failure; production app behavior remains correct.Steps of Reproduction ✅
1. Run the `testLaunchLoginCompleteSessionAndHistoryPersistence` UI test in
`ios/StillPointAppUITests/StillPointAppUITests.swift:45-138` as part of the
`ios-e2e-critical` or `ios-e2e-smoke` suite; the test drives a full login, session,
completion, and history persistence flow.
2. Observe the completion screen sequence near
`ios/StillPointAppUITests/StillPointAppUITests.swift:104-120`: the test types into
`completion.endNoteEditor`, dismisses the keyboard via `dismissKeyboardIfPresent(in:)` at
line 112, waits for `completion.savedIndicator`, then obtains `let returnButton =
app.buttons["completion.returnButton"]` at line 118.
3. Notice that the test now invokes `tapByStableCenter(returnButton, in: app)` at line 119
instead of a direct `returnButton.tap()`. The helper calls
`stableFrame(for:in:timeout:file:line:)` at lines 348–357, which repeatedly rejects frames
that are small, partially offscreen, or whose center is briefly outside
`app.frame.insetBy(dx: -1, dy: -1)` during scroll/transition animations on the completion
view.
4. On slower CI runs or devices where the completion screen is embedded in a scrolling
container and animating when the tap is attempted, `stableFrame` can time out (line
375–388) and call `XCTFail("Element existed but never had a stable tappable frame:
\(element.debugDescription)", file: file, line: line)` at line 390 even though a direct
`returnButton.tap()` would have succeeded; this causes the test to fail before the
subsequent
`XCTAssertTrue(app.otherElements["root.currentView.home"].waitForExistence(timeout: 8))`
at line 120 and `app.terminate()` at line 122, making this regression test flaky purely
due to the helper's gating logic.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ios/StillPointAppUITests/StillPointAppUITests.swift
**Line:** 119:119
**Comment:**
*Possible Bug: Replacing the direct return-button tap with the stable-frame helper makes this step flaky because the helper rejects taps when the button's frame is not considered "stable" in the scroll/transition state right before app termination. In this specific flow, keep a direct tap followed by the explicit root wait before terminating.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished running the review. |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:L
Sequence DiagramThis PR changes the iOS app launch and auth flow so UI tests always see a stable root.currentView marker, SwiftData is reset before model initialization in test mode, and cold-start diagnostics are logged and asserted. sequenceDiagram
participant UITest
participant iOSApp
participant RootView
participant AppViewModel
participant APIClient
UITest->>iOSApp: Launch with UI test env flags
iOSApp->>iOSApp: Reset SwiftData store when reset flag is set
iOSApp->>RootView: Create RootView with loading overlay and background
RootView->>AppViewModel: checkAuth()
AppViewModel->>APIClient: Fetch current user (me)
APIClient-->>AppViewModel: Return auth result and log e2e diagnostic
AppViewModel-->>RootView: Set currentView and cold start metric
RootView-->>UITest: Expose root.currentView.slug marker with metric
UITest->>UITest: waitForRoot asserts target view and coldStartAuthCheckMs bound
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. |
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
2 participants
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.
User description
Closes #276.
Summary
The iOS UI test suite was failing every test except the first
seedAuthenticated:truesmoke test, all timing out onapp.otherElements["root.currentView.auth"].waitForExistence(timeout: 30). Root-cause hypothesis: whileAppViewModel.isLoading == true, the outer ZStack inRootView.swiftrendered only two staticTextviews (the brand lockup), and SwiftUI did not surface that ZStack as a queryableotherElementsmatch.What changed
RootView:.accessibilityElement(children: .contain)on the outer ZStack soroot.currentView.*is always queryable as anotherElementsmatch, regardless of which branch is rendered.APIClient.init: switched[E2E-DIAG]fromprint()toos_logvia anonisolated static let diagLog = Logger(...).print()from the app process isn't captured in xcresult — that's why the prior diagnostic from Add iOS Release-config UI test gate to TestFlight workflow (#253) #261 never surfaced in CI logs.APIClient.init: extracted persistence intononisolated static func persist(store:key:)so the synchronous init no longer calls the actor-isolated instance method. This kills the two Swift 6 actor-isolation warnings onAPIClient.swift:52,60.APIClient: newclearSwiftDataPersistentStore()removes~/Library/Application Support/default.store{,-shm,-wal}whenSP_UI_TEST_RESET_STORE=1. SwiftData was the one piece of persistent state Stabilize iOS UI test infrastructure (state pollution between tests) #266's reset path missed.AppViewModel.checkAuth: sameos_logconversion for the[E2E-DIAG] checkAuth.doneline, still gated onSP_UI_TEST_MODE=1to avoid PII leakage in production logs.Test plan
ios-e2e-smokepasses on this PRios-e2e-criticalpasses on this PRAPIClient.swift:52or:60in the build log[E2E-DIAG]lines appear in the xcresult bundle (verifiable via os_log capture); previously zero diagnostic lines surfacedNotes
If
ios-e2e-smokestill fails after this lands, the new[E2E-DIAG]os_log lines should now appear in the CI log and tell us exactly whatAPIClient.initparsed and whatcurrentViewslugcheckAuthended on — the previous instrumentation was unverified and silent.CodeAnt-AI Description
Fix iOS app startup and session flow issues in UI tests
What Changed
Impact
✅ Fewer iOS launch timeouts✅ More reliable sign-in and session flows✅ Fewer blocked taps on login and session screens🔄 Retrigger CodeAnt AI Review
Details
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.