[UI Tests] Added a UI Test for "Pages" dashboard card navigation. #20788

pachlava · 2023-06-01T20:33:41Z

Description

This PR adds a UI test for the "Pages" dashboard card:

What it does:

Checks that that card with expected header, context menu button, pages list and Create Page button is available at Home screen.
Taps the card header and checks that the redirect to Pages screen takes place

I hope the code is pretty self-explanatory, so I'm not providing a lot of notes on it. There are 12 changed files, however most of them are mocks, accessibility identifiers addition, and extensions for scrolling to an element.

To test

All tests are green on CI, including the new testPagesCardHeaderNavigation

Regression 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.

wpmobilebot · 2023-06-01T20:53:12Z

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr20788-b440c3d
	Version	22.4
	Bundle ID	com.jetpack.alpha
	Commit	`b440c3d`
	App Center Build	jetpack-installable-builds #4898

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot · 2023-06-01T20:57:20Z

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr20788-b440c3d
	Version	22.4
	Bundle ID	org.wordpress.alpha
	Commit	`b440c3d`
	App Center Build	WPiOS - One-Offs #5868

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

pachlava · 2023-06-02T08:41:33Z

WordPress/UITestsFoundation/Screens/MySiteScreen.swift

+    func scrollToCard(withId id: String) {
+        let collectionView = app.collectionViews.firstMatch
+        let cardCell = collectionView.cells.containing(.any, identifier: id).firstMatch
+        app.scrollDownToElement(element: cardCell)
+    }


Since there are two cards now - "Domains" and "Pages" (and more coming) - that need scrolling, I extracted the scrolling part to a separate method. The major change here is that I'm not using scrollIntoView extension any longer, because it appeared unstable for the case when more than one swipe is needed to scroll to the element. Instead, I'm using newly added scrollDownToElement.

jostnes

thanks for adding the tests @pachlava! i left out reviewing the scrolling bit a part of this PR as i saw that it is updated again the other PR.

i added a couple of nitpicks but the reason i have not approved is that i noticed that we might end up in a false positive in the case where pages are listed on the dashboard card but not listed on the pages screen.

i'm not sure if the test is going to be extended or not (i.e there will be a separate pages test to validate the value on pages screen), if it is then this can be ✅ if not, then we should update the assertions to include checking the pages.

jostnes · 2023-06-05T03:14:04Z

API-Mocks/WordPressMocks/src/main/assets/mocks/mappings/wpcom/dashboard/dashboard.json

+          "modified": "2023-05-18 10:33:38",
+          "date": "2023-05-18 10:33:38"


