test: stabilize iOS simulator UI tests#387
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe iPhone simulator tests add reusable polling and tapping helpers for asynchronous UI interaction. Reminders navigation now handles multiple onboarding and confirmation states before opening a new reminder. Swipe, keyboard, Safari, Home screen, and Settings lifecycle tests wait for expected elements to appear or disappear instead of relying on fixed delays and single-shot screen snapshots. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/iphone-simulator.ts`:
- Around line 76-183: Add an explicit settle delay to both retry loops in
openNewReminder when no actionable state transition occurs: after the first loop
finds BackButton without Done, New Reminder, or the Reminders label, and after
tapping titleField in the second loop. Ensure retries wait for the UI to render
rather than immediately re-satisfying waitForElements, while preserving the
existing navigation and editor actions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d13e71cb-986e-474e-8eca-0b416c70ea00
📒 Files selected for processing (1)
test/iphone-simulator.ts
| const openNewReminder = async (): Promise<void> => { | ||
| let remindersListReady = false; | ||
|
|
||
| for (let step = 0; step < 10; step++) { | ||
| const elements = await waitForElements( | ||
| currentElements => currentElements.some(element => | ||
| ["Continue", "Not Now", "Cancel", "Discard Changes", "Reminders"].includes(element.label || "") || | ||
| element.name === "BackButton", | ||
| ), | ||
| "Reminders navigation to become ready", | ||
| ); | ||
|
|
||
| const onboardingAction = elements.find(element => | ||
| element.type === "Button" && ["Continue", "Not Now"].includes(element.label || ""), | ||
| ); | ||
| if (onboardingAction) { | ||
| await tapElement(onboardingAction); | ||
| continue; | ||
| } | ||
|
|
||
| const discardButton = elements.find(element => | ||
| element.type === "Button" && element.label === "Discard Changes", | ||
| ); | ||
| if (discardButton) { | ||
| await tapElement(discardButton); | ||
| continue; | ||
| } | ||
|
|
||
| const cancelButton = elements.find(element => | ||
| element.type === "Button" && element.label === "Cancel", | ||
| ); | ||
| if (cancelButton) { | ||
| await tapElement(cancelButton); | ||
| continue; | ||
| } | ||
|
|
||
| const isInRemindersList = elements.some(element => element.name === "BackButton"); | ||
| const doneButton = elements.find(element => | ||
| element.type === "Button" && element.label === "Done", | ||
| ); | ||
| if (isInRemindersList && doneButton) { | ||
| await tapElement(doneButton); | ||
| continue; | ||
| } | ||
|
|
||
| const newReminderButton = elements.find(element => | ||
| element.type === "Button" && element.label === "New Reminder", | ||
| ); | ||
| if (isInRemindersList && newReminderButton) { | ||
| remindersListReady = true; | ||
| break; | ||
| } | ||
|
|
||
| const remindersList = elements.find(element => | ||
| element.type === "StaticText" && element.label === "Reminders", | ||
| ); | ||
| if (remindersList) { | ||
| await tapElement(remindersList); | ||
| } | ||
| } | ||
|
|
||
| if (!remindersListReady) { | ||
| throw new Error("Unable to navigate to the Reminders list"); | ||
| } | ||
|
|
||
| for (let step = 0; step < 8; step++) { | ||
| const elements = await waitForElements( | ||
| currentElements => currentElements.some(element => | ||
| (element.type === "TextField" && element.name === "Title" && element.value === "") || | ||
| (element.type === "Button" && element.label === "Continue") || | ||
| (element.type === "Button" && element.identifier === "Return") || | ||
| (element.type === "Button" && element.label === "New Reminder"), | ||
| ), | ||
| "the new reminder editor", | ||
| ); | ||
|
|
||
| if (elements.some(element => | ||
| element.type === "Button" && element.identifier === "Return", | ||
| )) { | ||
| return; | ||
| } | ||
|
|
||
| const continueButton = elements.find(element => | ||
| element.type === "Button" && element.label === "Continue", | ||
| ); | ||
| if (continueButton) { | ||
| await tapElement(continueButton); | ||
| continue; | ||
| } | ||
|
|
||
| const newReminderButton = elements.find(element => | ||
| element.type === "Button" && element.label === "New Reminder", | ||
| ); | ||
| if (newReminderButton) { | ||
| await tapElement(newReminderButton); | ||
| continue; | ||
| } | ||
|
|
||
| const titleField = elements.find(element => | ||
| element.type === "TextField" && element.name === "Title" && element.value === "", | ||
| ); | ||
| if (titleField) { | ||
| await tapElement(titleField); | ||
| } | ||
| } | ||
|
|
||
| throw new Error("Unable to focus a new reminder after dismissing onboarding"); | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Retry loops in openNewReminder have no-op branches with zero backoff.
In the first loop, when isInRemindersList is true but neither "Done" nor "New Reminder" has rendered yet, and the top-level "Reminders" StaticText isn't present either (lines 112-134), no tap and no delay occurs. Since the guarding waitForElements call at the top of the loop (lines 80-86) is already satisfied by BackButton alone, it resolves on the very first getElementsOnScreen() poll with no wait — so this branch can burn through the 10-attempt budget before the list finishes rendering its action buttons after the push transition, risking a premature "Unable to navigate to the Reminders list" failure. The second loop has the same shape at lines 174-180: re-tapping an empty titleField that stays empty re-satisfies the predicate instantly on the next iteration, with no wait for the keyboard/Return-key to actually appear, within only 8 attempts.
This directly works against the PR's goal of removing timing-based flakiness — both retry loops rely entirely on incidental round-trip latency of getElementsOnScreen()/tap() rather than an explicit settle delay.
🔧 Suggested fix: add an explicit backoff when no action is taken
const remindersList = elements.find(element =>
element.type === "StaticText" && element.label === "Reminders",
);
if (remindersList) {
await tapElement(remindersList);
+ } else if (isInRemindersList) {
+ // Already in the list but "Done"/"New Reminder" haven't rendered yet —
+ // give the UI a moment to finish its transition before retrying.
+ await new Promise(resolve => setTimeout(resolve, 250));
}Apply the analogous change after the titleField tap at lines 174-180 (or, more robustly, only re-tap titleField if it wasn't already tapped in the previous iteration).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/iphone-simulator.ts` around lines 76 - 183, Add an explicit settle delay
to both retry loops in openNewReminder when no actionable state transition
occurs: after the first loop finds BackButton without Done, New Reminder, or the
Reminders label, and after tapping titleField in the second loop. Ensure retries
wait for the UI to render rather than immediately re-satisfying waitForElements,
while preserving the existing navigation and editor actions.
Replace fixed delays with condition-based UI polling and normalize stateful Reminders navigation before entering text. - wait for Settings, Safari, and Home screen readiness - dismiss Reminders onboarding and stale quick-entry state - verify keyboard and reminder transitions before continuing
1 participant
Replace fixed delays with condition-based UI polling and normalize stateful Reminders navigation before entering text.