Skip to content

Conversation

cryptotavares
Copy link
Contributor

@cryptotavares cryptotavares commented Oct 1, 2025

Description

This PR remove unnecessary mocking dependencies. The main changes focus on standardizing how token details are handled in tests by removing the useAssetDetails mock and instead relying on the background service methods (getTokenStandardAndDetails and getTokenStandardAndDetailsByChain) with a standardized decimal value of '4'. Additionally, this PR addresses several minor issues including missing React keys, console log cleanup, and prop type adjustments

Changes

  • Test Infrastructure: Added a polyfill for structuredClone using Node's v8 module to support integration tests
  • Mock Removal: Eliminated useAssetDetails mocks from 15 test files, simplifying test setup and maintenance
  • Token Decimals Standardization: Unified all test token decimal values to '4' for consistency
  • React Warning Fix: Added proper Fragment keys in simulation-details.tsx to resolve React iterator warnings
  • Prop Type Fix: Made isSigningQRHardwareTransaction optional in home.component.js as it's no longer always required
  • Code Cleanup: Removed stray console.log statement from tests

Open in GitHub Codespaces

Changelog

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

Refactors confirmation integration tests to drop useAssetDetails mocks and standardize token decimals, adds a global structuredClone for tests, fixes SimulationDetails list keying, and makes a Home prop optional.

  • Tests (Confirmations):
    • Remove useAssetDetails mocks; rely on background getTokenStandardAndDetails/getTokenStandardAndDetailsByChain with decimals: '4'.
    • Update setup helpers/mocks accordingly; tweak expectations; remove stray console.log.
  • Test Setup:
    • Add v8-based structuredClone implementation and assign to global.
  • UI:
    • ui/pages/confirmations/.../simulation-details.tsx: wrap mapped rows in Fragment with a key to fix list keying.
    • ui/pages/home/home.component.js: make isSigningQRHardwareTransaction PropTypes optional.

Written by Cursor Bugbot for commit ccd5433. This will update automatically on new commits. Configure here.

@cryptotavares cryptotavares requested review from a team as code owners October 1, 2025 22:26
Copy link
Contributor

github-actions bot commented Oct 1, 2025

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.

cursor[bot]

This comment was marked as outdated.

@github-actions github-actions bot added the size-M label Oct 3, 2025
/>
<BalanceChangesAlert transactionId={transactionId} />
</>
</Fragment>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Component Duplication in Mapping Loop

The BalanceChangesAlert component is rendered inside the staticRows.map() loop. This causes it to appear multiple times, which can lead to duplicate alerts or unexpected behavior.

Fix in Cursor Fix in Web

@metamaskbot
Copy link
Collaborator

✨ Files requiring CODEOWNER review ✨

@MetaMask/confirmations (1 files, +3 -4)
  • 📁 ui/
    • 📁 pages/
      • 📁 confirmations/
        • 📁 components/
          • 📁 simulation-details/
            • 📄 simulation-details.tsx +3 -4

👨‍🔧 @MetaMask/core-extension-ux (1 files, +1 -1)
  • 📁 ui/
    • 📁 pages/
      • 📁 home/
        • 📄 home.component.js +1 -1

@metamaskbot
Copy link
Collaborator

📊 Page Load Benchmark Results

Current Commit: ccd5433 | Date: 10/3/2025

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 1.05s (±73ms) 🟡 | historical mean value: 1.06s ⬇️ (historical data)
  • domContentLoaded-> current mean value: 736ms (±71ms) 🟢 | historical mean value: 741ms ⬇️ (historical data)
  • firstContentfulPaint-> current mean value: 77ms (±13ms) 🟢 | historical mean value: 81ms ⬇️ (historical data)
📈 Detailed Results
Metric Mean Std Dev Min Max P95 P99
pageLoadTime 1.05s 73ms 1.00s 1.32s 1.26s 1.32s
domContentLoaded 736ms 71ms 696ms 997ms 951ms 997ms
firstPaint 77ms 13ms 60ms 172ms 92ms 172ms
firstContentfulPaint 77ms 13ms 60ms 172ms 92ms 172ms
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms

Results generated automatically by MetaMask CI

@metamaskbot
Copy link
Collaborator

Builds ready [ccd5433]
UI Startup Metrics (1254 ± 76 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1254111214777613141395
load108596712767011321203
domContentLoaded107896312676911251196
domInteractive18134371740
firstPaint63472127544410751202
backgroundConnect2552422756260266
firstReactRender24166582439
getState1255271426
initialActions607511522
loadScripts829715100069878946
setupStore1063951017
WebpackHomeuiStartup20241540259826621802523
load16471249199220617801967
domContentLoaded16381239196220517701956
domInteractive191278151567
firstPaint1696336165197318
backgroundConnect281369123052
firstReactRender91383437483319
getState2953196813271
initialActions52294612
loadScripts16341236195220317671944
setupStore195292431425
FirefoxBrowserifyHomeuiStartup16031382194712016791884
load1375120015938914461542
domContentLoaded1375119915938914461542
domInteractive1173836564116281
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect3923149233888
firstReactRender33275753543
getState9417117812
initialActions4261647
loadScripts1346118115678814191512
setupStore146148181146
WebpackHomeuiStartup15821369210014916931881
load13461157162112414581547
domContentLoaded13451157162012414581546
domInteractive1083232961108298
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect322110293741
firstReactRender38325744045
getState9313718715
initialActions41828311
loadScripts13221127159412614401529
setupStore10515415924
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 58 Bytes (0%)
  • ui: -22 Bytes (0%)
  • common: 10 Bytes (0%)

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Oct 4, 2025
@cryptotavares cryptotavares added the team-confirmations Push issues to confirmations team label Oct 7, 2025
@cryptotavares cryptotavares added this pull request to the merge queue Oct 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 8, 2025
@cryptotavares cryptotavares added this pull request to the merge queue Oct 8, 2025
Merged via the queue into main with commit 5774311 Oct 8, 2025
158 of 161 checks passed
@cryptotavares cryptotavares deleted the cryptotavares/clean-up-confirmations-integration-tests branch October 8, 2025 15:30
@github-actions github-actions bot locked and limited conversation to collaborators Oct 8, 2025
@metamaskbot metamaskbot added the release-13.6.0 Issue or pull request that will be included in release 13.6.0 label Oct 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

INVALID-PR-TEMPLATE PR's body doesn't match template release-13.6.0 Issue or pull request that will be included in release 13.6.0 size-M team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants