Plans on JP: Update UI tests for Plan Selection #20728

staskus · 2023-05-22T18:52:16Z

Fixes #20720

Update UI tests for Plan Selection:

Update Domain card to Free to Paid card
Test Domain selection
Verify Plan selection appears after Domain selection

To test:

Wait for UI to complete

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

plan.selection.mp4

peril-wordpress-mobile · 2023-05-22T18:52:20Z

	Warnings
⚠️	This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

wpmobilebot · 2023-05-22T19:14:22Z

📲 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	pr20728-a3d5b7b
	Version	22.4
	Bundle ID	com.jetpack.alpha
	Commit	`a3d5b7b`
	App Center Build	jetpack-installable-builds #4816

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

wpmobilebot · 2023-05-22T19:14:32Z

📲 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	pr20728-a3d5b7b
	Version	22.4
	Bundle ID	org.wordpress.alpha
	Commit	`a3d5b7b`
	App Center Build	WPiOS - One-Offs #5785

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

…card

- Added Domain suggestion, product fetching, and adding to cart API mocks - Select domain and confirm selection in UI tests

guarani

👋 @staskus, thanks for adding this UI test! The code changes look good.

When I run this locally for the first time (fresh run of testFreeToPaidCardNavigation on a simulator that's had "Erase all content and settings" applied), the test gets stuck because the card is missing:

Screen.Recording.2023-05-23.at.15.58.43.mov

If I re-run the test, the card is now there and the test passes. Can you reproduce this failure on the 1st run or does it work for you?

If this is failing locally I thought it might be failing on CI on the 1st attempt and passing on the 2nd (assuming its set to retry automatically). I searched the Buildkite logs and artifacts but couldn't find evidence of this. I also looked at https://github.com/wordpress-mobile/WordPress-iOS/blob/trunk/.buildkite/commands/run-ui-tests.sh and

WordPress-iOS/fastlane/lanes/build.rb

Line 81 in 2719dfb

lane :test_without_building do |options|

but couldn't even find evidence that its set to retry automatically (e.g. via number_of_retries).

I also looked at the dashboard code and noticed that it loads (remote) cards from a cache via loadCardsFromCache when it loads local cards. It seems like the remote cards which are fetched via /wpcom/v2/sites/106707880/dashboard/cards-data/?cards=todays_stats,posts&_locale=en fail consistently in DashboardServiceRemote even thought they should load from the mock. Perhaps there's a corner case that when the remote cards fail to load so do the local ones. Then on the 2nd attempt, the cache is set (but still emty) and that allows the local ones to load. This is just a thought though since I haven't been able to verify.

Before spending more time digging, I want to check with you if it's just me seeing this failure or if you also see it.

staskus · 2023-05-24T06:27:51Z

👋 @staskus, thanks for adding this UI test! The code changes look good.

When I run this locally for the first time (fresh run of testFreeToPaidCardNavigation on a simulator that's had "Erase all content and settings" applied), the test gets stuck because the card is missing

Thanks for reviewing, @guarani! Yes, I see a similar thing. From warnings I would assume some mocks are outdated, but maybe there's an issue in the code as well that prevents a dashboard card from showing up even though it potentially could. I would need to investigate to understand more.

staskus · 2023-05-24T10:31:54Z

I investigated the issue.

Yes, there're misconfigured api-mocks related to /wpcom/v2/sites/106707880/dashboard/cards-data/?cards=todays_stats,posts&_locale=en which indirectly affects the Free to Paid Plans card.

Free to Paid Plans card depends on domains being loaded blog.domains?.isEmpty == false. On the first app launch, usually this is the flow:

Dashboard appears, blog.domains is empty, Free to Paid Plans is hidden
refreshDomains is called in syncBlogAndAllMetadata
loadCardsFromCache is called, which calls /wpcom/v2/sites/106707880/dashboard/cards-data/?cards=todays_stats,posts&_locale=en
refreshDomains finishes
loadCardsFromCache remote call finishes, with triggers the reload of the dashboard
Now blog.domains is not empty, Free to Paid Plans appears

With just setting different breakpoints, I could reproduce the same issue on debug, as we have on UI Tests. I always assumed dashboard cards are reloaded when syncBlogAndAllMetadata is called but I was wrong. They are reloaded when the remote dashboard cards call is finished or when navigating around the app.

Therefore, our initial issue in i tests was caused by /wpcom/v2/sites/106707880/dashboard/cards-data/?cards=todays_stats,posts&_locale=en failing and not triggering additional reload of the dashboard.

However, fixing mocks is not enough, since with api-mocks, /wpcom/v2/sites/106707880/dashboard/cards-data/?cards=todays_stats,posts&_locale=en call always finishes before the refreshDomains. Therefore, we need to make sure we reload dashboard cards when refreshDomains are finished.

staskus · 2023-05-24T15:12:52Z

@guarani

I assigned you for another review. However, if you prefer, we can first focus on finishing and merging other tasks and leave UI tests for the end.

pachlava

@staskus 👋

Huuuge thanks for refactoring the test 👍 It works for me locally with no issues (after e2ae033, I suppose), both after having a clean Simulator, and also on the second try. I tried iOS 16.4 and 16.2.

I left a few nitpicky comments about functions naming/format, it's not anything blocking, feel free to apply the suggestions or ignore them.

P.S. Even though the most recent Buildkite executions are marked green, the test failed for iPad on Buildkite (see JetpackUITests.xml), however from the log it's seen that this happens during Set Up, so it's not related to the test itself. Actually, the same happens to testInsightsStatsLoadProperly, also during Set Up, and looks like the legendary spinning wheel issue (cc @tiagomar for confirmation) :

pachlava · 2023-05-25T14:10:23Z

WordPress/UITestsFoundation/Screens/DomainsSuggestionsScreen.swift

    }
+
+    @discardableResult
+    public func selectDomain() throws -> Self {


Since the screen contains Select domain button, it's easy to assume that the function taps this button. However, it does a different thing, which is tapping the last cell. WDYT of renaming the function?

Suggested change

public func selectDomain() throws -> Self {

public func tapLastDomain() throws -> Self {

?

pachlava · 2023-05-25T14:12:43Z

WordPress/UITestsFoundation/Screens/DomainsSuggestionsScreen.swift

+    @discardableResult
+    public func goToPlanSelection() throws -> PlanSelectionScreen {
+        app.buttons["Select domain"].tap()
+        return try PlanSelectionScreen()
+    }


It's a question of test steps details level, just thinking aloud. Do we need this function separately? What if we perform both the domain selection and Select domain tap in a single function?

Yup, I think that would make sense!

guarani

Tested locally and working now on the 1st try. Thanks @staskus!

…ate-ui-tests There were conflicts on `WordPress/WordPress.xcodeproj/project.pbxproj` because both the working branch and `trunk` added a file. I resolved the by keeping both files and verifying the build.

mokagio · 2023-05-28T11:40:30Z

Took over the PR to resolve the merge conflict and enable auto-merge in order to get ready for the upcoming 22.5 code freeze.

mokagio · 2023-05-29T04:24:06Z

🤔 looks like after merging trunk into here the UI tests started failing consistently (as can be seen in Buildkite for the release/22.5 builds).

I'll be looking into this ASAP, but cc @staskus @guarani @pachlava in case you have ideas.

mokagio · 2023-05-29T08:26:56Z

See #20751

pachlava · 2023-05-29T09:24:13Z

The new fail seem to be related to #20731. "Search domains" screen was redesigned a bit, this is why testFreeToPaidCardNavigation started to fail.

pachlava · 2023-05-29T10:01:48Z

I have a fix here (the commit is there, but the PR is not presented well yet. I'll be able to get back to it in 2 hours). Fails were caused by the redesign of Search domains screen. Now, the screen is presented as an overlay (not sure if I'm using a correct term), instead of a usual screen. Since this is an overlay, the bottom navigation bar is not visible:

However, it was still mentioned in the init of DomainsSuggestionsScreen (as tabBar). Removing it was all what was needed.

mokagio · 2023-05-29T10:47:06Z

🎉 thanks @pachlava ! I was still looking into it but to no avail, as you can see from my PR still failing.

What do you make of all the failures like:

[09:55:52]: ▸ 2023-05-29 09:55:52.402 xcodebuild[16392:31485] DVTCoreDeviceEnabledState: DVTCoreDeviceEnabledState_Disabled set via user default (DVTEnableCoreDevice=disabled)
  | [09:59:42]: ▸ 2023-05-29 09:59:42.663 xcodebuild[16392:38281]  iOSSimulator: 62F876DC-0EF7-4A04-8B65-4D8EBECF0C75: Failed to launch app with identifier: com.automattic.jetpack and options: {
  | [09:59:42]: ▸     "activate_suspended" = 0;
  | [09:59:42]: ▸     arguments =     (
  | [09:59:42]: ▸         "-AppleLanguages",
  | [09:59:42]: ▸         "(en)",
  | [09:59:42]: ▸         "-AppleTextDirection",
  | [09:59:42]: ▸         NO,
  | [09:59:42]: ▸         "-AppleLocale",
  | [09:59:42]: ▸         "en_US",
  | [09:59:42]: ▸         "-wpcom-api-base-url",
  | [09:59:42]: ▸         "http://localhost:8282/",
  | [09:59:42]: ▸         "-no-animations",
  | [09:59:42]: ▸         "-ui-testing",
  | [09:59:42]: ▸         "-com.apple.CoreData.ConcurrencyDebug",
  | [09:59:42]: ▸         1
  | [09:59:42]: ▸     );
  | [09:59:42]: ▸     environment =     {
  | [09:59:42]: ▸         "CA_ASSERT_MAIN_THREAD_TRANSACTIONS" = 0;
  | [09:59:42]: ▸         "CA_DEBUG_TRANSACTIONS" = 0;
  | [09:59:42]: ▸         "DYLD_FRAMEWORK_PATH" = "/usr/local/var/buildkite-agent/builds/MV-MKE-X64-013/automattic/wordpress-ios/DerivedData/Build/Products/Debug-iphonesimulator:/usr/local/var/buildkite-agent/builds/MV-MKE-X64-013/automattic/wordpress-ios/DerivedData/Build/Products/Debug-iphonesimulator/PackageFrameworks";
  | [09:59:42]: ▸         "DYLD_INSERT_LIBRARIES" = "/Applications/Xcode-14.3.0.app/Contents/Developer/Platforms/iPhoneOS.platform/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot/usr/lib/libMainThreadChecker.dylib";
  | [09:59:42]: ▸         "DYLD_LIBRARY_PATH" = "/usr/local/var/buildkite-agent/builds/MV-MKE-X64-013/automattic/wordpress-ios/DerivedData/Build/Products/Debug-iphonesimulator";
  | [09:59:42]: ▸         NSUnbufferedIO = YES;
  | [09:59:42]: ▸         "OS_ACTIVITY_MODE" = info;
  | [09:59:42]: ▸         "__XCODE_BUILT_PRODUCTS_DIR_PATHS" = "/usr/local/var/buildkite-agent/builds/MV-MKE-X64-013/automattic/wordpress-ios/DerivedData/Build/Products/Debug-iphonesimulator";
  | [09:59:42]: ▸         "__XPC_DYLD_FRAMEWORK_PATH" = "/usr/local/var/buildkite-agent/builds/MV-MKE-X64-013/automattic/wordpress-ios/DerivedData/Build/Products/Debug-iphonesimulator";
  | [09:59:42]: ▸         "__XPC_DYLD_LIBRARY_PATH" = "/usr/local/var/buildkite-agent/builds/MV-MKE-X64-013/automattic/wordpress-ios/DerivedData/Build/Products/Debug-iphonesimulator";
  | [09:59:42]: ▸     };
  | [09:59:42]: ▸     stderr = "/dev/ttys004";
  | [09:59:42]: ▸     stdout = "/dev/ttys004";
  | [09:59:42]: ▸     "terminate_running_process" = 1;
  | [09:59:42]: ▸     "wait_for_debugger" = 0;
  | [09:59:42]: ▸ } (error = Error Domain=FBSOpenApplicationServiceErrorDomain Code=4 "Simulator device returned an error for the requested operation." UserInfo={BSErrorCodeDescription=InvalidRequest, NSUnderlyingError=0x60000146abb0 {Error Domain=FBSOpenApplicationServiceErrorDomain Code=4 "The request to open "com.automattic.jetpack" failed." UserInfo={NSLocalizedDescription=The request to open "com.automattic.jetpack" failed., BSErrorCodeDescription=InvalidRequest, FBSOpenApplicationRequestID=0x9dfb, NSUnderlyingError=0x60000146aeb0 {Error Domain=FBSOpenApplicationErrorDomain Code=4 "Application info provider (FBSApplicationLibrary) returned nil for "com.automattic.jetpack"" UserInfo={BSErrorCodeDescription=NotFound, NSLocalizedFailureReason=Application info provider (FBSApplicationLibrary) returned nil for "com.automattic.jetpack"}}}}, FBSOpenApplicationRequestID=0x9dfb, SimCallingSelector=launchApplicationWithID:options:pid:error:, NSLocalizedDescription=Simulator device returned an error for the requested operation.})

They occur on my end, too, and don't seem related to the UI changes. I say that because it looks like it fails immediately:

mokagio · 2023-05-29T12:03:10Z

@pachlava I cherry picked your work on my PR, and I still got those failed to launch errors. However, the iPhone UI tests passed, which is a good improvement 🙌

I'm quite confused by those logs, by the way. I tried the same commit locally and got some of those failures but also tests that passed:

pachlava · 2023-05-29T13:27:19Z

@mokagio 👋

What do you make of all the failures like:
...
Failed to launch app with identifier: com.automattic.jetpack and options
...

These kinds of failures seem to be present in all cases of UI tests execution, no matter if they pass or fail. They might be related to the fact of parallel testing being enabled (@jostnes / @tiagomar ), but I might be wrong.

Not having them is an ideal case, however it's not the reason why the tests fail.

In my testing PR, the testFreeToPaidCardNavigation also failed for iPad. Unfortunately, the log does not contain screenshots, for some reason. It failed for me locally too, once, because the card was not displayed in the Dashboard. This might be something that was mentioned by @staskus here. However, after rerunning (even after clearing the simulator) both locally and on CI (here), it passed (but another test failed).

pachlava · 2023-05-29T13:41:13Z

These kinds of failures seem to be present in all cases of UI tests execution, no matter if they pass or fail. They might be related to the fact of parallel testing being enabled

Taking my words back, this was also taking place before parallel testing.

mokagio · 2023-05-30T00:45:01Z

@pachlava, thank you for the investigation. Parallel testing would have been my first line of enquiry too 😄

staskus · 2023-05-30T05:27:22Z

@mokagio, @pachlava, thank you for your work in merging and fixing the test failures happening due to the parallel work in #20731! 🙇‍♂️

tiagomar · 2023-06-13T15:36:58Z

@pachlava @mokagio

Taking my words back, this was also taking place before parallel testing.

Yep, these errors started showing up after we started deleting the app after each test. My understanding is that the test runner tries to launch the app while it's still being re-installed. It makes the log noisy and a bit misleading but so far I think the execution time saved and increased reliability with deleting the app kind of pays-off?

staskus added [Type] Enhancement WP.com Plans labels May 22, 2023

staskus requested a review from guarani May 22, 2023 18:52

staskus requested a review from a team as a code owner May 22, 2023 18:52

staskus added this to the 22.5 milestone May 22, 2023

staskus added 4 commits May 23, 2023 13:42

Updating UI tests to test free to paid plans card instead of domains …

d8001ef

…card

Test Domain Selection

77bd11a

- Added Domain suggestion, product fetching, and adding to cart API mocks - Select domain and confirm selection in UI tests

Verify Plan selection is opened after confirming Domain selection

183d1aa

Selecting a cell in a specific table

ae1b70e

staskus force-pushed the task/plans-on-jp-update-ui-tests branch from 5cbbf96 to ae1b70e Compare May 23, 2023 11:17

guarani reviewed May 23, 2023

View reviewed changes

staskus added 2 commits May 24, 2023 12:12

Change expected request for dashboard cards in api mocks

cfc63d4

Reload dashboard when domains are refreshed

e2ae033

staskus requested a review from guarani May 24, 2023 15:11

pachlava approved these changes May 25, 2023

View reviewed changes

guarani approved these changes May 25, 2023

View reviewed changes

Merge remote-tracking branch 'origin/trunk' into task/plans-on-jp-upd…

a3d5b7b

…ate-ui-tests There were conflicts on `WordPress/WordPress.xcodeproj/project.pbxproj` because both the working branch and `trunk` added a file. I resolved the by keeping both files and verifying the build.

mokagio enabled auto-merge May 28, 2023 11:40

mokagio merged commit 69ae650 into trunk May 28, 2023

mokagio deleted the task/plans-on-jp-update-ui-tests branch May 28, 2023 12:09

mokagio mentioned this pull request May 29, 2023

Merge 22.5 code freeze #20750

Merged

5 tasks

pachlava mentioned this pull request May 29, 2023

[UI Test] Removed tabBar init from redesigned Domains Suggestions Screen #20752

Merged

This was referenced Jun 7, 2023

Plans on JP: Update UI test to reach purchase result screen #20805

Closed

Plans on JP: Update UI tests #20720

Closed

tiagomar mentioned this pull request Jun 13, 2023

Cover more user paths where UI tests ask to save password #20751

Merged

jostnes mentioned this pull request Jun 16, 2023

[UI Tests] - Add logout at launch capability #20891

Merged

	public func selectDomain() throws -> Self {
	public func tapLastDomain() throws -> Self {

Plans on JP: Update UI tests for Plan Selection #20728

Plans on JP: Update UI tests for Plan Selection #20728

Uh oh!

Conversation

staskus commented May 22, 2023

Uh oh!

peril-wordpress-mobile bot commented May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

guarani left a comment

Choose a reason for hiding this comment

Uh oh!

staskus commented May 24, 2023

Uh oh!

staskus commented May 24, 2023

Uh oh!

staskus commented May 24, 2023

Uh oh!

pachlava left a comment

Choose a reason for hiding this comment

Uh oh!

pachlava May 25, 2023

Choose a reason for hiding this comment

Uh oh!

pachlava May 25, 2023

Choose a reason for hiding this comment

Uh oh!

staskus May 30, 2023

Choose a reason for hiding this comment

Uh oh!

guarani left a comment

Choose a reason for hiding this comment

Uh oh!

mokagio commented May 28, 2023

Uh oh!

mokagio commented May 29, 2023

Uh oh!

mokagio commented May 29, 2023

Uh oh!

pachlava commented May 29, 2023

Uh oh!

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

Uh oh!

Uh oh!

mokagio commented May 29, 2023

Uh oh!

mokagio commented May 29, 2023

Uh oh!

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

Uh oh!

Uh oh!

pachlava commented May 29, 2023

Uh oh!

mokagio commented May 30, 2023

Uh oh!

staskus commented May 30, 2023

Uh oh!

tiagomar commented Jun 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

peril-wordpress-mobile bot commented May 22, 2023 •

edited

Loading

wpmobilebot commented May 22, 2023 •

edited

Loading

wpmobilebot commented May 22, 2023 •

edited

Loading

pachlava commented May 29, 2023 •

edited

Loading

pachlava commented May 29, 2023 •

edited

Loading