i've seen in some mocks where the date value is set to today's date. would having this (and other dates in this response) hardcoded cause any issues in the future? (e.g. page not appearing because it's an old page or something like that)

As long as this is not something time sensitive (not stats / messages / reviews / posts), the pages should be still shown in the card regardless of creation time 🤔. I haven't heard of any requirements for this card not to show the pages of a certain age. So if the card stops showing them, this would be an issue.

jostnes · 2023-06-05T03:46:10Z

WordPress/UITestsFoundation/Screens/MySiteScreen.swift

+        XCTAssertTrue(pagesCardHeaderButton.waitForIsHittable(), "Pages card: header not displayed.")
+        XCTAssertTrue(pagesCard.buttons["More"].waitForIsHittable(), "Pages card: context menu not displayed.")


super nit:

Suggested change

XCTAssertTrue(pagesCardHeaderButton.waitForIsHittable(), "Pages card: header not displayed.")

XCTAssertTrue(pagesCard.buttons["More"].waitForIsHittable(), "Pages card: context menu not displayed.")

XCTAssertTrue(pagesCardHeaderButton.waitForIsHittable(), "Pages card: Header not displayed.")

XCTAssertTrue(pagesCard.buttons["More"].waitForIsHittable(), "Pages card: Context menu not displayed.")

Thanks! I updated this, and another neighbouring message in 865dd41.

jostnes · 2023-06-05T03:49:34Z

WordPress/UITestsFoundation/Screens/MySiteScreen.swift

+        XCTAssertTrue(pagesCardHeaderButton.waitForIsHittable(), "Pages card: header not displayed.")
+        XCTAssertTrue(pagesCard.buttons["More"].waitForIsHittable(), "Pages card: context menu not displayed.")


just curious to know if there are any reasons why the More button is not made a property like the rest of the buttons?

Firstly, I wanted to reply with:
This is because "More" button is not used anywhere else besides this assertion (e.g. not tapped). Creating a getter for it in this case would not be justified, IMO (however, it's a matter of preference, too).

However, I then noticed that Create Page button is also used only in assertion once, but still has a getter.

To make it consistent, I created a getter for More button too in 865dd41.

jostnes · 2023-06-05T04:01:55Z

WordPress/UITests/Tests/DashboardTests.swift

            .verifyPlanSelectionScreenLoaded()
    }
+
+    func testPagesCardHeaderNavigation() throws {


is this test also only for Jetpack? i see that the first test in this test class has a comment on it // This test is JP only. what do you think about adding a comment for the whole test class instead of per test so it's clearer that it's Jetpack only?

Good point! Done in 865dd41.

jostnes · 2023-06-05T04:11:25Z

WordPress/UITests/Tests/DashboardTests.swift

+            .verifyPagesCard(hasPage: "Blog")
+            .verifyPagesCard(hasPage: "Shop")
+            .verifyPagesCard(hasPage: "Cart")


i wonder if we need to have three pages cards for this test. would be fine to test with just 2? (to test single vs multiple cases, that way it's less time for test validation and fewer files to maintain)

Just to be sure that we're talking about the same thing: this function checks that the card contains a link to the page. It's not testing a separate card. According to project specs, the card should display three pages (if the website has three pages).

that way it's less time for test validation

Since it's a check for a single static text presence, the assertion takes does not take that long. In fact, the execution time of this test as is, and with all three pages checks disabled, one time was the same, and one time it was 1 second longer 🙃.

and fewer files to maintain

All the pages that should be listed in the Pages card are defined in a single file: dashboard.json, so the maintenance effort does not change depending on the number of pages we check in a card.

jostnes · 2023-06-05T04:43:36Z

WordPress/UITestsFoundation/Screens/PagesScreen.swift

+
+    @discardableResult
+    public func verifyPagesScreenLoaded() -> Self {
+        XCTAssertTrue(PagesScreen.isLoaded(), "\"Pages\" screen isn't loaded.")


depending if this test will be extended again or not, i think we could end up with a false positive if we're only checking for the PagesTable table value, i tested with an empty table and saw that this returned
Table, 0x7ff76e80c5c0, {{0.0, 137.0}, {390.0, 624.0}}, identifier: 'PagesTable', label: 'Empty list'

if we're expecting the existing pages to also be listed, we should add additional assertions to validate them so we don't get a passing test when an empty page view is returned.

Since I was adding this test as a part of "Pages and Activity Log cards" project, I did not test the "Pages List" screen content, because the test is about card content and working navigation to the Pages List screen. The Pages List screen itself exists for a long time, so automating the check for it would be out of scope.

The same approach is used in testFreeToPaidCardNavigation, where the test checks that the Domains screen is loaded, but does not check for actual domains suggestions. Let me know if this makes sense, though.

However 😆, the time it took to add the assertions for Pages being present in Pages Screen was around 2 minutes, and the test execution time did not suffer anyhow. I've added this with a9e2df1.

peril-wordpress-mobile · 2023-06-05T13:17:58Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

pachlava · 2023-06-05T14:21:04Z

Thank you for your review and questions @jostnes 🙇 I answered all the comments from you, and made a few changes. Could you have a look please?

i'm not sure if the test is going to be extended or not

This test checks the card navigation by tapping the card header (as in all the other tests for cards). There potentially can be more tests:

Test context menu
Test tapping individual page
Test "Create Page" button

However, I don't think we should add them within the current infra and run on every commit. There are more valuable flows that are not automated, IMO, and spending both writing and execution time of fully testing each card (please remember that there will be at least 4 cards) is not justified. We might want to have them, for the case of daily execution, but we're not there yet.

UPD: I've added checking that pages are shown on Pages Screen with a9e2df1.

jostnes

This test checks the card navigation by tapping the card header (as in all the other tests for cards). There potentially can be more tests:

Test context menu
Test tapping individual page
Test "Create Page" button

However, I don't think we should add them within the current infra and run on every commit. There are more valuable flows that are not automated, IMO, and spending both writing and execution time of fully testing each card (please remember that there will be at least 4 cards) is not justified. We might want to have them, for the case of daily execution, but we're not there yet.

Yup agree with this, the reason I asked if the test was going to be extended was more related to the potential false positive (which has been updated 🙇 ) the rest of the tests from the list above I agree we shouldn't run on every commit.

And thank you for the responses and the updates! The test works well locally and is 🟢 in CI

jostnes · 2023-06-06T02:47:12Z

WordPress/UITests/Tests/DashboardTests.swift

    }

-    // This test is JP only.
+


super nit - we can remove this extra line now the comment is moved

…ard-ui-test [UI Tests] Added a UI Test for "Activity Log" dashboard card navigation.

pachlava requested a review from a team as a code owner June 1, 2023 20:33

pachlava marked this pull request as draft June 1, 2023 20:33

pachlava added the Testing Unit and UI Tests and Tooling label Jun 2, 2023

pachlava added this to the Someday milestone Jun 2, 2023

pachlava force-pushed the add-pages-card-ui-test branch 4 times, most recently from b1cadd9 to 5e176b8 Compare June 2, 2023 08:24

Added testPagesCardHeaderNavigation MK1

d3a0fff

pachlava force-pushed the add-pages-card-ui-test branch from 5e176b8 to d3a0fff Compare June 2, 2023 08:35

pachlava commented Jun 2, 2023

View reviewed changes

pachlava changed the title ~~[UI Tests] Added testPagesCardHeaderNavigation MK1~~ [UI Tests] Added a UI Test for "Pages" dashboard card navigation. Jun 2, 2023

pachlava marked this pull request as ready for review June 2, 2023 11:17

pachlava requested a review from startuptester June 2, 2023 11:17

Added testActivityLogCardHeaderNavigation.

3351220

pachlava mentioned this pull request Jun 2, 2023

[UI Tests] Added a UI Test for "Activity Log" dashboard card navigation. #20796

Merged

jostnes reviewed Jun 5, 2023

View reviewed changes

Minor refactoring in Pages card test area.

865dd41

Adjusted static texts that are asserted.

cd980c4

pachlava requested a review from jostnes June 5, 2023 14:21

pachlava added 2 commits June 5, 2023 17:43

Added an assertion for Pages Screen content

a9e2df1

Added a check for activity log screen content too.

c824165

pachlava mentioned this pull request Jun 5, 2023

[UI Tests] Added a UI test for "Pages" dashboard card navigation. wordpress-mobile/WordPress-Android#18578

Merged

jostnes approved these changes Jun 6, 2023

View reviewed changes

pachlava and others added 2 commits June 6, 2023 13:36

Removed an extra line.

c7c3b30

Merge pull request #20796 from wordpress-mobile/add-recent-activity-c…

b440c3d

…ard-ui-test [UI Tests] Added a UI Test for "Activity Log" dashboard card navigation.

pachlava merged commit 67b0494 into trunk Jun 7, 2023

pachlava deleted the add-pages-card-ui-test branch June 7, 2023 10:39

		"modified": "2023-05-18 10:33:38",
		"date": "2023-05-18 10:33:38"

		XCTAssertTrue(pagesCardHeaderButton.waitForIsHittable(), "Pages card: header not displayed.")
		XCTAssertTrue(pagesCard.buttons["More"].waitForIsHittable(), "Pages card: context menu not displayed.")

[UI Tests] Added a UI Test for "Pages" dashboard card navigation. #20788

[UI Tests] Added a UI Test for "Pages" dashboard card navigation. #20788

Uh oh!

Conversation

pachlava commented Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

To test

Regression Notes

Uh oh!

wpmobilebot commented Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jostnes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peril-wordpress-mobile bot commented Jun 5, 2023

Uh oh!

pachlava commented Jun 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jostnes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pachlava commented Jun 1, 2023 •

edited

Loading

wpmobilebot commented Jun 1, 2023 •

edited

Loading

wpmobilebot commented Jun 1, 2023 •

edited

Loading

pachlava commented Jun 5, 2023 •

edited

Loading