Skip to content

test(e2e): serve snap tarballs from local binaries (MMQA-1366)#29282

Open
chrisleewilcox wants to merge 17 commits into
mainfrom
MMQA-1366-snap-local-binaries
Open

test(e2e): serve snap tarballs from local binaries (MMQA-1366)#29282
chrisleewilcox wants to merge 17 commits into
mainfrom
MMQA-1366-snap-local-binaries

Conversation

@chrisleewilcox
Copy link
Copy Markdown
Contributor

@chrisleewilcox chrisleewilcox commented Apr 23, 2026

Description

Eliminates live npm registry tarball downloads during flask E2E tests by serving snap binaries from local files via MockServer.

Problem: iOS flask shards were timing out at 35m (25% shard failure rate) because installSnap() fetched tarballs live from registry.npmjs.org. The first test in a shard would hang on the registry fetch, cascade-failing every subsequent test.

Solution: Pre-download all 21 snap tarballs and serve them from MockServer's /proxy handler via testSpecificMock, matching the pattern used by the Extension team in MetaMask/metamask-extension#32555.

Changes:

  • scripts/update-snap-binary.ts — CLI tool to download snap tarballs from npm (yarn update-snap-binary --<snap>@<version>). Includes input sanitization and path traversal guards (CodeQL).
  • 42 binary files in tests/api-mocking/mock-response-data/snaps/snap-binaries-and-headers/ — pre-downloaded tarballs + response headers for all 21 snaps
  • tests/api-mocking/mock-response-data/snaps/snap-binary-mocks.tscreateSnapMock() registers mockttp rules to intercept npm registry tarball URLs and return local binaries. 21 exported convenience functions, one per snap.
  • 23 snap spec files — each now has a testSpecificMock calling the appropriate mock function(s)
  • tests/api-mocking/MockServerE2E.ts — retains the npm:@metamask/ regex allowlist bypass in isUrlAllowed(), since the ethereum-provider and multichain-provider snaps make a direct GET to a malformed npm: URI that bypasses the /proxy route
  • .gitattributes — marks snap binary .txt files as binary to prevent CRLF corruption

How requests are protected:

There are two distinct request paths for snap installation:

  1. Tarball downloads (via /proxy) — fully protected. The app fetches tarballs through GET /proxy?url=https://registry.npmjs.org/@metamask/.... Our testSpecificMock intercepts these with a forGet('/proxy').matching(...) rule. If the mock doesn't match (e.g. snap version updated but local binary wasn't refreshed), the request falls through to the proxy catch-all handler, which decodes the URL to registry.npmjs.org/.... Since registry.npmjs.org is not in the allowlist, isUrlAllowed() returns false → the request is tracked in _liveRequestsvalidateLiveRequests() fails the test. A stale binary will never silently hit the live registry.

  2. Malformed npm: URI requests (direct, not via /proxy) — harmless. The ethereum-provider and multichain-provider snaps make a direct GET to a malformed URL like https://https//npm:@metamask/ethereum-provider-example-snap. These hit the unmatched request handler, not /proxy. The npm URI bypass in isUrlAllowed() catches these so they don't fail validateLiveRequests(). They always fail with DNS errors (getaddrinfo ENOTFOUND https) because the URL is unparseable — they can never reach a live server.

Keeping binaries up to date:

