Skip to content

Conversation

@dbrans
Copy link
Contributor

@dbrans dbrans commented Mar 17, 2025

Description

This PR resolves conflicting types between jest and mocha that were causing issues with the type of "it" in unit tests which were masking type errors. It also fixes type issues that were masked by previously disregarding type errors.

In particular, this PR creates an isolated TypeScript configuration for the e2e tests where they get their own type checking context. Since mocha is only needed in e2e tests, Mocha types will not be considered for files outside of that folder.

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

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.

@github-actions
Copy link
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.

@metamaskbot metamaskbot added the team-extension-platform Extension Platform team label Mar 17, 2025
@dbrans dbrans marked this pull request as ready for review March 17, 2025 17:34
@dbrans dbrans requested review from a team as code owners March 17, 2025 17:34
@metamaskbot
Copy link
Collaborator

Builds ready [2be0d1e]
Page Load Metrics (3837 ± 1457 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1497998629961762846
domContentLoaded171513951336629681425
load179414674383730351457
domInteractive2789810319091
backgroundConnect106972476270129
firstReactRender422891116933
getState36835216213102
initialActions02010
loadScripts124012171252725401219
setupStore1875014517483
uiStartup251122106677051372467
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@dbrans dbrans changed the title test: resolve jest.It.each type and create separate tsconfig for e2e tests test: fix mocha vs jest type conflicts in test files Mar 17, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [c2f5ea8]
Page Load Metrics (2750 ± 853 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1667885025691562750
domContentLoaded1608859223701519729
load1707994627501777853
domInteractive24182604421
backgroundConnect901341389290139
firstReactRender303961097737
getState2554617512459
initialActions01000
loadScripts1163701617021256603
setupStore1954213514369
uiStartup216211045476722451078
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@dbrans dbrans enabled auto-merge March 27, 2025 15:13
@metamaskbot
Copy link
Collaborator

✨ Files requiring CODEOWNER review ✨

🔑 @MetaMask/accounts-engineers

  • app/scripts/lib/snap-keyring/keyring-snaps-permissions.test.ts
  • app/scripts/lib/snap-keyring/utils/isBlockedUrl.test.ts

✅ @MetaMask/confirmations

  • app/scripts/lib/ppom/ppom-util.test.ts

🔄 @MetaMask/swaps-engineers

  • app/scripts/controllers/bridge-status/validators.test.ts
  • ui/hooks/bridge/useBridging.test.ts

Copy link
Contributor

@ccharly ccharly left a comment

Choose a reason for hiding this comment

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

Reviewed only for the accounts team related files:

app/scripts/lib/snap-keyring/keyring-snaps-permissions.test.ts
app/scripts/lib/snap-keyring/utils/isBlockedUrl.test.ts

@metamaskbot
Copy link
Collaborator

Builds ready [e9981a2]
UI Startup Metrics (1173 ± 58 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1173106513155812101289
load1023894116259950991
domContentLoaded1017887115759952991
domInteractive16132841526
firstPaint7221591169402248988
backgroundConnect96455910
firstReactRender19154961933
getState10431768
initialActions001001
loadScripts80867694558854922
setupStore7422378
WebpackHomeuiStartup953840123082953970
load81358496759838917
domContentLoaded80856695859834906
domInteractive15123561333
firstPaint46555929340826873
backgroundConnect16115281438
firstReactRender14123541326
getState6412278
initialActions001000
loadScripts80555694858833896
setupStore7515279
FirefoxBrowserifyHomeuiStartup13671172197415313871703
load12291056181914812501552
domContentLoaded12291056181914812501552
domInteractive9938183298797
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect24168992737
firstReactRender22183132428
getState7426379
initialActions001001
loadScripts12071037178114512291529
setupStore7451657
WebpackHomeuiStartup10058531567177911981
load8797461361156803941
domContentLoaded8797461361156803941
domInteractive117372733114986
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect221394132342
firstReactRender19162921925
getState1047513759
initialActions001001
loadScripts8617331329152794962
setupStore8586989
Bundle size diffs
  • background: 0 Bytes (0%)
  • ui: 0 Bytes (0%)
  • common: 0 Bytes (0%)

@dbrans dbrans removed the request for review from matthewwalsh0 March 28, 2025 17:13
Copy link
Member

@wachunei wachunei left a comment

Choose a reason for hiding this comment

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

Ramp changes LGTM

@dbrans dbrans added this pull request to the merge queue Mar 28, 2025
Merged via the queue into main with commit 1e87559 Mar 28, 2025
147 checks passed
@dbrans dbrans deleted the djb/e2e-tsconfig branch March 28, 2025 19:00
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2025
@metamaskbot metamaskbot added the release-12.17.0 Issue or pull request that will be included in release 12.17.0 label Mar 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-12.17.0 Issue or pull request that will be included in release 12.17.0 team-extension-platform Extension Platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants