-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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: [POM] create ResetPasswordPage for e2e tests and migrate 2 test files to POM #27244
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. |
Quality Gate passedIssues Measures |
Builds ready [4618b4a]
Page Load Metrics (1810 ± 98 ms)
Bundle size diffs
|
LGTM ! |
@@ -33,6 +33,19 @@ class AccountListPage { | |||
'.multichain-account-menu-popover__list--menu-item-hidden-account [data-testid="account-list-item-menu-button"]'; | |||
} | |||
|
|||
async check_pageIsLoaded(): Promise<void> { |
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.
nit: maybe to follow the same as what we did in the other PR, I would suggest to place the checks
at the end
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.
oh or maybe is it better to order everything alphabetically? As I see that in other places it is ordered alphabetically including the check
s. In that case we can leave this as it is, but we can modify the order of the other page (linked PR above), in another PR
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.
Thanks for pointing out @seaona ! What I'm thinking for the order is: check_pageIsLoaded()
first, followed by action methods
ordered alphabetically, and then all check_ methods
ordered alphabetically(as they all start with check_
).
The reason for placing check_pageIsLoaded()
first is that all page classes will have this specific method, and when we create an object for the page, we usually need to call it before performing other actions. Thus, putting this specific method at the beginning will make it easier to find.
What do you think about this approach? If you agree, we can make it a standard.
…wed by action and check methods alphabetically
717e1b2
Quality Gate passedIssues Measures |
Builds ready [c541f90]
Page Load Metrics (1763 ± 54 ms)
Bundle size diffs
|
Missing release label release-12.5.0 on PR. Adding release label release-12.5.0 on PR and removing other release labels(release-12.6.0), as PR was added to branch 12.5.0 when release was cut. |
Description
This pull request creates the ResetPasswordPage class for the E2E tests page object model and migrates 2 test files in
test/e2e/tests/account
to POM and TypeScript, with enhanced log messages.I have placed the test
test/e2e/tests/account/metamask-responsive-ui.spec.js
outside the account folder because these tests don't test account functionality. They are for testing the extension UI in small viewports.Related issues
Fixes: #27245
Manual testing steps
Check code readability, make sure tests pass.
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist