Skip to content

Add E2E test regular send flow#547

Merged
dewabisma merged 19 commits into
mainfrom
beast/e2e-regular-send-flow
Jul 2, 2026
Merged

Add E2E test regular send flow#547
dewabisma merged 19 commits into
mainfrom
beast/e2e-regular-send-flow

Conversation

@dewabisma

@dewabisma dewabisma commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

Add e2e test for regular send. When running the test, found UI overflow bug. Already addressed here.


Note

Medium Risk
Touches real send/review UI and submits on-chain transfers in E2E (testnet/fixture only), plus a small layout change on the review screen users see when confirming sends.

Overview
Adds a Patrol E2E test that imports a wallet, sends the minimal funded amount on a simulator/emulator only, and asserts a pending outgoing send appears on home. Test support includes shared WalletFlows.importFromWelcome, SendPreflight (balance + fee vs existential deposit), locale-aware amount text, typeTextIntoField so amount parsing/onChanged runs, and TEST_SEND_RECIPIENT_ADDRESS via dart-define (with .env.test injection unchanged for other secrets).

Production changes wire E2EKeys through home send, the full send stack (recipient → amount → review → submitted), and the first pending send row in home activity; AddressInputField gains an optional field key.

review_send_screen layout is adjusted so the summary scrolls inside an Expanded region—fixing overflow found while exercising the flow.

Reviewed by Cursor Bugbot for commit c57e681. Configure here.

@dewabisma dewabisma requested a review from n13 July 1, 2026 11:43

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ 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 c57e681. Configure here.

final feeData = await balancesService.getBalanceTransferFee(account, recipientAddress, ed);
final required = ed + feeData.fee;

if (balance < required) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preflight ignores sender ED reserve

Medium Severity

SendPreflight treats a wallet as funded when on-chain balance is at least existential deposit plus fee, but the send amount screen validates against effectiveMaxBalanceProvider, which reserves another existential deposit on the sender. Wallets funded only to the preflight threshold can pass the check yet stay blocked on review with insufficient balance.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c57e681. Configure here.

DART_DEFINES+=(--dart-define-from-file=.env.test)
elif [[ -n "${TEST_IMPORT_MNEMONIC:-}" ]]; then
echo "==> Injecting test secrets from environment"
DART_DEFINES+=(--dart-define=TEST_IMPORT_MNEMONIC="$TEST_IMPORT_MNEMONIC")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI dart-defines env fallback removed

Medium Severity

patrol_collect_dart_defines no longer maps exported environment variables (e.g. TEST_IMPORT_MNEMONIC) to --dart-define when .env.test is absent, though comments still describe that CI path. Runners that inject secrets via env without creating .env.test will build Patrol tests without required defines and fail at runtime.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c57e681. Configure here.

@n13 n13 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — E2E regular send flow

Reviewed the full diff in an isolated worktree with flutter analyze (clean, including patrol_test/). The production surface is tiny — inert E2E Keys plus one overflow fix — everything else is test-only. Nice work: DRY is respected and the Patrol/Flutter pitfalls (debounced fee, hit-testable disabled buttons, locale-aware amount formatting) are handled thoughtfully.

Bugbot findings — verified

1. Preflight ignores sender ED reserve — VALID (low practical severity).
SendPreflight guarantees balance ≥ ED + fee and sends amount = ED, but the amount screen validates against effectiveMaxBalanceProvider (balance − ED, since the ED toggle defaults to true) via getAmountStatus ((amount + fee) > balance ⇒ insufficientBalance). So actually sending ED requires balance ≥ 2·ED + fee. A wallet funded in the [ED+fee, 2·ED+fee) window passes preflight but the Review button stays disabled, so _openReviewSend taps a disabled button until the 45s timeout (slow failure) instead of surfacing the intended clear error. The window is only ~0.001 QUAN wide (ED = 1e9 planck, 12 decimals), so any sensibly funded fixture clears it.
Fix: make preflight mirror the UI — require balance ≥ 2·ED + fee and update the "need>=…" error text.

2. dart-define env fallback removed — PARTIALLY VALID (stale docs, no CI breakage).
The elif [[ -n "${TEST_IMPORT_MNEMONIC}" ]] branch is gone; only .env.test is read now. But there is no Patrol/E2E CI job — ci.yaml runs analyze + unit tests on main and only touches .env (not .env.test), and nothing in .github/ references patrol / .env.test / these vars. So no existing CI breaks. The genuine defects are documentation-level: the header comment (patrol_common.sh lines 76–79) still says "CI: export TEST_IMPORT_MNEMONIC…", and the warning names only TEST_IMPORT_MNEMONIC, not the new TEST_SEND_RECIPIENT_ADDRESS (which the old elif never injected anyway).
Fix: update the comment + warning to state .env.test is the only supported mechanism, or restore env injection for both vars if the beast/add-android-e2e-setup base will export secrets in CI.

