Skip to content

Convert UI tests screens to be ScreenObject subclasses – Part 3 of many#17359

Merged
mokagio merged 9 commits intodevelopfrom
convert-to-screen-objects-p3
Nov 4, 2021
Merged

Convert UI tests screens to be ScreenObject subclasses – Part 3 of many#17359
mokagio merged 9 commits intodevelopfrom
convert-to-screen-objects-p3

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Oct 25, 2021

See #17221 and #17348.

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 Oct 25, 2021

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

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Oct 25, 2021

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

@mokagio mokagio marked this pull request as ready for review October 25, 2021 05:56
Comment on lines +17 to +38
public func openMagicLink() {
XCTContext.runActivity(named: "Open magic link in Safari") { activity in
let safari = XCUIApplication(bundleIdentifier: "com.apple.mobilesafari")
safari.launch()

// Select the URL bar when Safari opens
let urlBar = safari.textFields["URL"]
if !urlBar.waitForExistence(timeout: 5) {
safari.buttons["URL"].tap()
}

// Follow the magic link
var magicLinkComponents = URLComponents(url: WireMock.URL(), resolvingAgainstBaseURL: false)!
magicLinkComponents.path = "/magic-link"
magicLinkComponents.queryItems = [URLQueryItem(name: "scheme", value: "wpdebug")]

urlBar.typeText("\(magicLinkComponents.url!.absoluteString)\n")

// Accept the prompt to open the deep link
safari.scrollViews.element(boundBy: 0).buttons.element(boundBy: 1).tap()
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is duplicated from the BaseScreen version. Once we'll have no more BaseScreen in the UI tests, it will become the only one in the codebase. It's also something we might want to move to the authentication framework.

Comment on lines -11 to +12
mySiteScreen = TabNavComponent()
.gotoMySiteScreen()
mySiteScreen = try TabNavComponent()
.goToMySiteScreen()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just an indentation fix


// Get Stats screenshot
let statsScreen = mySite.gotoStatsScreen()
let statsScreen = try mySite.goToStatsScreen()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There'll be a bunch more of this changes because of the goto to goTo rename. Sorry for the noise.

@mokagio mokagio requested a review from pachlava October 25, 2021 10:25
@mokagio mokagio added this to the 18.6 milestone Oct 25, 2021
@mokagio mokagio added the Testing Unit and UI Tests and Tooling label Oct 25, 2021
@mokagio mokagio enabled auto-merge October 26, 2021 01:25
@mokagio mokagio changed the title Convert UI tests screens to be a ScreenObject subclasses – Part 3 of many Convert UI tests screens to be ScreenObject subclasses – Part 3 of many Oct 26, 2021
@oguzkocer oguzkocer modified the milestones: 18.6, 18.7 Nov 1, 2021
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 executed the UI tests and screenshot generation locally, and both passed. Thanks for doing the code clean-up besides the 'main' task 👍

Comment on lines 35 to 37
static func isLoaded() -> Bool {
return XCUIApplication().buttons[ElementStringIDs.loginButton].exists
(try? WelcomeScreen().isLoaded) ?? false
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is the same in previous PRs, but for some reason it caught my eye only now.

To be honest, I spent some time not understanding the reason for having an isLoaded() method for some of the screens, when there's an isLoaded variable in the ScreenObject. After a bit of processing, I realized that this is a shorter way to calculate that property in the runtime, without the need to handle potential error every time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct.

To be fair, the static approach made sense only in the BaseScreen context, where a calling init on a screen not displayed would fail the test. With ScreenObject, it's less useful. I'd like to remove as many of these as possible.

For example, the following doesn't make much sense anymore:

private func confirmPublish() throws {
if FancyAlertComponent.isLoaded() {
try FancyAlertComponent().acceptAlert()
} else {
publishNowButton.tap()
}
}

We could replace it with something like:

guard let alert = FancyAlertComponent() else {
    publishNowButton.tap() 
    return
}

alert.acceptAlert()

On the other hand, this does. Since we don't access any of those screen directly, it's handy to call a method on the type:

while EditorPostSettings.isLoaded() || CategoriesComponent.isLoaded() || TagsComponent.isLoaded() || MediaPickerAlbumListScreen.isLoaded() || MediaPickerAlbumScreen.isLoaded() {
navBackButton.tap()
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for explaining it, Gio!

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