Skip to content

Commit 5cc6a74

Browse files
test: improves mocks to allow plain text reponses (#26625)
<!-- 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** - Fix geolocation mock to return plain text (us-ca) instead of a JSON object, matching the real API's response format. The app reads this with response.text(), so a JSON object gets stringified and can produce malformed URLs when used in path segments. - Support string responses in the mock server layer — both MockServerE2E.ts and mockHelpers.ts now return string responses as raw text instead of JSON-serializing them. - Add catch-all mock for the legacy /regions/{region}/tokens endpoint to DEFAULT_RAMPS_API_MOCKS, preventing unmocked live API calls. - On mUSD convert disables the synchronization only after tapping the CTA to make sure it is displayed before on a first time user. This PR addresses a gigantic mocking error on default mocks, specifically speaking the onramp geolocation. This issue was causing random flakiness due to a bad formatted url coming from a mock. Example: `https://on-ramp-cache.uat-api.cx.metamask.io/regions/%7B%22id%22:%22/regions/us-ca%22,%22name%22:%22california%22,%22emoji%22:%22%F0%9F%87%BA%F0%9F%87%B8%22,%22detected%22:true%7D/tokens?action=deposit&sdk=2.1.5` (see [run reference](https://github.com/MetaMask/metamask-mobile/actions/runs/22429213914/job/64944977184)) The reason for this is that the request for `https://on-ramp.<ENV>-api.cx.metamask.io/geolocation` was returning ```typescript { id: region.id, name: region.name, emoji: region.emoji, detected: true, } ``` when in reality the geolocation endpoint returns plaintext containing the country code for the user's location. This PR adds the ability to the mock server to accept plain text instead of json as a response and fixes the mocking. It then caused perps tests to fail due to region not available since the [fallback country (US) is currently being blacklisted](https://github.com/MetaMask/metamask-mobile/blob/main/builds.yml#L40) and this was the country used in the onramps default geolocation mock. ## CI changes `ref` was removed from the shard runner checkout as this was causing inconsistencies with the way every single workflow uses checkout. Builds do **not** contain a ref which results in building apps with a merge commit instead of the branch commit while tests would run with a different ref resulting in failed tests for changes that were already fixed on main. <!-- 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? --> ## **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** - [ ] 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). - [ ] 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 - [ ] 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] > **Medium Risk** > Moderate risk: changes test infrastructure by installing a lifecycle-wide `unhandledRejection` filter and altering mock server shutdown behavior, which could mask unexpected promise rejections if the filter is too broad. Functional app code is untouched; impact is limited to E2E reliability and mocking fidelity. > > **Overview** > Fixes onramp geolocation mocking to return *plain text* region codes (matching the real API) and updates the mocking layer (`MockServerE2E`, `setupMockRequest`) to return raw string bodies without JSON-serializing; adds a catch-all mock for legacy `/regions/{region}/tokens` to prevent live calls. > > Hardens E2E infra by adding `MockServerE2E.startDraining()` (return 503 during cleanup) and installing a lifecycle-wide filter that suppresses mockttp `Error('Aborted')` unhandled rejections, with cleanup integrated into `withFixtures`. > > Stabilizes flaky Detox flows: waits for elements to stop moving before taps (wallet token rows, confirm button), relaxes a swap analytics assertion to *min length*, adjusts unified-buy analytics region assertions to expect a string, adds perps geolocation mocking (non-blocked region), and tweaks/simplifies a few smoke tests (mUSD sync timing, SOL send assertions). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 27fc622. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 1ae46b5 commit 5cc6a74

15 files changed

Lines changed: 260 additions & 122 deletions

File tree

.github/workflows/run-e2e-workflow.yml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,6 @@ jobs:
9696
steps:
9797
- name: Checkout
9898
uses: actions/checkout@v4
99-
with:
100-
ref: ${{ github.event_name == 'merge_group' && github.event.merge_group.head_sha || github.event.pull_request.head.sha || github.sha }}
101-
clean: true
10299

103100
- name: Install Android System Images
104101
if: ${{ inputs.platform == 'android' }}

tests/api-mocking/MockServerE2E.ts

Lines changed: 103 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@ export default class MockServerE2E implements Resource {
162162
private _testSpecificMock?: TestSpecificMock;
163163
private _activeRequests = 0;
164164
private _shuttingDown = false;
165+
private _abortFilterHandler: ((...args: unknown[]) => void) | null = null;
166+
private _abortSuppressCount = 0;
167+
private _originalRejectionHandlers: ((...args: unknown[]) => void)[] = [];
165168

166169
constructor(params: {
167170
events: MockEventsObject;
@@ -326,7 +329,10 @@ export default class MockServerE2E implements Resource {
326329

327330
return {
328331
statusCode: matchingEvent.responseCode,
329-
body: JSON.stringify(matchingEvent.response),
332+
body:
333+
typeof matchingEvent.response === 'string'
334+
? matchingEvent.response
335+
: JSON.stringify(matchingEvent.response),
330336
};
331337
}
332338

@@ -430,6 +436,27 @@ export default class MockServerE2E implements Resource {
430436
});
431437

432438
this._serverStatus = ServerStatus.STARTED;
439+
this._installAbortFilter();
440+
}
441+
442+
/**
443+
* Puts the server into draining mode — handlers return 503 immediately
444+
* without forwarding. Call this before stopping backend services (Anvil/Ganache)
445+
* to prevent forwarding requests to dead backends during cleanup.
446+
*/
447+
startDraining(): void {
448+
logger.info(
449+
'MockServer entering drain mode — new requests will receive 503',
450+
);
451+
this._shuttingDown = true;
452+
}
453+
454+
/**
455+
* Removes the lifecycle-wide abort filter. Call this AFTER all cleanup is
456+
* complete to ensure late async "Aborted" rejections are caught.
457+
*/
458+
removeAbortFilter(): void {
459+
this._removeAbortFilter();
433460
}
434461

435462
async stop(): Promise<void> {
@@ -453,7 +480,12 @@ export default class MockServerE2E implements Resource {
453480
await this._waitForActiveRequests();
454481

455482
try {
456-
await this._stopServerSuppressingAbortErrors();
483+
if (this._server) {
484+
await this._server.stop();
485+
}
486+
// Brief drain period: 'aborted' events from destroyed sockets may fire
487+
// asynchronously on the next event-loop tick after `stop()` resolves.
488+
await new Promise((resolve) => setTimeout(resolve, 150));
457489
} catch (error) {
458490
logger.error('Error stopping mock server:', error);
459491
} finally {
@@ -468,56 +500,55 @@ export default class MockServerE2E implements Resource {
468500
}
469501

470502
/**
471-
* Stops the mockttp server while suppressing "Aborted" unhandled promise rejections.
472-
*
473-
* When mockttp's `server.stop()` is called, its underlying `destroyable-server` immediately
474-
* destroys all TCP connections via `socket.destroy()`. Any `IncomingMessage` whose body was
475-
* being streamed through mockttp's `streamToBuffer` (buffer-utils.ts) will emit 'aborted',
476-
* causing the buffer promise to reject with `Error('Aborted')`.
503+
* Installs a lifecycle-wide filter for mockttp "Aborted" unhandled promise rejections.
477504
*
478-
* There is a race window where requests have entered mockttp's internal pipeline
479-
* (CallbackHandler.handle → waitForCompletedRequest → streamToBuffer) but have not yet
480-
* reached our callback (so `_activeRequests` hasn't been incremented). For these requests,
481-
* `_waitForActiveRequests()` sees zero active requests and proceeds with `stop()`, but the
482-
* body buffering is still in progress. When the socket is destroyed, the buffer promise
483-
* rejects and Jest catches it as an unhandled rejection, failing the test suite.
505+
* When the app unexpectedly drops connections during a test (e.g., UI transitions,
506+
* RN bridge interruptions), mockttp's internal `streamToBuffer` rejects with
507+
* `Error('Aborted')` from `buffer-utils.ts`. Since this happens in mockttp's pipeline
508+
* before reaching our handler callback, the rejection is unhandled and Jest catches it
509+
* as a test failure.
484510
*
485-
* This method temporarily replaces the process `unhandledRejection` handlers with a filter
486-
* that suppresses "Aborted" errors originating from mockttp's buffer-utils (verified via
487-
* stack trace), forwarding all other rejections to the original handlers.
488-
* After the server is stopped and a brief drain period elapses, the original handlers are
489-
* restored along with any handlers that were added by other code during the shutdown window.
511+
* This filter is active for the entire server lifecycle (start → stop), not just
512+
* during shutdown. It removes all existing `unhandledRejection` handlers (e.g. Jest's)
513+
* and replaces them with a single filter that suppresses mockttp Aborted errors and
514+
* forwards everything else to the original handlers. This ensures Jest never sees
515+
* the Aborted rejections.
490516
*/
491-
private async _stopServerSuppressingAbortErrors(): Promise<void> {
492-
// Snapshot current handlers so we can restore them after shutdown.
493-
const originalHandlers = process.rawListeners('unhandledRejection').slice();
517+
private _installAbortFilter(): void {
518+
if (this._abortFilterHandler) {
519+
return; // Already installed
520+
}
521+
522+
this._abortSuppressCount = 0;
523+
524+
// Snapshot and remove all existing handlers so Jest never sees Aborted errors.
525+
this._originalRejectionHandlers = process
526+
.rawListeners('unhandledRejection')
527+
.slice() as ((...args: unknown[]) => void)[];
494528
process.removeAllListeners('unhandledRejection');
495529

496-
let suppressCount = 0;
497-
const filterHandler = (reason: unknown, promise: Promise<unknown>) => {
530+
this._abortFilterHandler = (reason: unknown, promise: unknown) => {
531+
const rejectedPromise = promise as Promise<unknown>;
498532
if (
499533
reason instanceof Error &&
500534
reason.message === 'Aborted' &&
501535
reason.stack?.includes('mockttp')
502536
) {
503537
// Mark the promise as handled so Node.js does not consider it unhandled.
504538
// eslint-disable-next-line no-empty-function
505-
promise.catch(() => {});
506-
suppressCount++;
539+
rejectedPromise.catch(() => {});
540+
this._abortSuppressCount++;
507541
return;
508542
}
543+
509544
// Forward any other unhandled rejection to the original handlers (e.g. Jest).
510-
if (originalHandlers.length === 0) {
511-
// No original handlers were registered. Since our filterHandler is a
512-
// registered listener, Node considers the rejection "handled" and skips
513-
// its default behaviour (warning + exit). Re-throw so the rejection
514-
// surfaces as an uncaught exception instead of being silently dropped.
545+
if (this._originalRejectionHandlers.length === 0) {
515546
throw reason;
516547
}
517548
let firstError: unknown;
518-
for (const handler of originalHandlers) {
549+
for (const handler of this._originalRejectionHandlers) {
519550
try {
520-
(handler as (...args: unknown[]) => void)(reason, promise);
551+
handler(reason, rejectedPromise);
521552
} catch (err) {
522553
if (firstError === undefined) {
523554
firstError = err;
@@ -528,40 +559,49 @@ export default class MockServerE2E implements Resource {
528559
throw firstError;
529560
}
530561
};
531-
process.on('unhandledRejection', filterHandler);
532562

533-
try {
534-
if (this._server) {
535-
await this._server.stop();
536-
}
537-
// Brief drain period: 'aborted' events from destroyed sockets may fire
538-
// asynchronously on the next event-loop tick after `stop()` resolves.
539-
await new Promise((resolve) => setTimeout(resolve, 150));
540-
} finally {
541-
// Preserve any handlers that were added by other code during the
542-
// shutdown window so they are not permanently lost.
543-
const currentHandlers = process
544-
.rawListeners('unhandledRejection')
545-
.slice();
546-
const addedDuringShutdown = currentHandlers.filter(
547-
(h) => h !== filterHandler && !originalHandlers.includes(h),
548-
);
563+
process.on('unhandledRejection', this._abortFilterHandler);
564+
logger.debug('Installed lifecycle-wide Aborted error filter');
565+
}
549566

550-
// Remove all and restore: originals first, then any newly added handlers.
551-
process.removeAllListeners('unhandledRejection');
552-
for (const handler of [...originalHandlers, ...addedDuringShutdown]) {
553-
process.on(
554-
'unhandledRejection',
555-
handler as (...args: unknown[]) => void,
556-
);
557-
}
567+
/**
568+
* Removes the lifecycle-wide Aborted error filter and restores original handlers.
569+
* Any handlers added by other code during the filter's lifetime are preserved.
570+
*/
571+
private _removeAbortFilter(): void {
572+
if (!this._abortFilterHandler) {
573+
return;
574+
}
558575

559-
if (suppressCount > 0) {
560-
logger.info(
561-
`Suppressed ${suppressCount} "Aborted" rejection(s) during mock server shutdown`,
562-
);
563-
}
576+
// Preserve any handlers that were added by other code during the
577+
// filter's lifetime so they are not permanently lost.
578+
const currentHandlers = process.rawListeners('unhandledRejection').slice();
579+
const addedDuringLifecycle = currentHandlers.filter(
580+
(h) =>
581+
h !== this._abortFilterHandler &&
582+
!this._originalRejectionHandlers.includes(
583+
h as (...args: unknown[]) => void,
584+
),
585+
);
586+
587+
// Remove all and restore: originals first, then any newly added handlers.
588+
process.removeAllListeners('unhandledRejection');
589+
for (const handler of [
590+
...this._originalRejectionHandlers,
591+
...(addedDuringLifecycle as ((...args: unknown[]) => void)[]),
592+
]) {
593+
process.on('unhandledRejection', handler);
594+
}
595+
596+
this._abortFilterHandler = null;
597+
this._originalRejectionHandlers = [];
598+
599+
if (this._abortSuppressCount > 0) {
600+
logger.info(
601+
`Suppressed ${this._abortSuppressCount} mockttp "Aborted" rejection(s) during server lifecycle`,
602+
);
564603
}
604+
this._abortSuppressCount = 0;
565605
}
566606

567607
/**

tests/api-mocking/helpers/mockHelpers.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,9 @@ export async function setupMockRequest(
203203
)}`,
204204
);
205205
logger.debug(`Returning response:`, response.response);
206-
return {
207-
statusCode: response.responseCode ?? 200,
208-
json: response.response,
209-
};
206+
return typeof response.response === 'string'
207+
? { statusCode: response.responseCode ?? 200, body: response.response }
208+
: { statusCode: response.responseCode ?? 200, json: response.response };
210209
});
211210
}
212211

tests/api-mocking/mock-responses/defaults/onramp-apis.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ export const DEFAULT_RAMPS_API_MOCKS: MockEventsObject = {
5656
responseCode: 200,
5757
response: [],
5858
},
59+
{
60+
urlEndpoint:
61+
/^https:\/\/on-ramp-cache\.uat-api\.cx\.metamask\.io\/regions\/.*\/tokens\?.*$/,
62+
responseCode: 200,
63+
response: RAMPS_TOP_TOKENS_RESPONSE,
64+
},
5965
{
6066
urlEndpoint:
6167
/^https:\/\/on-ramp-cache\.uat-api\.cx\.metamask\.io\/v2\/regions\/[^/]+\/topTokens\?.*$/,

0 commit comments

Comments
 (0)