Skip to content

Commit c6197b8

Browse files
cmd-obcortisiko
cmd-ob
and
cortisiko
authored
fix: flaky snaps and identity tests (MetaMask#25795)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ### Root Causes **Flask Snap tests (Android + iOS):** The invalid-entropy-source and past-date-scheduling error tests were failing on **both** platforms but for different reasons: - **iOS**: errors appear as native WebView alert dialogs — `Assertions.expectTextDisplayed()` works. - **Android**: errors render inside the web-view result span as JSON with escaped quotes — `checkResultSpanIncludes()` is needed, matching a quote-free substring to avoid `\"invalid\"` vs `"invalid"` mismatches. The fix uses `device.getPlatform()` to pick the correct assertion per platform. **Identity multi-SRP test:** - `profile-sync-controller@27.1.0` bumped `snaps-controllers` from `^14` to `^17.2.0`, slowing down the initial full sync enough that `enqueueSingleGroupSync` silently drops subsequent sync requests while `isAccountTreeSyncingInProgress` is still `true`. - The "Discovering accounts..." spinner was still active when the test tried to tap the Wallet 2 "Add account" button, which is disabled during discovery — so the tap silently did nothing and the 4th account was never created. ### Changes **Flask Snap tests** (`test-snap-bip-32`, `test-snap-bip-44`, `test-snap-get-entropy`, `test-snap-background-events`): - Platform-conditional error assertions: native dialog check on iOS, web-view result span check on Android - Increased assertion timeouts to 30s for slow CI emulators **Identity sync tests** (`multi-srp.spec.ts`): - Wait for initial full sync via `waitUntilSyncedAccountsNumberEquals(1)` before mutating accounts - Wait for "Discovering accounts..." to disappear before tapping "Add account" on Wallet 2 - Set up PUT event counter after initial sync completes to only track subsequent mutations **Identity sync helpers** (`helpers.ts`): - Increased `BASE_TIMEOUT` from 12s to 30s - Improved error messages to include actual vs expected counts ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [ ] I've included tests if applicable - [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Test-only changes (timeouts, waits, and assertions) with no impact on production logic; main risk is slightly longer CI runtime if failures occur. > > **Overview** > Reduces flakiness in identity account-sync smoke coverage by **waiting for the initial full sync to complete** before mutating accounts in `multi-srp.spec.ts`, and by increasing the shared helper `BASE_TIMEOUT` to 30s. > > Improves debuggability of identity sync helpers by including *actual vs expected* counts in timeout errors. > > Stabilizes multiple Snap smoke tests by making error assertions **platform-aware** (native alert on iOS vs web-view result on Android) and by increasing assertion timeouts to 30s; also increases the optional delay when tapping the V2 “Add Account” button (1.5s � 5s) to avoid UI timing issues. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 92ac3cf. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Curtis David <Curtis.David7@gmail.com>
1 parent 7a5ce94 commit c6197b8

7 files changed

Lines changed: 82 additions & 18 deletions

File tree

‎e2e/pages/wallet/AccountListBottomSheet.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class AccountListBottomSheet {
145145

146146
await Gestures.waitAndTap(button, {
147147
elemDescription: 'Add Account button in V2 multichain accounts',
148-
delay: options?.shouldWait ? 1500 : 0,
148+
delay: options?.shouldWait ? 5000 : 0,
149149
});
150150
}
151151

‎tests/smoke/identity/account-syncing/multi-srp.spec.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,16 @@ describe(SmokeIdentity('Account syncing - Mutiple SRPs'), () => {
7474
prepareEventsEmittedCounter,
7575
waitUntilSyncedAccountsNumberEquals,
7676
} = arrangeTestUtils(userStorageMockttpController);
77+
78+
// Wait for the initial full sync to complete before adding accounts.
79+
// The AccountTreeController's enqueueSingleGroupSync silently drops
80+
// sync requests when isAccountTreeSyncingInProgress is true or
81+
// hasAccountTreeSyncingSyncedAtLeastOnce is false, so we must ensure
82+
// the first full sync has finished pushing the initial group.
83+
await waitUntilSyncedAccountsNumberEquals(1);
84+
85+
// Set up event counter AFTER the initial sync completes so it only
86+
// tracks events from subsequent account mutations.
7787
const { waitUntilEventsEmittedNumberEquals } =
7888
prepareEventsEmittedCounter(
7989
UserStorageMockttpControllerEvents.PUT_SINGLE,

‎tests/smoke/identity/utils/helpers.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export const getSrpIdentifierFromHeaders = (
4747
export const arrangeTestUtils = (
4848
userStorageMockttpController: UserStorageMockttpController,
4949
) => {
50-
const BASE_TIMEOUT = 12000;
50+
const BASE_TIMEOUT = 30000;
5151
const BASE_INTERVAL = 1000;
5252

5353
const prepareEventsEmittedCounter = (
@@ -134,9 +134,13 @@ export const arrangeTestUtils = (
134134

135135
ids.timeout = setTimeout(() => {
136136
clearInterval(ids.interval);
137+
const actual =
138+
userStorageMockttpController.paths.get(
139+
USER_STORAGE_GROUPS_FEATURE_KEY,
140+
)?.response?.length ?? 0;
137141
reject(
138142
new Error(
139-
`Timeout waiting for synced accounts number to be ${expectedNumber}`,
143+
`Timeout waiting for synced accounts number to be ${expectedNumber}\n Actual: ${actual}`,
140144
),
141145
);
142146
}, BASE_TIMEOUT);
@@ -177,9 +181,13 @@ export const arrangeTestUtils = (
177181

178182
ids.timeout = setTimeout(() => {
179183
clearInterval(ids.interval);
184+
const actual =
185+
userStorageMockttpController.paths.get(
186+
USER_STORAGE_FEATURE_NAMES.addressBook,
187+
)?.response?.length ?? 0;
180188
reject(
181189
new Error(
182-
`Timeout waiting for synced contacts number to be ${expectedNumber}`,
190+
`Timeout waiting for synced contacts number to be ${expectedNumber}\n Actual: ${actual}`,
183191
),
184192
);
185193
}, BASE_TIMEOUT);
@@ -226,9 +234,11 @@ export const arrangeTestUtils = (
226234

227235
ids.timeout = setTimeout(() => {
228236
clearInterval(ids.interval);
237+
const actual =
238+
userStorageMockttpController.paths.get(path)?.response?.length ?? 0;
229239
reject(
230240
new Error(
231-
`Timeout waiting for synced accounts number to be ${expectedNumber}`,
241+
`Timeout waiting for synced elements at "${path}" to be ${expectedNumber}\n Actual: ${actual}`,
232242
),
233243
);
234244
}, BASE_TIMEOUT);

‎tests/smoke/snaps/test-snap-background-events.spec.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,20 @@ describe(FlaskBuildTests('Background Events Snap Tests'), () => {
119119

120120
await TestSnaps.fillMessage('backgroundEventDateInput', pastDate);
121121
await TestSnaps.tapButton('scheduleBackgroundEventWithDateButton');
122-
await Assertions.expectTextDisplayed(
123-
'Cannot schedule an event in the past.',
124-
);
122+
// iOS shows the error as a native alert; Android renders it in the
123+
// web-view result span as JSON with escaped quotes.
124+
if (device.getPlatform() === 'ios') {
125+
await Assertions.expectTextDisplayed(
126+
'Cannot schedule an event in the past.',
127+
{ timeout: 30000 },
128+
);
129+
} else {
130+
await TestSnaps.checkResultSpanIncludes(
131+
'scheduleBackgroundEventResultSpan',
132+
'Cannot schedule an event in the past.',
133+
{ timeout: 30000 },
134+
);
135+
}
125136
},
126137
);
127138
});

‎tests/smoke/snaps/test-snap-bip-32.spec.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,20 @@ describe(FlaskBuildTests('BIP-32 Snap Tests'), () => {
148148
await TestSnaps.selectInDropdown('bip32EntropyDropDown', 'Invalid');
149149
await TestSnaps.fillMessage('messageSecp256k1Input', 'bar baz');
150150
await TestSnaps.tapButton('signMessageBip32Secp256k1Button');
151-
await Assertions.checkIfTextIsDisplayed(
152-
'Entropy source with ID "invalid" not found.',
153-
);
151+
// iOS shows the error as a native alert; Android renders it in the
152+
// web-view result span as JSON with escaped quotes.
153+
if (device.getPlatform() === 'ios') {
154+
await Assertions.expectTextDisplayed(
155+
'Entropy source with ID "invalid" not found.',
156+
{ timeout: 30000 },
157+
);
158+
} else {
159+
await TestSnaps.checkResultSpanIncludes(
160+
'bip32MessageResultSecp256k1Span',
161+
'Entropy source with ID',
162+
{ timeout: 30000 },
163+
);
164+
}
154165
},
155166
);
156167
});

‎tests/smoke/snaps/test-snap-bip-44.spec.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import { FlaskBuildTests } from '../../../e2e/tags';
22
import { loginToApp, navigateToBrowserView } from '../../../e2e/viewHelper';
33
import FixtureBuilder from '../../framework/fixtures/FixtureBuilder';
44
import { withFixtures } from '../../framework/fixtures/FixtureHelper';
5-
import Assertions from '../../framework/Assertions';
65
import TestSnaps from '../../../e2e/pages/Browser/TestSnaps';
6+
import Assertions from '../../framework/Assertions';
77

88
jest.setTimeout(150_000);
99

@@ -107,9 +107,20 @@ describe(FlaskBuildTests('BIP-44 Snap Tests'), () => {
107107
await TestSnaps.selectInDropdown('bip44EntropyDropDown', 'Invalid');
108108
await TestSnaps.fillMessage('messageBip44Input', 'foo bar');
109109
await TestSnaps.tapButton('signMessageBip44Button');
110-
await Assertions.expectTextDisplayed(
111-
'Entropy source with ID "invalid" not found.',
112-
);
110+
// iOS shows the error as a native alert; Android renders it in the
111+
// web-view result span as JSON with escaped quotes.
112+
if (device.getPlatform() === 'ios') {
113+
await Assertions.expectTextDisplayed(
114+
'Entropy source with ID "invalid" not found.',
115+
{ timeout: 30000 },
116+
);
117+
} else {
118+
await TestSnaps.checkResultSpanIncludes(
119+
'bip44SignResultSpan',
120+
'Entropy source with ID',
121+
{ timeout: 30000 },
122+
);
123+
}
113124
},
114125
);
115126
});

‎tests/smoke/snaps/test-snap-get-entropy.spec.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,20 @@ describe(FlaskBuildTests('Get Entropy Snap Tests'), () => {
8585
await TestSnaps.fillMessage('entropyMessageInput', 'foo bar');
8686
await TestSnaps.tapButton('signEntropyMessageButton');
8787
await TestSnaps.approveSignRequest();
88-
await Assertions.checkIfTextIsDisplayed(
89-
'Entropy source with ID "invalid" not found.',
90-
);
88+
// iOS shows the error as a native alert; Android renders it in the
89+
// web-view result span as JSON with escaped quotes.
90+
if (device.getPlatform() === 'ios') {
91+
await Assertions.expectTextDisplayed(
92+
'Entropy source with ID "invalid" not found.',
93+
{ timeout: 30000 },
94+
);
95+
} else {
96+
await TestSnaps.checkResultSpanIncludes(
97+
'entropySignResultSpan',
98+
'Entropy source with ID',
99+
{ timeout: 30000 },
100+
);
101+
}
91102
},
92103
);
93104
});

0 commit comments

Comments
 (0)