Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add scenario for eth sign using hardware wallet #23313

Merged
merged 9 commits into from
Mar 18, 2024

Conversation

segun
Copy link
Contributor

@segun segun commented Mar 5, 2024

Description

In this PR we add the scenario for the flow: eth sign with hardware wallet.
This task belongs to the effort of documenting manual QA flows in this Epic.

Open in GitHub Codespaces

Related issues

Fixes: #22026

Manual testing steps

  1. Check the video below.

Screenshots/Recordings

Performing the steps defined in the scenario

eth_sign_ledger.mov

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • 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.

@segun segun added team-confirmations-secure-ux-PR PRs from the confirmations team needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Mar 5, 2024
@segun segun self-assigned this Mar 5, 2024
@segun segun requested a review from a team as a code owner March 5, 2024 11:21
Copy link
Contributor

github-actions bot commented Mar 5, 2024

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.

@seaona
Copy link
Contributor

seaona commented Mar 6, 2024

hey @segun the flow description looks good. I've seen that there's some csv formatting problem, since if I check the file, I don't see the table rendered.
See here.

Screenshot from 2024-03-06 16-34-54

If you check another file, you will see the table rendered. Ie.
Screenshot from 2024-03-06 16-35-02

This is likely caused by some comma missing or extra

Copy link
Contributor

@seaona seaona left a comment

Choose a reason for hiding this comment

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

See comment

@segun segun force-pushed the olu/test-scenario-eth-sign branch 2 times, most recently from 06010d6 to 393e337 Compare March 6, 2024 16:30
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.45%. Comparing base (5222d6f) to head (f173ee3).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #23313   +/-   ##
========================================
  Coverage    68.45%   68.45%           
========================================
  Files         1140     1140           
  Lines        43757    43757           
  Branches     11722    11722           
========================================
  Hits         29950    29950           
  Misses       13807    13807           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Collaborator

Builds ready [17f51e5]
Page Load Metrics (1313 ± 433 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint852501455125
domContentLoaded168534199
load7224011313902433
domInteractive168534199
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

blackdevelopa
blackdevelopa previously approved these changes Mar 12, 2024
Copy link
Contributor

@blackdevelopa blackdevelopa left a comment

Choose a reason for hiding this comment

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

Looks good. The csv is formatted as expected

@metamaskbot
Copy link
Collaborator

Builds ready [0dd6eb8]
Page Load Metrics (1599 ± 239 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint721921403014
domContentLoaded117033199
load12719181599498239
domInteractive117033199
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

jpuri
jpuri previously approved these changes Mar 13, 2024
@segun segun dismissed stale reviews from jpuri and blackdevelopa via a857216 March 14, 2024 15:41
@segun segun force-pushed the olu/test-scenario-eth-sign branch from 0dd6eb8 to a857216 Compare March 14, 2024 15:41
@metamaskbot
Copy link
Collaborator

Builds ready [6dbbe7e]
Page Load Metrics (967 ± 390 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint602151183416
domContentLoaded96331189
load512107967812390
domInteractive96331189
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Copy link
Contributor

@seaona seaona left a comment

Choose a reason for hiding this comment

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

LGTM

@segun segun requested review from jpuri and blackdevelopa March 18, 2024 08:40
@metamaskbot
Copy link
Collaborator

Builds ready [f173ee3]
Page Load Metrics (1032 ± 466 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint682451204823
domContentLoaded968272010
load5225431032970466
domInteractive968272010
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@segun segun merged commit 00828da into develop Mar 18, 2024
66 checks passed
@segun segun deleted the olu/test-scenario-eth-sign branch March 18, 2024 15:01
@github-actions github-actions bot locked and limited conversation to collaborators Mar 18, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Mar 18, 2024
@metamaskbot metamaskbot added the release-11.14.0 Issue or pull request that will be included in release 11.14.0 label Mar 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-testSuite release-11.14.0 Issue or pull request that will be included in release 11.14.0 team-confirmations-secure-ux-PR PRs from the confirmations team
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Hardware wallet: Eth Sign
7 participants