[LWM] test(e2e): split mobile ledger sync into small pre-seeded tests (QAA-1404) - #20931
Open
VicAlbr wants to merge 22 commits into
Open
[LWM] test(e2e): split mobile ledger sync into small pre-seeded tests (QAA-1404)#20931VicAlbr wants to merge 22 commits into
VicAlbr wants to merge 22 commits into
Conversation
Review follow-up on the ledger sync layer. Cleanup correctness: - resolveApplicationPath read the application path out of the JWT permissions, taking the first key in backend order and falling back to paths[0], which can be the trustchain root. Replaced with TrustchainSDK.restoreTrustchain, which resolves the stream itself and picks the highest child index. It needs no device, so it is safe on this path. - It also re-derives walletSyncEncryptionKey. Removing a member rotates the application stream and the key with it, and the old helper only ever wrote the path back, leaving a later pull or push on a stale key. Renamed to refreshTrustchain and both fields are now written to the push and pull args. - deleteLedgerSyncData destroyed the trustchain only if the cloud-sync delete succeeded first, so a suite that never pushed data leaked its trustchain on the backend. The destroy moved into a finally. - ALREADY_DESTROYED_ERRORS dropped TrustchainNotFound, which only app-side hooks throw, and gained LedgerAPI4xx and CloudSyncHttpError, which are what this path actually raises once the trustchain is gone. Sonar: rejections carry Error objects rather than strings (S6671), node: protocol on builtin imports (S7772), the helper's statics are readonly (S1444), and the id comparison sorts with localeCompare (S2871, the only reliability bug and the sole cause of the D rating).
Every suite now boots with a pre-seeded trustchain, so nothing drives the in-app activation flow any more and the page objects behind it had no callers left: LedgerSyncDrawer.syncAccounts, closeLedgerSync, expectSyncAccountsButtonExist and expectSynchronizationSuccess, plus SettingsPage.enableWalletSync and the six locators that only they used. QAA-1503 tracks restoring that coverage with a dedicated activation suite. Also drops four dependencies from e2e/desktop that moved to live-e2e-shared with the CLI layer and now have no imports left (ledger-key-ring-protocol, cloud-sync, platform-wallet-sync, live-wallet), and defers the TrustchainPage import so its SDK graph is loaded on first use rather than in every worker.
setupSeed wrote the generated seed to process.env.SEED but restored the old one with setEnv, which only writes the in-memory live-env store. startSpeculos reads process.env.SEED directly, so the restore never took effect and the generated seed leaked into whatever ran next in that worker. Also moves the LIVE-35808 TODO onto the first suite; above APP_INSTANCE_NAME it read as documenting the constant.
LIVE-35808 is fixed, so staging cloud-sync can verify the JWT staging
trustchain issues and the suites can run again.
Desktop: the five suites drop test.describe.skip, and with the blocker gone so
do the addBugLink("LIVE-35808") annotations.
Mobile: ledgerSync.skip.spec.ts goes back to ledgerSync.spec.ts, the name
jest.config.js excludes it by, and loses its allure.issue for the same bug.
No test in either suite is skipped now.
Splits the single mobile Ledger Sync test into five suites mirroring desktop, each booting the app already a member of a freshly created trustchain. - bridge: importTrustchain, so the app can be pre-seeded from the userdata blob the CLI writes, and setAllFeatureFlags to replace the persisted override map rather than add to it - reuse the shared ledger sync CLI layer instead of the mobile duplicate - TrustchainPage asserts trustchain contents through the CLI - relaunch the app on the ledger sync flag before init: the trustchain SDK is a module singleton pinned on first render, so an override sent to a running app never changes the environment - route commands and the generated seed through app.ledgerSync: jest gives each test file its own module registry and its own copy of process.env, so a spec mutating either only ever changes its own Work in progress: the add-account assertion has not passed yet, and the specs still need renaming back to .skip.spec.ts for LIVE-35808.
TinyCard puts its testID on the outer container, but that container is a plain Flex — the only touchable is the Pressable wrapping the CTA text, and it had no testID of its own. A test tapping the card by its testID hits dead space and silently does nothing. Derives the CTA testID from the one the caller passes, so the synchronized instances list gets walletSync-manage-instance-<id>-cta alongside its row.
addAccountAtIndex taps "deselect all" only when more than one account is discovered, but tapped the account row unconditionally. A single discovered account arrives already selected, so that tap cleared it: Confirm greyed out, the enabled-add-accounts-continue-button testID stopped existing, and finishAccountsDiscovery spun until its 60s budget ran out on "No elements found". Only the multi-account path needs deselecting and then picking one, so the row tap moves inside that branch and the single-account case is left alone. Existing suites all use the shared seed and discover several accounts, so they stay on the unchanged path; the ledger sync suites generate a seed per run and are the first to discover exactly one.
LIVE-35808 is fixed, so the suites run against staging — which is both the CLI default and the environment the app boots with. The terminate/relaunch dance existed only so the app would follow LEDGER_SYNC_ENVIRONMENT=PROD, and it cost a relaunch per suite for nothing in the default case. Removes it along with setAllFeatureFlags, which had no other caller, and the allure.issue markers for a bug that is now closed. The app still pins its trustchain SDK on first render, so PROD would silently put the CLI and the app on different backends and read as an empty trustchain. A guard now refuses that up front instead. Also follows the rename of resolveApplicationPath to restoreTrustchain.
Three separate reasons the delete instance test could never pass. The row is not touchable, so tapping it did nothing and the test walked on to a device prompt that was never coming. It now taps the CTA inside the row. Removing an instance goes through device selection on mobile, unlike desktop where the row's Remove button reaches the device directly, so the test has to pick the device before Speculos is asked to confirm. And it targeted the wrong member: addTrustchainMember mints new credentials before adding the member, so the pubkey left in ledgerKeyRingProtocolArgs belongs to the member the app is then seeded with — the phone itself. The app refuses that with "You can't remove this phone while you're using it". The trustchain's original member is now kept as LedgerSyncCliHelper.initialMember, which is the CLI and the only instance a test can remove.
Covers the e2e package, the shared layer and live-mobile — the app carries the Detox-only importTrustchain bridge message and two testIDs.
live-github-bot
Bot
changed the title
VicAlbr
marked this pull request as ready for review
August 19, 2026 12:03
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Ledger Wallet Mobile’s Ledger Sync E2E coverage from a single monolithic suite into five smaller, isolated suites that run against freshly generated, pre-seeded trustchains, aligning mobile with the shared desktop approach introduced in the stacked PR (#20775).
Changes:
- Split the Ledger Sync mobile E2E suite into five ticket-focused specs driven by a shared
ledgerSync.tsrunner. - Pre-seed trustchain state into the app via a new Detox bridge message (
importTrustchain) and shared CLI helper utilities. - Fix E2E interaction gaps by adding missing testIDs (manage-instances row + TinyCard CTA) and correcting account-selection behavior in the add-account drawer.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| libs/live-e2e-shared/src/ledgerSync/helper.ts | Adds initialization guards and tracks the initial trustchain member for instance-removal tests. |
| e2e/mobile/utils/cliUtils.ts | Replaces mobile-specific Ledger Sync CLI implementation with shared live-e2e-shared CLI entrypoints. |
| e2e/mobile/specs/ledgerSync/ledgerSync.ts | Introduces shared test runners for the five mobile Ledger Sync suites, including seed setup/cleanup and pre-seeded init flows. |
| e2e/mobile/specs/ledgerSync/ledgerSyncAddAccount.spec.ts | Adds dedicated “add account” Ledger Sync test spec. |
| e2e/mobile/specs/ledgerSync/ledgerSyncRenameAccount.spec.ts | Adds dedicated “rename account” Ledger Sync test spec. |
| e2e/mobile/specs/ledgerSync/ledgerSyncDeleteAccount.spec.ts | Adds dedicated “delete account” Ledger Sync test spec. |
| e2e/mobile/specs/ledgerSync/ledgerSyncDeleteInstance.spec.ts | Adds dedicated “delete instance” Ledger Sync test spec. |
| e2e/mobile/specs/ledgerSync/ledgerSyncDeleteBackup.spec.ts | Adds dedicated “delete backup” Ledger Sync test spec. |
| e2e/mobile/specs/ledgerSync/ledgerSync.spec.ts | Removes the previous monolithic Ledger Sync test suite. |
| e2e/mobile/page/trustchain.page.ts | Adds a CLI-backed page object to assert trustchain contents without relying on UI state. |
| e2e/mobile/page/settings/ledgerSync.page.ts | Refactors Ledger Sync page object to use shared CLI helper/setup utilities and adds instance-management helpers. |
| e2e/mobile/page/index.ts | Registers the new TrustchainPage on the app page object. |
| e2e/mobile/page/accounts/addAccount.drawer.ts | Adds conditional import prompt handling and adjusts account selection behavior during discovery. |
| e2e/mobile/bridge/server.ts | Sends importTrustchain bridge messages when data.trustchain exists in userdata config. |
| apps/ledger-live-mobile/src/mvvm/features/WalletSync/screens/Manage/index.tsx | Adds a testID to the “manage instances” row so tests can open the instances list reliably. |
| apps/ledger-live-mobile/src/mvvm/features/WalletSync/components/TinyCard.tsx | Moves actionable testID onto the touchable CTA (Pressable) so Detox taps hit the correct element. |
| apps/ledger-live-mobile/src/e2e/bridge/types.ts | Extends bridge message types to include importTrustchain. |
| apps/ledger-live-mobile/src/e2e/bridge/client.ts | Implements importTrustchain message handling by dispatching importTrustchainStoreState. |
| .changeset/lwm-ledger-sync-small-tests.md | Adds a changeset documenting the E2E refactor and related mobile testability fixes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
111
to
114
| if (accountCount > 1) { | ||
| await tapById(this.deselectAllButtonId); | ||
| await tapById(this.accountItemRegExp(), 0); | ||
| } |
Contributor
Rsdoctor Bundle Diff AnalysisFound 7 projects in monorepo, 7 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 desktop-mainPath:
📁 desktop-preloaderPath:
📁 desktop-rendererPath:
📁 desktop-webviewDappPreloaderPath:
📁 desktop-webviewPreloaderPath:
📁 desktop-workersPath:
📁 mobilePath:
Generated by Rsdoctor GitHub Action |
VicAlbr
force-pushed
the
test/e2e-QAA-1404-ledger-sync-small-tests
branch
2 times, most recently
from
August 19, 2026 16:30
f04d865 to
417319c
Compare
Base automatically changed from
test/e2e-QAA-1404-ledger-sync-small-tests
to
develop
August 19, 2026 17:24
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
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.
📝 Description
Note
Stacked on #20775 (
test/e2e-QAA-1404-ledger-sync-small-tests) and targets that branch, notdevelop. Review #20775 first — merging it will retarget this one automatically.Problem. Mobile was where desktop started: one monolithic Ledger Sync test covering three
B2CQA tickets at once, opening with a destroy-then-recreate burst against the backend because every
run shared one hardcoded seed and had to clear the previous run's state. One flake took all three
tickets down.
Solution. The same five small tests desktop got in #20775, one per Xray ticket, each booting the
app already a member of a freshly created trustchain:
What made that possible:
importTrustchainStoreStateat boot and persists the slice; the gap was one hop, soloadConfignow relays
data.trustchainas animportTrustchainmessage. The CLI half needed no change —CliCommandalready receivesuserdataPath, so the sharedsaveTrustchainToUserdatawrites thesame blob desktop uses. Detox-only code (
if (Config.DETOX)), no production impact.e2e/mobile/utils/cliUtils.tsdrops from 182to 53 lines; only
registerSpeculosTransportstays local, since the constantspeculos-httpidis a real mobile difference. Assertions go through a
TrustchainPagethat pulls via the CLI.afterAll.Mobile-specific gotcha worth knowing. Jest gives the test environment and the spec file separate
module registries and separate copies of
process.env. A spec that imports a stateful helperdirectly gets a second copy of it, and a spec that writes
process.env.SEEDonly changes its owncopy while Speculos keeps booting on the shell's seed. Everything stateful therefore enters through
app.ledgerSync.*, which is why the page object grew command factories rather than the specimporting them. The tell for this class of bug is a module-level
console.logprinting twice.Also included — three defects the split surfaced, each its own commit:
addAccountAtIndexcleared the selection when exactly one account was discovered. It taps"deselect all" only for multiple accounts but tapped the row unconditionally, and a lone account
arrives pre-selected — so Confirm greyed out and discovery timed out. Existing suites use the
shared seed and discover several accounts, so they never hit it; a generated seed discovers one.
This is shared page-object code: the multi-account path is unchanged.
TinyCardput its testID on a non-touchableFlexwhile the only touchable was an unlabelledinner
Pressable, so taps on an instance row silently did nothing.addTrustchainMembermints new credentialsbefore adding the member, so the pubkey left behind belongs to the member the app is then seeded
with — the phone. The app refuses that ("You can't remove this phone while you're using it"). The
trustchain's original member is now kept as
LedgerSyncCliHelper.initialMember.✅ Validation
Full suite green on both platforms in CI, on STG:
run 32237373557 —
iOS 5/5, Android 5/5,
test_filter=ledgerSync, nanoX.Also 5/5 locally on iOS (debug) and 4/5 on Android (release); the one local Android failure was a
stale APK predating the
TinyCardtestID, which CI confirms by passing.LEDGER_SYNC_ENVIRONMENTis unset in CI, so both the CLI and the app default to staging. Mobilecannot run against PROD — the app pins its trustchain SDK on first render, so an override sent to a
running app moves the flag but not the SDK, leaving the two on different backends and reading as an
empty trustchain. A guard fails fast rather than letting that happen silently.
The old monolithic test also covered B2CQA-2292 (settings entry point) and B2CQA-2293
(activation flow), which the pre-seeded suites do not exercise. Both remain in "Automated" status in
Xray. Same gap as desktop, tracked by
QAA-1503 — those two tickets should be attached to
it or moved out of "Automated".
🔗 Context