-
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): cross-page chaining of page objects #27155
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. |
Quality Gate passedIssues Measures |
Thanks for the implementation @dbrans . I can see a lot of improvement, though I still have one comment: |
|
||
export const getApp = (driver: Driver): App => { | ||
return new App(driver); | ||
}; |
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.
I'm wondering if we really need to add this class. For the getHeaderNavbar()
and getLoginPage()
methods, we can create an instance of HeaderNavbar()
or LoginPage()
directly when we need them. For the openSettings()
, we can do new HeaderNavbar(this.driver).openSettings()
in tests. For the login method, I think the original loginWithBalanceValidation
is more straightforward and efficient enough.
As page object model is in the adoption phase, we are trying to keep it as simple as possible to facilitate easier onboarding and usage.
Thanks for your help and assistance I will definitely have a proper look
after work I am very novers and don't understand much about it et
Kind regards Shawn
…On Mon, 16 Sept 2024, 11:39 chloeYue, ***@***.***> wrote:
Thanks for the implementation @dbrans <https://github.com/dbrans> . I can
see a lot of improvement, though I still have one comment:
We try to keep page objects and flows isolated from each other, which
means we want to avoid one page returning an instance of another page or
one flow/page method taking a page instance as a parameter. The reason for
this is to avoid circular references and simplify the usage.
In the case of complex workflows where several pages are involved, we
perform each function on the corresponding page. Then, when we move to the
next page, we create an instance for this new page and start using methods
for this new page. This way, users don't need to think a lot about the
pages, they just need to create instances and use the methods.
—
Reply to this email directly, view it on GitHub
<#27155 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BJ2YEZF6U5CZ7TZQMDJBR3LZW2RMZAVCNFSM6AAAAABOIDYSY2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJSGQZTMNZSGE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
async scrollToAndClickElement(locator) { | ||
const element = await this.findElement(locator); | ||
await this.scrollToElement(element); | ||
await this.clickElement(element); |
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.
I’m so lost it’s sad, this is new and from my cell
Description
Introduce chaining across pages objects in order to build-out in future tests.
Prior art: https://test-automation.blog/selenium/method-chaining-page-object-model/
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist