test: migrate more ocurrences to FixtureBuilderV2#41760
Merged
Conversation
Contributor
✨ Files requiring CODEOWNER review ✨🫰 @MetaMask/core-platform (1 files, +4 -2)
|
Contributor
Builds ready [6b03fc7] [reused from d9bf0ba]
⚡ Performance Benchmarks (Total: 🟢 3 pass · 🟡 2 warn · 🔴 0 fail)
Bundle size diffs
|
Contributor
Builds ready [f4c94b9] [reused from 877452e]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 11 warn · 🔴 1 fail)
Bundle size diffs
|
Contributor
Builds ready [955a499] [reused from 3bbcc6b]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 12 warn · 🔴 1 fail)
Bundle size diffs
|
Contributor
Builds ready [f839697] [reused from 3bbcc6b]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 12 warn · 🔴 1 fail)
Bundle size diffs
|
Contributor
Builds ready [e79838e] [reused from 3bbcc6b]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 12 warn · 🔴 1 fail)
Bundle size diffs
|
Contributor
Builds ready [570f104] [reused from 3bbcc6b]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 12 warn · 🔴 1 fail)
Bundle size diffs
|
Contributor
Builds ready [72cec3b] [reused from 3bbcc6b]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 12 warn · 🔴 1 fail)
Bundle size diffs
|
|
Contributor
Builds ready [b64c461]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ 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 b64c461. Configure here.
| .withEnabledNetworks({ eip155: { '0x1': true } }); | ||
| const fixtureBuilder = new FixtureBuilderV2().withEnabledNetworks({ | ||
| eip155: { '0x1': true }, | ||
| }); |
There was a problem hiding this comment.
Bridge benchmark lost mainnet network selection
Medium Severity
The migration dropped the withNetworkControllerOnMainnet() call without an equivalent in FixtureBuilderV2. The old code explicitly selected mainnet as the active network (selectedNetworkClientId: 'mainnet'), but the new code only calls withEnabledNetworks. The default fixture's selectedNetworkClientId points to the localhost network client ID, so this benchmark now runs with localhost selected instead of mainnet, which changes the bridge benchmark's behavior.
Reviewed by Cursor Bugbot for commit b64c461. Configure here.
Member
Author
There was a problem hiding this comment.
that is a good point. The test work because using the enabled networks, selects that given network in the UI. But it's true that the selected network remains as it was.
I will add some description and the selected network in a follow up PR
| fixtures: new FixtureBuilder() | ||
| fixtures: new FixtureBuilderV2() | ||
| .withNetworkControllerDoubleNode() | ||
| .withPreferencesControllerSmartTransactionsOptedOut() |
There was a problem hiding this comment.
Smart transactions manifest flag override silently removed
Medium Severity
The migration removed the manifestFlags: { testing: { disableSmartTransactionsOverride: true } } option alongside the fixture builder swap. The old FixtureBuilder code included this manifest flag to fully disable smart transactions in tests. The V2 withSmartTransactionsOptedOut() only sets the preference, but the manifest flag override could re-enable smart transactions regardless, potentially causing different transaction routing behavior in this test.
Reviewed by Cursor Bugbot for commit b64c461. Configure here.
Member
Author
There was a problem hiding this comment.
that's expected as there's no migration run now
| .withSnapsPrivacyWarningAlreadyShown() | ||
| .withPermissionControllerConnectedToTestDapp({ | ||
| account: ACCOUNT_2, | ||
| }) |
There was a problem hiding this comment.
Contract interaction test lost hardfork configuration
Medium Severity
The migration removed localNodeOptions: { hardfork: 'london' } from the piggybank contract interaction test. The London hardfork setting was explicitly configured to control EIP-1559 gas behavior. The default hardfork may differ, which could change gas estimation and transaction confirmation behavior for the contract deposit test.
Reviewed by Cursor Bugbot for commit b64c461. Configure here.
Member
Author
There was a problem hiding this comment.
that's intended as we now have prague as default (latest one)
hjetpoluru
approved these changes
Apr 15, 2026
Mrtenz
approved these changes
Apr 15, 2026
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.
5 participants
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
Migrate more entries to FixtureBuilderV2
Changelog
CHANGELOG entry:
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Mostly test-only refactors, but it expands and changes
FixtureBuilderV2fixture generation (accounts and permission scopes), which could affect many E2E specs/benchmarks if the new defaults diverge from legacy fixtures.Overview
Migrates additional E2E specs, benchmarks, and helper scripts from legacy
FixtureBuildertoFixtureBuilderV2, updating fixture setup calls (e.g., smart-tx opt-out, enabled networks, snap privacy warning) and aligning tests to shared address constants.Extends
FixtureBuilderV2with new convenience helpers (notablywithAccountsControllerAdditionalAccountVaultandwithPermissionControllerConnectedToMultichainTestDapp) and makes permission fixtures more flexible by allowing customscopesto be injected instead of always deriving them fromchainIds.Refactors fixture account
methodslists into shared constants and updates a few test expectations/permissions setup (e.g.,eth_accountsorder, connected network counts in multi-provider connection tests).Reviewed by Cursor Bugbot for commit b64c461. Bugbot is set up for automated code reviews on this repo. Configure here.