-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[UI Tests] Added a UI test for "Pages" dashboard card navigation. #18578
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
Conversation
5942bb3 to
e618135
Compare
|
| App Name | Jetpack |
|
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr18578-691f307 | |
| Commit | 691f307 | |
| Direct Download | jetpack-prototype-build-pr18578-691f307.apk |
|
| App Name | WordPress |
|
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr18578-691f307 | |
| Commit | 691f307 | |
| Direct Download | wordpress-prototype-build-pr18578-691f307.apk |
e618135 to
691f307
Compare
| } | ||
|
|
||
| private fun scrollToCard(elementID: Int): MySitesPage { | ||
| WPSupportUtils.waitForElementToBeDisplayed(elementID) |
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 added this function with #18345, this I have no idea why this line was here, it makes no sense 😕. This worked only because the Domains card was already on-screen, and did not require scrolling.
| fun scrollToDomainsCard(): MySitesPage { | ||
| return scrollToCard(R.id.dashboard_card_domain_cta) | ||
| } | ||
|
|
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.
Just moved it a few lines below, so that it belongs to the same card-related functions.
jostnes
left a comment
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.
added one non-blocking question related to , but other than that test works well locally and CI and is similar to the iOS implementation (which is approved) so it's a ✅ for me!
| ViewMatchers.hasDescendant( | ||
| Matchers.allOf( | ||
| ViewMatchers.withText(R.string.dashboard_pages_card_title), | ||
| ViewMatchers.withId(R.id.my_site_card_toolbar_title), |
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.
| ViewMatchers.withId(R.id.my_site_card_toolbar_title), | |
| ViewMatchers.withId(R.id.my_site_card_toolbar_title) |
i noticed that for multiple matches, the last matcher ends with a , which looks odd to me since it's the last one. is this intentional or is it a kotlin thing?
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.
In this particular case, this is most probably a result of me moving things around. Such a comma in the end, even if it's not followed by another line, is not considered a code style violation. I saw cases when people deliberately kept commas at the end of a line, to avoid extra noise in the future PRs when they add a next line (I left once a nitpick about this here in a PR of yours).


Description
Following the maximum similarity to its iOS counterpart, this PR adds a UI test for the "Pages" dashboard card:
What the test does:
There is some noise because I moved 3 dashboard-related mock files to a
dashboardfolder, and also renamed them.To test
e2ePagesCardNavigationRegression Notes
I'm not filling the regression notes since the PR adds a UI test, it does not change the app behaviour in any way.