Skip to content

test(e2e): stub v1 balances in the unfunded-swap test - #3031

Merged
piyalbasu merged 2 commits into
masterfrom
fix/e2e-unfunded-swap-hermetic
Oct 6, 2026
Merged

piyalbasu merged 2 commits into
masterfrom
fix/e2e-unfunded-swap-hermetic

Conversation

@piyalbasu

Copy link
Copy Markdown
Contributor

TL;DR

The e2e test "Swap doesn't throw error when account is unfunded" fails on every attempt on fork PRs and dependabot PRs (for example #3024). It passes on branches in this repo. The balances v2 migration (#2906) replaced the test's v1 balances stub with a v2-only one, but the app still fetches v1 balances by default. On fork PRs, CI has no backend secret, so that v1 request never gets a response and login hangs until the test times out. This PR stubs v1 balances as unfunded alongside v2, and corrects two test comments that wrongly say v2 is the default.

Implementation details (for agents)

What changed:

  • extension/e2e-tests/sendPayment.test.ts: registers page.route("**/account-balances/**") with an unfunded v1 response before login(), keeping the existing stubAccountBalancesV2(page, () => null).
    • login() blocks on the TESTNET balances response in helpers/login.ts.
    • The client defaults to v1 because use_balances_v2 is false, and Amplitude returns 401 in CI.
    • On fork and dependabot runs, runTests.yml falls back to INDEXER_URL=http://indexer.invalid. The v1 request fails, the response event never fires, and the test hits its 15s timeout.
  • addAsset.test.ts and collectiblesEmptyStateCta.test.ts: comment-only fixes. Both said "v2 is the default balances source", the same mistaken belief that led to the v2-only stub. Both tests already stub v1, so their behavior is unchanged.

Verification:

  • Built locally with CI=true INDEXER_URL=http://indexer.invalid INDEXER_V2_URL=http://indexer.invalid to match a fork PR.
  • Before the fix, the test failed at helpers/login.ts:61 with the same error as CI run 36743657621. After the fix, it passed 3/3 (--repeat-each=3 --retries=0).
  • Checked every test that calls stubAccountBalancesV2. This was the only one without a matching v1 stub.
  • Ran /code-review at high effort. It found no behavior defects.

Evidence that it's fork-specific: the test failed 6/6 on fork runs (36743657621, 37046286578, 36955440186) and on dependabot (37170115543). It passed on branches in this repo (37340347208, 36762203690).

Follow-ups / out of scope:

  • Add a shared stubUnfundedAccountBalances(page) helper that registers both v1 and v2, so a v2-only stub can't happen again. Four inline copies of the unfunded v1 fixture exist today.
  • Export fulfillV1Balances and use it for the v1 stubs, so they honor contract_ids hints the same way the v2 stub does.

🤖 Generated with Claude Code

piyalbasu and others added 2 commits October 6, 2026 11:20
"Swap doesn't throw error when account is unfunded" logs in with the
unstubbed login() helper, which blocks on the TESTNET balances response.
#2906 replaced the test's v1 stub with a v2-only one, but the client still
defaults to v1 (use_balances_v2 is false unless Amplitude enables it), so the
v1 request went to the real indexer. On fork PRs and dependabot runs CI has
no INDEXER_URL secret and builds with http://indexer.invalid; the request
fails, no response event fires, and the test times out on every retry
(e.g. #3024, run 36743657621).

Stub v1 /account-balances/ as unfunded alongside the v2 route, matching
collectiblesEmptyStateCta.test.ts. Verified 2026-10-06 against a local build
with INDEXER_URL=INDEXER_V2_URL=http://indexer.invalid: fails before the
change at helpers/login.ts:61, passes 3/3 after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The client defaults to v1 balances (use_balances_v2 is false unless
remote config enables it). The same false belief led #2906 to drop the
v1 stub from the unfunded-swap test; fix the two remaining copies so the
pattern isn't repeated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings October 6, 2026 15:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved review issues remain.

Review effort: Lite
Findings: None

What changed in this PR

Fixes fork/Dependabot E2E login timeouts by stubbing unfunded v1 balances alongside v2.

Changes:

  • Adds the missing v1 balance route stub.
  • Corrects outdated balance-source comments.
File Summary
extension/​e2e-tests/​sendPayment.test.ts Stubs v1 and v2 unfunded balances before login.
extension/​e2e-tests/​collectiblesEmptyStateCta.test.ts Corrects the balance-source comment.
extension/​e2e-tests/​addAsset.test.ts Corrects the balance-source comment.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-a256979cd6ea0483b195
Backend: sandbox (piyalbasu). SDF collaborators only — install instructions in the release description.

@piyalbasu
piyalbasu merged commit bc35fc0 into master Oct 6, 2026
12 checks passed
@piyalbasu
piyalbasu deleted the fix/e2e-unfunded-swap-hermetic branch October 6, 2026 15:51
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