Skip to content

Convert some UI tests screens to be ScreenObject subclasses – Part 7 of 7#17641

Merged
mokagio merged 3 commits intodevelopfrom
convert-to-screen-objects-wip-2
Dec 9, 2021
Merged

Convert some UI tests screens to be ScreenObject subclasses – Part 7 of 7#17641
mokagio merged 3 commits intodevelopfrom
convert-to-screen-objects-wip-2

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Dec 8, 2021

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

This is the last PR in the series. To celebrate, I included the commit that removes BaseScreen 🔥 .

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
Copy link

peril-wordpress-mobile bot commented Dec 8, 2021

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

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Dec 8, 2021

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

@mokagio mokagio force-pushed the convert-to-screen-objects-wip-2 branch from ec687f8 to b5ccadd Compare December 8, 2021 13:21
@mokagio mokagio marked this pull request as ready for review December 8, 2021 13:22
@mokagio mokagio marked this pull request as draft December 8, 2021 13:23
if var expectedUsername = username {
expectedUsername = "@\(expectedUsername)"
let actualUsername = usernameField.label
let actualUsername = app.staticTexts["login-epilogue-username-label"].label
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jostnes raised a valid question on whether to adopt this inline style vs moving everything at the start of the file.

I stuck to the approach I used so far because: 1) we haven't taken a decision yet; 2) it's what (most of) the rest of the ScreenObject subclasses here use.

Comment on lines -65 to +62
try XCTContext.runActivity(named: "Dismiss quick start prompt if needed.") { (activity) in
if QuickStartPromptScreen.isLoaded() {
Logger.log(message: "Dismising quick start prompt...", event: .i)
_ = try QuickStartPromptScreen().selectNoThanks()
return
}
try XCTContext.runActivity(named: "Dismiss quick start prompt if needed.") { _ in
guard QuickStartPromptScreen.isLoaded() else { return }

Logger.log(message: "Dismising quick start prompt...", event: .i)
_ = try QuickStartPromptScreen().selectNoThanks()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced a surrounding if with a guard let to remove one level of nesting.

@mokagio mokagio marked this pull request as ready for review December 8, 2021 16:07
@mokagio mokagio enabled auto-merge December 8, 2021 16:08
@mokagio mokagio added this to the 18.9 milestone Dec 8, 2021
@mokagio mokagio added the Testing Unit and UI Tests and Tooling label Dec 8, 2021
@jostnes
Copy link
Contributor

jostnes commented Dec 9, 2021

I tried running it locally and the test testTextPostPublish is failing for me locally (for some reason it's stuck after adding site address), started the run on CircleCI to see if it's an issue on my local only or not.

Copy link
Contributor

@pachlava pachlava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that CI is green, and locally I had issues only with testViewPostInSafari, which is not related to the PR, and the fail was caused by the cookies accept prompt shown in the WebView in the case of Simulator state reset:

Screenshot 2021-12-09 at 13 35 11

Once I accepted cookies, further execution of the same test went well. Anyway, this is not a part of this PR.

All the changes in the code make sense, and I saw no issues 👍 🥳 🎉

cc @tiagomar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Testing Unit and UI Tests and Tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants