Convert UI tests screens to be a ScreenObject subclasses – 6 of many

[mokagio]

See #17221, #17348, #17359, #17360, and #17361.

This might actually be the last one 😅 . I forgot about a couple more screens.

If the UI tests pass on both CircleCI and Buildkite, we should be good to merge.

Regression Notes

1. Potential unintended areas of impact
   N.A.

2. What I did to test those areas of impact (or what existing automated tests I relied on)
   N.A.

3. What automated tests I added (or what prevented me from doing so)
   N.A.

---

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

[peril-wordpress-mobile]

You can trigger an installable build for these changes by visiting CircleCI here.

[peril-wordpress-mobile]

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

[mokagio]

A few of these were unused, but the majority has been replaced either by a one off call to app.<query> or by a getter closure and computed variable pair.

The reason I moved from lets (= instantiated once, at init time) to computed var or runtime one-off queries is to make sure we query at the most appropriate time.

This shift also has the added benefit of moving all these single usages in the same function that uses them, meaning one can look at a single function (a few lines of code) and get all of the information on what that code requires and does.

[jostnes]

I recently did the opposite of this on the WooCommerce iOS repo where I moved them all to lets 😅 I thought having it grouped together would be easier to manage and see and also in case of reusability (though I have not work on the tests long enough to really know how much of the identifiers are being reused at this point)

I don't have strong opinions on either style, so I'm flexible but I am looking to see if we can get more consistency for the UI tests across apps so that it's easier to move between apps when writing UI tests.

[mokagio]

I don't have strong opinions on either style, so I'm flexible but I am looking to see if we can get more consistency for the UI tests across apps so that it's easier to move between apps when writing UI tests.

This is gold. Thank you for sharing your own rationale and for asking the question.

I settled for that approach for the reason stated above and also to keep the code more compact.

Regarding let vs runtime evaluation: My mental representation of how XCTest behaves is that it runs the query to fetch the element when it's defined. I developed this when bumping into some test failures that I resolved by moving to that style. I haven't built a dedicated project to verify this assumption, but I ought to.

If my assumption is incorrect, then my biggest argument in favor of not using lets would no longer be valid. 🤔

Your point about having them "grouped together would be easier to manage and see and also in case of reusability" is totally valid. I personally prefer short, local code, and might have been a bit idealistic in imagining all our ScreenObject subclass will remain so short that it would be trivial to notice duplication. There's a lot of value in optimizing code to make it easier to touch in the future, so maybe I've been over-optimizing on the wrong dimension.

I guess the next step would be to verify that assumption on how the XCUIElements are evaluated. With an answer to that, we can then decide what's best for us going forward.

[mokagio]

We don't see this syntax much: It's a closure that returns another closure.

You can parse it like this

let textViewGetter: (Input) -> (Output)

let textViewGetter: (String) -> (Output)

let textViewGetter: (String) -> (XCUIApplication) -> XCUIElement

That's why we have

{ identifier in
  return app in 

{ identifier in return ... } is the (String) -> (Output), { app in ... } is the (Output) value, i.e. XCUIApplication -> XCUIElement.

---

I wrote this to generate the getter closure based on a textField value that becomes known only at runtime, as you can see in the init code just below.

[mokagio]

Got a bunch of UI tests failures, clearly there some work left to do here.

Update: Solved on the main branch and then rebased here. Ready for review.

[jostnes]

Tested the tests locally on iPhone 11 Sim iOS 15.0 and all are passing: Executed 12 tests, with 0 failures (0 unexpected)

Thanks for making the changes and for adding explanations on why the changes were done! I find those explanations very helpful.

[jostnes]

I recently did the opposite of this on the WooCommerce iOS repo where I moved them all to lets 😅 I thought having it grouped together would be easier to manage and see and also in case of reusability (though I have not work on the tests long enough to really know how much of the identifiers are being reused at this point)

I don't have strong opinions on either style, so I'm flexible but I am looking to see if we can get more consistency for the UI tests across apps so that it's easier to move between apps when writing UI tests.

[pachlava]

I decided to wait for the CI to rerun the checks after the latest commit before approving the PR, but I missed the chance 😅

Still, I wanted to say thanks for the extra details about the code - it was something new to me!

All the tests passed locally on iPhone 13, as well as CI was green.

[mokagio]

My pleasure @pachlava 😄 Thank you for always double checking my work. It's most helpful and reassuring.