Other (non-blocking)

  • [Low] Inconsistent text entry: the recipient uses bulk enterText but the amount uses typeTextIntoField (send_flow_test.dart:43 vs :47). The onChanged-not-firing rationale in text_input.dart applies to the recipient step too — add a comment explaining why they differ, or unify.
  • [Low] The review_send_screen error message moved outside the scroll region (now pinned above the confirm button). Arguably better (always visible) but is a behavioral change — confirm it matches design intent.
  • [Nit] typeTextIntoField: if (char.isEmpty) continue; is dead code, and split('') breaks surrogate pairs (fine for numeric input — worth documenting the numeric-only assumption).
  • [Nit] The pending-activity key attaches to the first matching pending send; a leftover pending send from a prior run could take it. The assertion ("a pending outgoing send is visible") still holds.

Positives

  • Correct, minimal overflow fix — ScaffoldBase bounds mainContent height, so Expanded(SingleChildScrollView(_summarySection)) is valid (hero fixed, summary scrolls) with no nested-scroll hazard.
  • Faithful DRY extraction (WalletFlows.importFromWelcome reproduces the old test body exactly).
  • e2e_locale.dart mirrors the app's locale resolution precisely and reuses production types.
  • All 12 new E2EKeys are wired, mirrored in Selectors, and exercised; no collisions/orphans. flutter analyze clean.

Verdict

Approve with comments. Production risk is minimal and analyze is clean. Bugbot #1 is a real but low-probability funding-contract mismatch and #2 is stale docs — both worth a quick follow-up, neither blocks this test-only PR.

Reviewed in an isolated worktree at commit c57e681.

@n13 n13 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — E2E regular send flow

Reviewed the full diff in an isolated worktree with flutter analyze (clean, including patrol_test/). The production surface is tiny — inert E2E Keys plus one overflow fix — everything else is test-only. Nice work: DRY is respected and the Patrol/Flutter pitfalls (debounced fee, hit-testable disabled buttons, locale-aware amount formatting) are handled thoughtfully.

Bugbot findings — verified

1. Preflight ignores sender ED reserve — VALID (low practical severity).
SendPreflight guarantees balance >= ED + fee and sends amount = ED, but the amount screen validates against effectiveMaxBalanceProvider (balance - ED, since the ED toggle defaults to true) via getAmountStatus ((amount + fee) > balance => insufficientBalance). So actually sending ED requires balance >= 2*ED + fee. A wallet funded in the [ED+fee, 2*ED+fee) window passes preflight but the Review button stays disabled, so _openReviewSend taps a disabled button until the 45s timeout (slow failure) instead of surfacing the intended clear error. The window is only ~0.001 QUAN wide (ED = 1e9 planck, 12 decimals), so any sensibly funded fixture clears it.
Fix: make preflight mirror the UI — require balance >= 2*ED + fee and update the "need>=..." error text.

2. dart-define env fallback removed — PARTIALLY VALID (stale docs, no CI breakage).
The elif [[ -n "${TEST_IMPORT_MNEMONIC}" ]] branch is gone; only .env.test is read now. But there is no Patrol/E2E CI job — ci.yaml runs analyze + unit tests on main and only touches .env (not .env.test), and nothing in .github/ references patrol / .env.test / these vars. So no existing CI breaks. The genuine defects are documentation-level: the header comment (patrol_common.sh lines 76-79) still says "CI: export TEST_IMPORT_MNEMONIC...", and the warning names only TEST_IMPORT_MNEMONIC, not the new TEST_SEND_RECIPIENT_ADDRESS (which the old elif never injected anyway).
Fix: update the comment + warning to state .env.test is the only supported mechanism, or restore env injection for both vars if the beast/add-android-e2e-setup base will export secrets in CI.

Other (non-blocking)

  • [Low] Inconsistent text entry: the recipient uses bulk enterText but the amount uses typeTextIntoField (send_flow_test.dart:43 vs :47). The onChanged-not-firing rationale in text_input.dart applies to the recipient step too — add a comment explaining why they differ, or unify.
  • [Low] The review_send_screen error message moved outside the scroll region (now pinned above the confirm button). Arguably better (always visible) but is a behavioral change — confirm it matches design intent.
  • [Nit] typeTextIntoField: if (char.isEmpty) continue; is dead code, and split('') breaks surrogate pairs (fine for numeric input — worth documenting the numeric-only assumption).
  • [Nit] The pending-activity key attaches to the first matching pending send; a leftover pending send from a prior run could take it. The assertion ("a pending outgoing send is visible") still holds.

Positives

  • Correct, minimal overflow fix — ScaffoldBase bounds mainContent height, so Expanded(SingleChildScrollView(_summarySection)) is valid (hero fixed, summary scrolls) with no nested-scroll hazard.
  • Faithful DRY extraction (WalletFlows.importFromWelcome reproduces the old test body exactly).
  • e2e_locale.dart mirrors the app's locale resolution precisely and reuses production types.
  • All 12 new E2EKeys are wired, mirrored in Selectors, and exercised; no collisions/orphans. flutter analyze clean.

Verdict

Approve with comments. Production risk is minimal and analyze is clean. Bugbot #1 is a real but low-probability funding-contract mismatch and #2 is stale docs — both worth a quick follow-up, neither blocks this test-only PR.

@dewabisma dewabisma changed the base branch from beast/add-android-e2e-setup to main July 2, 2026 14:31
@dewabisma dewabisma merged commit efdbae4 into main Jul 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants