metamask-extension icon indicating copy to clipboard operation
metamask-extension copied to clipboard

Published 20 hours ago • verified publisher icon MetaMask

22542 add e2e test for update network

Open hjetpoluru opened this issue 1 year ago • 17 comments

Description

This PR adds test coverage for update network which is critical flow in the extension.

Related issues

Fixes:

Manual testing steps

Run the tests locally yarn yarn start:test yarn test:e2e:single test/e2e/tests/network/update-network.spec.ts --browser=chrome --debug --leave-running yarn test:e2e:single test/e2e/tests/network/update-network.spec.ts --browser=firefox --debug --leave-running

Pre-merge author checklist

  • [x] I’ve followed MetaMask Coding Standards.
  • [x] I've clearly explained what problem this PR is solving and how it is solved.
  • [ ] I've linked related issues
  • [x] I've included manual testing steps
  • [ ] I've included screenshots/recordings if applicable
  • [ ] I’ve included tests if applicable
  • [x] I’ve documented my code using JSDoc format if applicable
  • [x] I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • [ ] I’ve properly set the pull request status:
    • [ ] In case it's not yet "ready for review", I've set it to "draft".
    • [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

hjetpoluru avatar Feb 01 '24 02:02 hjetpoluru

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.

github-actions[bot] avatar Feb 01 '24 02:02 github-actions[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (3844eac) 68.47% compared to head (babc8df) 68.47%. Report is 3 commits behind head on develop.

Additional details and impacted files 
@@           Coverage Diff            @@
##           develop   #22762   +/-   ##
========================================
  Coverage    68.47%   68.47%           
========================================
  Files         1088     1088           
  Lines        42956    42956           
  Branches     11436    11436           
========================================
  Hits         29410    29410           
  Misses       13546    13546

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 01 '24 02:02 codecov[bot]

  1. Let's centralize those selectors, you're repeating some of the same ones in 3 places already. Create /test/e2e/helpers/selectors.ts and import from there.
  2. This is an optional challenge, but it would be nice to write new tests in TypeScript. We're supposed to be writing all new things in TypeScript, but few people are following it. I wrote some samples of TS E2E in the /test/e2e/accounts folder.

HowardBraham avatar Feb 01 '24 03:02 HowardBraham

@HowardBraham ,Thank you for your keen observation regarding centralizing the object selectors. I absolutely agree that there is duplication in that regard. Mariona is currently working on implementing the Page Object Model, which will be a significant framework change, and we will address this issue as part of that process.

I did explore TypeScript briefly, using your accounts as a reference, but I encountered some challenges. I plan to allocate some time today to give it another try. I agree with your recommendation that we should transition to writing our code in TypeScript.

hjetpoluru avatar Feb 01 '24 13:02 hjetpoluru

Builds ready [698d395]
Page Load Metrics (871 ± 41 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint921891322512
domContentLoaded126928168
load69910538718641
domInteractive126928168
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 01 '24 20:02 metamaskbot

Builds ready [dee828c]
Page Load Metrics (817 ± 19 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint102150119157
domContentLoaded10451894
load7569008174019
domInteractive10451894
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 05 '24 14:02 metamaskbot

I was thinking about what Mark Stacey said today with our very large CircleCI usage. In that context, it makes sense to combine and form larger tests. I tried, and this could be condensed into a single test that takes 55 seconds to run. This is true because nothing has to be "reset" in between tests.

How do other people feel about this?

HowardBraham avatar Feb 05 '24 21:02 HowardBraham

Builds ready [57034e2]
Page Load Metrics (896 ± 97 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint861651242110
domContentLoaded94820105
load755168889620397
domInteractive94720105
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 05 '24 21:02 metamaskbot

I was thinking about what Mark Stacey said today with our very large CircleCI usage. In that context, it makes sense to combine and form larger tests. I tried, and this could be condensed into a single test that takes 55 seconds to run. This is true because nothing has to be "reset" in between tests.

How do other people feel about this?

Hi @HowardBraham , Yes, I always agree with the idea of smart coupling for e2e test, It reduces execution time, it's more "end-to-end" to test a user flow, it's closer to real user's behaviour, as real user don't have a clean setup each time. Additionally, as you mentioned, it saves money on CircleCI. We do acknowledge the downside that if one step fails, the whole test will fail, making it slightly more challenging to debug. However, overall, the advantages outweigh the disadvantages, making it reasonable to combine certain our e2e tests.

chloeYue avatar Feb 06 '24 08:02 chloeYue

Builds ready [ec2d41b]
Page Load Metrics (930 ± 33 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1051731362010
domContentLoaded106628178
load80710869306933
domInteractive106628178
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 06 '24 19:02 metamaskbot

Builds ready [de97483]
Page Load Metrics (803 ± 31 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint861991112612
domContentLoaded107219136
load72410128036431
domInteractive107219136
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 07 '24 01:02 metamaskbot

Builds ready [05ef49b]
Page Load Metrics (744 ± 16 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint791159494
domContentLoaded9181331
load6858167443416
domInteractive9181331
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 07 '24 18:02 metamaskbot

Builds ready [6a70b6f]
Page Load Metrics (739 ± 14 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint831229194
domContentLoaded9261542
load6908127392814
domInteractive9261542
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 09 '24 04:02 metamaskbot

Builds ready [2079e33]
Page Load Metrics (765 ± 24 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7912597136
domContentLoaded9241742
load6978707655024
domInteractive9241742
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 153 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 09 '24 15:02 metamaskbot

Builds ready [8e45c04]
Page Load Metrics (919 ± 32 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1052831833818
domContentLoaded967292311
load81110339196632
domInteractive967292311
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 153 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 09 '24 21:02 metamaskbot

Builds ready [3e4b7a8]
Page Load Metrics (1096 ± 66 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1182862024521
domContentLoaded8110483115
load8561317109613766
domInteractive8110483115
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 153 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 12 '24 02:02 metamaskbot

Builds ready [babc8df]
Page Load Metrics (970 ± 41 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1153091754220
domContentLoaded998383014
load82311089708641
domInteractive998383014
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 153 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 13 '24 18:02 metamaskbot