test(firefox): extract xpi cache helper#41453
Merged
itsyoboieltr merged 26 commits intomainfrom Apr 9, 2026
Merged
Conversation
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. |
Contributor
Builds ready [ce49951]
⚡ Performance Benchmarks (Total: 🟢 18 pass · 🟡 0 warn · 🔴 0 fail)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs
|
Contributor
Builds ready [3a117b0]
⚡ Performance Benchmarks (Total: 🟢 17 pass · 🟡 1 warn · 🔴 0 fail)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Contributor
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/extension-platform (8 files, +253 -38)
👨🔧 @itsyoboieltr (8 files, +253 -38)
|
Contributor
Builds ready [701e1c9]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 12 warn · 🔴 0 fail)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Firefox E2E XPI build/cache logic into a shared helper and updates the build pipeline to optionally embed a build identifier into emitted extension manifests for test builds.
Changes:
- Extracted Firefox XPI caching/build logic into
test/e2e/helpers/xpi.tsand updated the Firefox WebDriver to use it. - Added an optional
setBuildIdflag toManifestPluginand enabled it for--testwebpack builds to emitbuild_idin the manifest. - Expanded webpack tests to cover
setBuildIdbehavior and verifybuild_idstability across no-op watch rebuilds.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/webdriver/firefox.js | Switches Firefox driver setup to use the new XPI helper. |
| test/e2e/helpers/xpi.ts | New helper implementing cached XPI creation and manifest patching. |
| development/webpack/webpack.config.ts | Enables manifest build_id emission for --test builds. |
| development/webpack/utils/plugins/ManifestPlugin/types.ts | Adds setBuildId option type definition. |
| development/webpack/utils/plugins/ManifestPlugin/schema.ts | Adds setBuildId to plugin option schema. |
| development/webpack/utils/plugins/ManifestPlugin/index.ts | Emits build_id into the manifest when enabled. |
| development/webpack/test/webpack.config.test.ts | Adds tests for --test enabling build IDs + watch rebuild stability. |
| development/webpack/test/plugins.ManifestPlugin.test.ts | Adds coverage for build_id emission and refines watch rebuild tests. |
| development/webpack/test/helpers.ts | Updates mock webpack compiler shape for modifiedFiles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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 ddeadaf. Configure here.
Contributor
Builds ready [ddeadaf]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Contributor
Builds ready [f1cf4a7]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs
|
seaona
approved these changes
Apr 9, 2026
Member
seaona
left a comment
seaona
left a comment
There was a problem hiding this comment.
nice optimitzation 🔥 overall LGTM! Just added a non-blocking question
|
|
||
| const MANIFEST_FILE_NAME = 'manifest.json'; | ||
| const XPI_TEMPLATE_VERSION = 1; | ||
| const MANIFEST_SIZE = 64 * 1024; |
Member
There was a problem hiding this comment.
just wondering what could happen if the manifest file grows or some changes can break these consts values 🤔 I'm unsure how many often we could have changes impacting this
Contributor
Author
There was a problem hiding this comment.
We're currently at 4KB, and this is enough space for a 64KB manifest file.
If the manifest.json file becomes too large the tests will fail with Manifest file is too large (${MANIFEST_SIZE} bytes max).
itsyoboieltr
approved these changes
Apr 9, 2026
| * @throws Will throw an error if the manifest file cannot be read or is too large | ||
| */ | ||
| async function getManifest(absExtDir: string) { | ||
| await using fd = await open(join(absExtDir, MANIFEST_FILE_NAME)); |
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 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 extracts the Firefox XPI cache/build logic into
test/e2e/helpers/xpi.ts, updates the Firefox WebDriver path to use that helper, and refreshes the benchmark so the optimized scenarios exercisegetOrBuildXpi(...)through the realistic filesystem flow. We no longer use the linuxziputility, and instead use only JavaScript (making use of Node.js modules).Behavior note: the previous Firefox cache logic rebuilt when
manifest.jsoncontent changed or when any non-manifest file had anmtimenewer than the cached XPI. The new helper no longer scans mtimes. It reuses on exact manifest-hash match, patches only when the manifest hash changed butbuild_idstayed the same, and fully rebuilds when the manifest hash changed andbuild_iddiffers or is missing.I ran some Benchmarks:
TL;DR:
Cold start is about 13% faster than before (~767ms faster), warm start is slightly faster (~60ms faster), warm start with a manifest change is about 25% faster (~1353ms faster).
Run: https://github.com/MetaMask/metamask-extension/actions/runs/24053569816
Refs:
main:c6abb14bf8734f99503767a58ebbf676a704f3becandidate:be5469700ca92278d8254ea4009d37e14b855fa6Iterations:
15Summary
Breakdown
Cold
Warm Unchanged
Warm Manifest Update
Notes
warmUnchangedis no longer a regression;installExtensionis about56 msfaster on average.warmManifestUpdatestill shows the biggest gain, at about1353 msfaster on average.warmManifestUpdatemeans a manifest-only change wherebuild_idstays the same. Real runtime bundle changes now force a full XPI rebuild instead of an in-place patch.builder.buildonmainstill has a huge outlier, so medians are more trustworthy than means for that phase.Changelog
CHANGELOG entry: null
Manual testing steps
rm -f /tmp/metamask-e2e-*.xpi yarn build:test:mv2[Firefox E2E] Cache missing or invalid, building XPIonce and Firefox installs the extension successfully.Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Build/test tooling now emits a
build_idinto manifests for test/watch builds and uses it to drive Firefox XPI caching/patching; mistakes could cause flaky watch rebuilds or broken Firefox addon installs in CI/local e2e runs.Overview
Adds optional manifest build IDs for test builds.
ManifestPlugingains asetBuildIdoption (schema/types/config) that injectsbuild_id(fromcompilation.fullHash) into emittedmanifest.jsonwithout mutating the in-memory manifest, with new/updated unit tests and watch-mode coverage.Revamps Firefox e2e XPI caching to be pure Node and build-id aware. Introduces
test/e2e/helpers/xpi.tsto build/cache XPIs viayazland to patch only the manifest in-place when the manifest hash changes butbuild_idis stable;test/e2e/webdriver/firefox.jsnow uses this helper and drops the previouszip-binary/mtime-based logic.Updates gulp manifest tasks for tests.
development/build/manifest.jscan now assign a randombuild_idfortest/testDevtasks and, fortestDev, re-apply manifest updates ondistchanges viagulp-watch.Reviewed by Cursor Bugbot for commit f1cf4a7. Bugbot is set up for automated code reviews on this repo. Configure here.