test: add snap-simple-keyring-site as a local server and remove live requests to the site
#36557
Conversation
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
seaona
added
flaky tests
skip-e2e-quality-gate
no-changelog
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. |
Collaborator
✨ Files requiring CODEOWNER review ✨🔑 @MetaMask/accounts-engineers (3 files, +14 -5)
✅ @MetaMask/confirmations (1 files, +3 -2)
🫰 @MetaMask/core-platform (3 files, +2 -2)
🎨 @MetaMask/design-system-engineers (1 files, +2 -1)
🧩 @MetaMask/extension-devs (6 files, +50 -41)
📜 @MetaMask/policy-reviewers (6 files, +50 -41)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. 🧪 @MetaMask/qa (2 files, +47 -20)
🔗 @MetaMask/supply-chain (6 files, +50 -41)
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring alerts on:
|
seaona
commented
Oct 3, 2025
| 'https://metamask.github.io/snap-simple-keyring/1.1.6/framework-fe667a09be4a08a9b5f4.js', | ||
| 'https://metamask.github.io/snap-simple-keyring/1.1.6/page-data/app-data.json', | ||
| 'https://metamask.github.io/snap-simple-keyring/1.1.6/page-data/index/page-data.json', | ||
| 'https://metamask.github.io/snap-simple-keyring/1.1.6/webpack-runtime-f26b9ef4aabef2136bf7.js', |
Member
Author
There was a problem hiding this comment.
less live requests in our e2e, yay! 🎉
seaona
commented
Oct 3, 2025
Member
Author
There was a problem hiding this comment.
ℹ️ the snap file binary changes are just the result of updating the snap package, by running this command: yarn update-snap-binary --snap-simple-keyring-snap@2.0.0
seaona
commented
Oct 3, 2025
| export async function installSnapSimpleKeyring( | ||
| driver: Driver, | ||
| isSyncFlow: boolean = true, | ||
| port: number = 8080, |
Member
Author
There was a problem hiding this comment.
ℹ️ we add this new param, to avoid port collisions in the case we need to use both the test dapp and the snap simple keyring page
seaona
commented
Oct 3, 2025
| "@metamask/foundryup": "^1.0.1", | ||
| "@metamask/phishing-warning": "^5.0.1", | ||
| "@metamask/preferences-controller": "^19.0.0", | ||
| "@metamask/snap-simple-keyring-site": "^2.0.0", |
Member
Author
There was a problem hiding this comment.
ℹ️ we can now use the npm package
Member
Author
There was a problem hiding this comment.
@MetaMask/policy-reviewers I'm adding this new dev dependency which is our snap simple keyring site, so we can use it in our e2e. That pulls in UI libs (MUI, Popper, Emotion/styled-components), which trigger policy entries.
- devicePixelRatio global (runtime): Needed for UI layout calculations
- navigator.userAgentData (runtime overrides): Used for modern UA feature checks by positioning libs
seaona
changed the title
snap-simple-keyring-site as a local serversnap-simple-keyring-site as a local server and remove live requests to the site
FrederikBolding
pushed a commit
to MetaMask/snaps-registry
that referenced
this pull request
Oct 3, 2025
<!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues or other links reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> Context: The update to 2.0.0 doesn't change anything on the snap-simple-keyring. It's a major update, because we have made the snap-simple-keyring-site publishable, in order to use it in our e2e as a local server. See more here: - MetaMask/snap-simple-keyring#163 - MetaMask/metamask-extension#36557
Member
Author
|
@metamaskbot update-policies |
Collaborator
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
Collaborator
|
❌ test-e2e-chrome-api-specs failed. View the html report here. |
Collaborator
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Results generated automatically by MetaMask CI |
Collaborator
Builds ready [2ae9ae5]
UI Startup Metrics (1243 ± 72 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Collaborator
Builds ready [1d4109c]
UI Startup Metrics (1358 ± 86 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
jvbriones
previously approved these changes
Oct 7, 2025
seaona
commented
Oct 7, 2025
| ...(args.test ? ['ret_nodes', 'browser', 'chrome', 'indexedDB'] : []), | ||
| ...(args.test | ||
| ? ['ret_nodes', 'browser', 'chrome', 'indexedDB', 'devicePixelRatio'] | ||
| : []), |
gantunesr
previously approved these changes
Oct 7, 2025
Member
gantunesr
left a comment
gantunesr
left a comment
There was a problem hiding this comment.
Approved for Accounts CO,
test/e2e/tests/multichain-accounts/multichain-account-list-menu.spec.tstest/e2e/tests/multichain-accounts/multichain-account-list-page.spec.ts
amandaye0h
previously approved these changes
Oct 8, 2025
seaona
dismissed stale reviews from amandaye0h, gantunesr, and jvbriones
via
d79b247
Member
Author
|
@metamaskbot update-policies |
Collaborator
|
No policy changes |
Collaborator
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Results generated automatically by MetaMask CI |
Collaborator
Builds ready [d79b247]
UI Startup Metrics (1287 ± 79 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
amandaye0h
approved these changes
Oct 8, 2025
Mrtenz
approved these changes
Oct 8, 2025
jvbriones
approved these changes
Oct 8, 2025
seaona
added a commit
to MetaMask/snap-account-abstraction-keyring
that referenced
this pull request
Oct 8, 2025
…le (#215) <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues or other links reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Description Similar to what we did for the snap-simple-keyring-site, we are now doing the same for the account abstraction keyring site: we make the site publishable as an npm package, so we can then use that in our e2e as a local server, instead of going to the live site. See PRs for context: - MetaMask/snap-simple-keyring#163 - MetaMask/snap-simple-keyring#166 - MetaMask/snap-simple-keyring#168 - MetaMask/metamask-extension#36557 <!-- Are there any examples of this change being used in another repository? When considering changes to the MetaMask module template, it's strongly preferred that the change be experimented with in another repository first. This gives reviewers a better sense of how the change works, making it less likely the change will need to be reverted or adjusted later. --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Makes the site package publishable on npm and updates CI to cache the built `public` directory. > > - **Package (`packages/site/package.json`)**: > - Add `description`, `files: ["public"]`, and `publishConfig.access: "public"`; remove `private` to enable npm publishing. > - **CI (`.github/workflows/publish-release.yml`)**: > - Cache site build output by adding `./packages/site/public` to cache paths in `publish-release`, `publish-npm-dry-run`, and `publish-npm` jobs. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 331f5ce. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
naugtur
approved these changes
Oct 8, 2025
Contributor
naugtur
left a comment
naugtur
left a comment
There was a problem hiding this comment.
approving on behalf of @MetaMask/policy-reviewers
Policy changes look unsubstantial
@popperjs/core has access to document and seems to have changed a bit. I can see in the lockfile that it was updated. But the version it was updated to is 2yo anyway, so all good on the malicious package risk front
DDDDDanica
approved these changes
Oct 8, 2025
jpuri
approved these changes
Oct 9, 2025
ccharly
reviewed
Oct 9, 2025
| languageName: node | ||
| linkType: hard | ||
|
|
||
| "@metamask/keyring-api@npm:^8.1.3": |
Contributor
There was a problem hiding this comment.
This one is expected since our SSK is using a very old keyring-api version, I do have a PR to update it:
metamaskbot
added
the
release-13.6.0
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
This PR implements a localserver for the
snap-simple-keyring-siteinstead of going to the live site, in our e2e, to avoid flakiness. This implies the following:snap-simple-keyring-siteas an npm package -> this causes updates in our dependencies, lavamoatyarn update-snap-binary --snap-simple-keyring@2.0.0PRs on the snap-simple-keyring-site for reference:
MetaMask/snap-simple-keyring#163
MetaMask/snap-simple-keyring#166
MetaMask/snap-simple-keyring#168
Changelog
CHANGELOG entry:
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Switches e2e to a locally served snap-simple-keyring site via proxy, updates tests/helpers, removes live allowlisted URLs, adds the site package, and adjusts LavaMoat/webpack policies with minor UI/type fixes.
@metamask/snap-simple-keyring-site(helpers.js), withdappPathssupport and site-specific path resolution.tests/account/snap-keyring-site-mocks.tsto proxymetamask.github.ioto localhost.mockSimpleKeyringSnapwithmockSnapSimpleKeyringAndSite; update tests to passdapp: trueanddappPaths: ['snap-simple-keyring-site'](and withtest-dappwhere needed).test/e2e/mock-e2e-allowlist.js.snap-simple-keyring@2.0.0.@metamask/snap-simple-keyring-sitetopackage.jsonand.depcheckrc.yml.yarn.lockrefresh due to new dependency tree (MUI, Emotion, styled-components, etc.).@material-ui/...>clsxpath,@popperjs/coreglobals incl.navigator.userAgentData, browserslist mappings, Babel traverse entries).devicePixelRatioin LavaMoat scuttle exceptions and plugin test globals.detailsingeneral-alertwith a fragment; casthelpTextinform-text-field.ReactNodeLike).Written by Cursor Bugbot for commit 1d4109c. This will update automatically on new commits. Configure here.