Local binaries are updated manually via yarn update-snap-binary --<snap-name>@<version>. This is the same pattern the Extension team uses (MetaMask/metamask-extension#32555) — neither repo has CI automation to detect stale binaries. The safety net is that a version mismatch causes the mock to not match, triggering a clear test failure via validateLiveRequests() rather than silently falling through to the live registry.

Changelog

CHANGELOG entry: null

Related issues

Fixes: MMQA-1366
Parent: MMQA-1364

Manual testing steps

Feature: Local snap binary serving in flask E2E tests

  Scenario: Snap installs use local binaries instead of live npm registry
    Given the iOS simulator has a flask build installed
    And MockServer is running with testSpecificMock configured

    When user runs a flask snap test (e.g. test-snap-rpc.spec.ts)
    Then the snap installs successfully from local binary files
    And no requests are made to registry.npmjs.org for tarballs
    And validateLiveRequests() does not report any unmocked requests

  Scenario: Multi-snap test installs two snaps from local binaries
    Given the iOS simulator has a flask build installed

    When user runs test-snap-rpc.spec.ts (installs bip32 + json-rpc snaps)
    Then both snaps install successfully
    And cross-snap RPC produces the expected public key

Screenshots/Recordings

Before

Specs had no testSpecificMock — snap tarballs were fetched live from registry.npmjs.org through the mock server's /proxy fallback:

// test-snap-bip-32.spec.ts (before)
await withFixtures(
  {
    fixture: new FixtureBuilder().withMultiSRPKeyringController().build(),
    restartDevice: true,
    skipReactNativeReload: true,
    // no testSpecificMock — live npm fetch via /proxy fallback
  },
  async () => { ... }
);

After

Each spec now declares a testSpecificMock that registers mockttp rules to intercept the npm tarball URL and return the pre-downloaded local binary:

// test-snap-bip-32.spec.ts (after)
await withFixtures(
  {
    fixture: new FixtureBuilder().withMultiSRPKeyringController().build(),
    restartDevice: true,
    skipReactNativeReload: true,
    testSpecificMock: async (mockServer: Mockttp) => {
      await mockBip32Snap(mockServer);
    },
  },
  async () => { ... }
);

Multi-snap tests call multiple mock functions:

// test-snap-rpc.spec.ts — installs bip32 + json-rpc
testSpecificMock: async (mockServer: Mockttp) => {
  await mockBip32Snap(mockServer);
  await mockJsonRpcSnap(mockServer);
},

Pre-merge author checklist

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.

Note

Medium Risk
Medium risk because it changes Snap tarball download behavior in NpmLocation (URL rewriting in E2E) and adds a large set of binary fixtures; failures would mainly impact E2E reliability rather than production behavior due to the isE2E gate.

Overview
Stops live npm tarball downloads during E2E Snap installs by rewriting NpmLocation.fetchNpmTarball to route registry tarball fetches through the local MockServer /proxy when isE2E is true (using a new getMockServerPortInApp value populated from launch args).

Adds a local snap-binary fixture system: a new yarn update-snap-binary script to download and persist tarballs + selected response headers, .gitattributes rules to treat the .txt tarballs as binary, and snap-binary-mocks.ts helpers that register mockttp rules to serve those local binaries.

Updates Snap smoke specs to use testSpecificMock to install snaps from local binaries (and tweaks a few assertions/sync behaviors for iOS vs Android), and expands unit coverage in npm.test.ts to verify the E2E URL rewrite behavior.

Reviewed by Cursor Bugbot for commit d9a4ae6. Bugbot is set up for automated code reviews on this repo. Configure here.

chrisleewilcox and others added 7 commits April 23, 2026 08:37
…balls

Ports the snap binary download tool from Extension for MMQA-1366.
Downloads tarball and response headers from npm registry,
saves as .txt and -headers.json files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Downloaded from npm registry for offline serving during E2E tests.
Each snap has a .txt (raw tarball) and -headers.json (response headers).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Provides createSnapMock() and 21 individual mock functions that
intercept npm registry tarball downloads via the /proxy pattern
and return local binary files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…provider specs

These specs already had testSpecificMock — adds the snap tarball
mock call alongside existing feature flag and genesis block mocks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Each spec now intercepts the npm registry tarball download for its
snap and serves the local binary via MockServer's /proxy pattern.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Mocks both bip32 and json-rpc snap tarballs in the same
testSpecificMock since the spec installs both snaps.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Now that all snap tarballs are served locally via testSpecificMock,
the bypass for malformed npm: URIs is no longer needed. Any unmocked
npm registry requests will be caught by validateLiveRequests().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbotv2 metamaskbotv2 Bot added the team-qa QA team label Apr 23, 2026
Comment thread scripts/update-snap-binary.ts Dismissed
chrisleewilcox and others added 2 commits April 23, 2026 10:01
- Add .gitattributes binary marker for snap tarball .txt files to
  prevent CRLF normalization corruption
- Inline getDecodedProxiedURL to remove cross-layer import from
  smoke/notifications into the shared api-mocking layer
- Add headersPath existence check in createSnapMock for a clear error
  message when the -headers.json file is missing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The ethereum-provider and multichain-provider snaps make a direct
GET request using a malformed npm: URI that bypasses the /proxy
route. The binary mocks only intercept proxied tarball requests,
so these direct requests still need the allowlist bypass.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.96%. Comparing base (3751d9a) to head (d9a4ae6).
⚠️ Report is 262 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #29282      +/-   ##
==========================================
+ Coverage   81.54%   81.96%   +0.41%     
==========================================
  Files        5343     5446     +103     
  Lines      142128   145459    +3331     
  Branches    32411    33227     +816     
==========================================
+ Hits       115899   119224    +3325     
+ Misses      18299    18105     -194     
- Partials     7930     8130     +200     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@chrisleewilcox chrisleewilcox marked this pull request as ready for review April 23, 2026 21:23
@chrisleewilcox chrisleewilcox requested review from a team as code owners April 23, 2026 21:23
@chrisleewilcox chrisleewilcox added no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed no changelog required No changelog entry is required for this change labels Apr 23, 2026
…nary

Address CodeQL "Network data written to file" finding by validating
the snap name against path traversal characters and verifying resolved
output paths stay within the target directory.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@chrisleewilcox chrisleewilcox added skip-e2e-flakiness-detection Skips the E2E flakiness detection (extra runs on new and modified E2E files) and removed skip-e2e-flakiness-detection Skips the E2E flakiness detection (extra runs on new and modified E2E files) labels Apr 23, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

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 and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f2ef5e4. Configure here.

Comment thread tests/api-mocking/mock-response-data/snaps/snap-binary-mocks.ts Outdated
@chrisleewilcox
Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

…binaries

# Conflicts:
#	.gitattributes
#	tests/smoke/snaps/test-snap-background-events.spec.ts
#	tests/smoke/snaps/test-snap-bip-32.spec.ts
#	tests/smoke/snaps/test-snap-bip-44.spec.ts
#	tests/smoke/snaps/test-snap-client-status.spec.ts
#	tests/smoke/snaps/test-snap-cronjob.spec.ts
#	tests/smoke/snaps/test-snap-dialog.spec.ts
#	tests/smoke/snaps/test-snap-get-entropy.spec.ts
#	tests/smoke/snaps/test-snap-get-file.spec.ts
#	tests/smoke/snaps/test-snap-get-preferences.spec.ts
#	tests/smoke/snaps/test-snap-image.spec.ts
#	tests/smoke/snaps/test-snap-installed.spec.ts
#	tests/smoke/snaps/test-snap-interactive-ui.spec.ts
#	tests/smoke/snaps/test-snap-jsx.spec.ts
#	tests/smoke/snaps/test-snap-lifecycle.spec.ts
#	tests/smoke/snaps/test-snap-manage-state.spec.ts
#	tests/smoke/snaps/test-snap-management.spec.ts
#	tests/smoke/snaps/test-snap-name-lookup.spec.ts
#	tests/smoke/snaps/test-snap-rpc.spec.ts
#	tests/smoke/snaps/test-snap-uilinks.spec.ts
#	tests/smoke/snaps/test-snap-wasm.spec.ts

Co-authored-by: chrisleewilcox <6626407+chrisleewilcox@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented May 15, 2026

@copilot resolve the merge conflicts in this pull request

Resolved and pushed in 38f7282. I merged latest main into this branch, resolved all conflicts by keeping the local snap binary mocks and preserving new disableSynchronization settings from main, and combined the .gitattributes updates.

Mocks for snap tarball downloads (added in this PR) never fired because
ReactNativeBlobUtil.fetch() and config(...).fetch() invoke the native
HTTP bridge directly, bypassing shim.js's fetch/XHR/expo-fetch patches.
Tests have been silently hitting the live npm registry.

Adds a JS-layer shim that wraps RNBlobUtil's fetch APIs so HTTP/HTTPS
URLs are rewritten to MockServerE2E's /proxy?url= endpoint before the
native call. Mirrors the existing shim.js pattern for fetch/XHR/WebSocket
/expo-fetch. iOS and Android both work without OS-level proxy infra.

- app/util/test/blobUtilShim.js: rewriteBlobUtilUrl + idempotent
  patchReactNativeBlobUtilForE2E that wraps fetch and config in place
- app/util/test/blobUtilShim.test.js: 14 tests covering edge cases
  (https/http, file://, content://, RNBlobUtil-file://, already-proxied,
  non-strings, custom host, idempotency) plus an integration test that
  mirrors npm.ts's call pattern exactly
- shim.js: wires the patch into the existing isMockServerAvailable block
- tests/api-mocking/MockServerE2E.ts: extends handleDirectFetch to
  forward content-length / content-type / content-encoding /
  content-disposition / etag / last-modified upstream headers (npm.ts
  asserts content-length is present)

Runtime evidence on Android emulator (recorded in MMQA-1805):
- [SPIKE] blob-util GET -> status=200 len=17 logged from inside the app
- /proxy?url=https://registry.npmjs.org/spike-test received by mock server

Follow-up: the npm: regex bypass in MockServerE2E.ts (line 163) is left
in place. The malformed https://https//npm:@metamask/... URIs from
ethereum-provider and multichain-provider snaps now flow through the
shim to /proxy, where the URL constructor parses host as "https" — not
in any allowlist. Removing the bypass requires either a synthetic mock
matcher for the malformed pattern or an upstream Snaps fix; tracked
separately.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size-XL and removed size-L labels May 15, 2026
@github-actions github-actions Bot added size-L and removed size-XL labels May 15, 2026
ReactNativeBlobUtil.fetch() bypasses shim.js's JS-layer fetch/XHR/expo-fetch
patches because it dispatches directly to OkHttp/NSURLSession. As a result,
the testSpecificMock rules registered for snap tarball downloads never fire,
and tests silently hit the live npm registry.

Rewrite the tarball URL at the source — inside NpmLocation.fetchNpmTarball —
when running in E2E. The request body and headers contract with native
ReactNativeBlobUtil stays untouched; only the target URL changes from
`https://registry.npmjs.org/...` to
`http://localhost:<mockServerPort>/proxy?url=<encoded-target>`.

Changes:
- app/core/Snaps/location/npm.ts: rewrite tarballUrl when isE2E
- app/util/test/utils.js: export FALLBACK_MOCK_SERVER_PORT + getMockServerPortInApp helper, mirroring fixtureServerPort/commandQueueServerPort pattern
- shim.js: store launchArgs.mockServerPort in testConfig so app code can read it
- app/core/Snaps/location/npm.test.ts: 2 new tests covering the rewrite both ways (E2E on / off)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeSnaps
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: high
  • AI Confidence: 97%
click to see 🤖 AI reasoning details

E2E Test Selection:
This PR is entirely focused on the MetaMask Snaps E2E test infrastructure. The key changes are:

  1. app/core/Snaps/location/npm.ts (CRITICAL): Modified NpmLocation to route snap tarball downloads through a mock server proxy (/proxy endpoint) when isE2E is true. This is a production code change that affects how snaps are installed during E2E tests - ReactNativeBlobUtil's native fetch bypasses JS-layer fetch/XHR patches, so the redirect must happen at the source URL level.

  2. app/util/test/utils.js + shim.js: Added getMockServerPortInApp() and FALLBACK_MOCK_SERVER_PORT to support mock server port configuration via launch arguments.

  3. tests/api-mocking/mock-response-data/snaps/snap-binary-mocks.ts: New mock infrastructure that intercepts npm registry tarball downloads and serves locally-stored snap binaries.

  4. All tests/smoke/snaps/*.spec.ts files: Updated to use testSpecificMock with the new snap binary mock functions. Also removed disableSynchronization: true from subsequent test cases (only needed for the first snap install step).

  5. Snap binary data files: 22 snap binaries stored locally with their headers for offline mocking.

  6. scripts/update-snap-binary.ts: New utility to download and update snap binaries.

  7. .gitattributes: Marks snap binary .txt files as binary to prevent line ending normalization.

The entire PR is about making SmokeSnaps tests work with local binary mocking instead of live npm registry access. The production code change in npm.ts (the isE2E URL rewriting) is the critical piece that needs validation - it must correctly route snap downloads through the mock server in E2E mode while leaving production behavior unchanged. All 22+ snap test spec files have been updated to use the new mocking infrastructure.

Only SmokeSnaps is needed - no other test areas are affected by these changes.

Performance Test Selection:
These changes are entirely focused on E2E test infrastructure for Snaps - snap binary mocking, mock server routing, and test spec updates. There are no UI rendering changes, no state management changes, no component changes, and no changes to critical user flows that would impact performance metrics. The production code change in npm.ts only affects behavior when isE2E is true (E2E test mode), not in production builds.

View GitHub Actions results

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog required No changelog entry is required for this change no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed size-L team-qa QA team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants