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: Signature Redesign e2e tests for each supported signature type #24352
base: develop
Are you sure you want to change the base?
Conversation
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. |
- looks like eslint it not setup for these files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably can improve the abstractions as @matthewwalsh0 is suggesting, but we should get define some guidelines on how to do so (nudging you @seaona 😅). I don't think that that's in the scope of this PR. So lgtm 🟢
a79c16d
4e6910d
…NotificationWindow
confirmations helper -> helpers
somehow, this unit test failed in this PR we should double-check this is not a flaky test separately from this PR cc: @jpuri |
https://app.circleci.com/jobs/github/MetaMask/metamask-extension/2855977 failing on the chrome-confirmation-redesign builds 🤔
|
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** created script to build script with ENABLE_CONFIRMATION_REDESIGN flag and added to test-e2e-chrome-confirmation-redesign command [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24684?quickstart=1) ## **Related issues** Fixes: #24712 Fixes: #23975 Continues and related to: #24240 ## **Manual testing steps** check ci/circleci: test-e2e-chrome-confirmation-redesign job ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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. --------- Co-authored-by: Priya Narayanaswamy <[email protected]>
- asynchronous thenable that is not awaited is not being tested
Builds ready [a50557f]
Page Load Metrics (1271 ± 582 ms)
Bundle size diffs
|
Description
Related issues
Fixes: #23977
Blocked By: #24712
Manual testing steps
ENABLE_CONFIRMATION_REDESIGN=true
in .metamaskrcScreenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist