feat: finish adding recovery phrase e2e test#548
Conversation
…into beast/add-android-e2e-setup
…tus-Network/quantus-apps into beast/e2e-regular-send-flow
n13
left a comment
There was a problem hiding this comment.
Review: finish adding recovery phrase e2e test
Verdict: Approve with comments — clean, low-risk, test-only PR. All 10 selectors used by the new test are correctly wired to real widget keys (each attached exactly once), the reveal/continue/caution logic is sound, flutter analyze is clean, and Bugbot passed. Reviewed in an isolated worktree off the head commit (incremental diff over #547 only).
Should-fix (non-blocking)
Dropped assertion during extraction — mobile-app/patrol_test/support/wallet_flows.dart:13-22. WalletFlows.createFromWelcome omits the original expect($('Account 1'), findsOneWidget) that create_wallet_test asserted between waiting for and tapping the done button — minor lost coverage. Re-add it after the accountReadyDoneButton wait (the file already imports flutter_test); ideally key the account-name widget to avoid the locale-fragile text finder.
Nits
recovery_phrase_confirmation_screen.dart:51/recovery_phrase_screen.dart:47wrap the child inKeyedSubtree(key:). Correct and no remount/state-loss risk (stableconstkey), but you could passkey:straight to the component to match how sibling screens keyScaffoldBase(key:)directly (settings_screen.dart:39,wallet_settings_screen.dart:61). Purely stylistic.- The test assumes no biometrics enrolled on the device: Continue →
LocalAuthService().authenticate()returnstrueearly only when biometrics are unavailable; with enrolled biometrics a native prompt would hang the test (it never calls$.native…). Pre-existing prod behavior, not introduced here — worth documenting the "no enrolled biometrics" assumption in the E2E setup.
Nice
E2EKeys(String consts inlib/) shared by prod widgets andSelectors(Keyconsts in test) prevents selector/app drift; new keys follow the existing naming/ordering.- Good DRY:
createFromWelcomeshared by two tests, mirrors the existingimportFromWelcome. - Minimal, behavior-neutral production edits (key attachments + one optional null-safe
continueButtonKey, correctly plumbed through_SettingsCautionBottom, no collision with thewalletResetcaution flow). - The reveal key is attached only to the revealed hint, so
expect(recoveryPhraseRevealed, findsOneWidget)is satisfied only after the tap — precise, no false positives.
n13
left a comment
There was a problem hiding this comment.
Review — recovery phrase E2E
Reviewed the incremental (stacked-on-#547) diff in an isolated worktree with flutter analyze (clean). Low-risk and test-only: inert Keys plus one optional continueButtonKey param, with no behavioral changes for existing callers. I verified all 10 selectors used by view_recovery_phrase_test map to real attached keys (each attached exactly once), and the reveal/continue/caution flow is correct — the reveal key is attached only to the revealed hint, so the tap->assert is unambiguous.
Findings
- [Low] The extraction into
WalletFlows.createFromWelcomesilently dropped the originalexpect($('Account 1'), findsOneWidget)assertion thatcreate_wallet_testused to make. Minor lost coverage (the flow still asserts landing onhomeScreen). Recommend restoring it — ideally via aKeyon the account-name widget to avoid the locale-fragile text finder. - [Low] The test assumes no enrolled biometrics on the device: Continue ->
LocalAuthService().authenticate()returnstrueearly only when biometrics are unavailable; with biometrics enrolled a native prompt appears and the test hangs (it never drives$.native...). Pre-existing production behavior — worth documenting the "no enrolled biometrics" assumption in the E2E setup. - [Nit] The
KeyedSubtree(key:)wrapping inrecovery_phrase_confirmation_screen/recovery_phrase_screenis correct and carries no remount/state-loss risk, but you could passkey:straight to the component widgets (as the sibling screens do withScaffoldBase(key:)) and drop the wrapper. Purely stylistic.
Positives
- Clean anti-drift indirection:
E2EKeys(strings inlib/) are shared by both the widgets andSelectors(keys in test), so they can't diverge. - Good DRY:
createFromWelcomeis shared by two tests and mirrors the existingimportFromWelcome. continueButtonKeyis plumbed correctly with no collisions; reveal toggle logic is sound.
Verdict
Approve with comments. Clean, analyze-clean, correctly-wired test-only PR. Only the dropped Account 1 assertion is worth restoring before/at merge.
…into beast/e2e-show-secret-phrase
Summary
Add test flow for viewing recovery phrase.
Note
Low Risk
Changes are limited to E2E keys, test helpers, and a new Patrol test; production recovery-phrase and auth behavior is unchanged aside from test-only widget keys.
Overview
Adds a Patrol E2E test that creates a wallet, opens Settings → Wallet → Recovery phrase, passes the caution screen, and asserts the phrase can be tapped to reveal.
To drive that flow reliably, the PR extends
E2EKeysand mirrors them inSelectors, then attachesKeys on the home settings button, settings hub (wallet row), wallet settings screen, recovery phrase confirmation/screen scaffolds, the caution Continue button (via new optionalcontinueButtonKeyonSettingsCautionScaffold), and the recovery phrase reveal tap target / revealed hint inRecoveryPhraseBody.WalletFlows.createFromWelcomeis extracted socreate_wallet_testand the newview_recovery_phrase_testshare the same onboarding steps.Reviewed by Cursor Bugbot for commit 9c2017c. Configure here.