Skip to content

Convert some of the UI tests screen objects from BaseScreen to ScreenObject – Part 1 of many#17221

Merged
mokagio merged 12 commits intodevelopfrom
convert-to-screen-objects
Oct 19, 2021
Merged

Convert some of the UI tests screen objects from BaseScreen to ScreenObject – Part 1 of many#17221
mokagio merged 12 commits intodevelopfrom
convert-to-screen-objects

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Sep 27, 2021

This is the first PR of a series that will convert all BaseScreen subclasses into ScreenObject subclasses.

It might not be super clear by looking at the diff, so here's the structure of a simple ScreenObject:

import ScreenObject // ScreenObject comes from a Swift package, so we need to import it
import XCTest

public class FeaturedImageScreen: ScreenObject {

    // expectedElement comes from the superclass and gets the first expectedElementGetters result
    var removeButton: XCUIElement { expectedElement }

    // The subclass (this one, FeaturedImageScreen) wraps the superclass (ScreenObject)
	// init method so that we can call it with no parameters. All the defaults are set in
    // the context of this file and callers don't need to know about them.
    public init(app: XCUIApplication = XCUIApplication()) throws {
        try super.init(
			// In this simple screen, we only check for one element to verify wether the
            // screen is loaded.
            //
  			// This here is a closure and its type, as defined in the ScreenObject init
			// method signature is (XCUIApplication) -> XCUIElement. That means it
			// takes a XCUIApplication as input and returns an XCUIElement. The $0 is a
            // shorthand to represent the first (0 index) argument passed to the closure.
            //
            // Internally, ScreenObject uses the app instance we pass to in in the next line
            // to call this closure and get the expected element.
            expectedElementGetters: [ { $0.navigationBars.buttons["Remove Featured Image"] } ],
            app: app
        )
    }

	// ScreenObject subclasses in UITestFoundation can expose many methods to interact with
    // the screen they represent.
    public func tapRemoveFeaturedImageButton() {
        removeButton.tap()
        // app is the same value passed at init
        app.sheets.buttons.element(boundBy: 0).tap()
    }
}

Most screens use more than one element to verify if they're loaded correctly and for the test interactions. In those cases, we use a two step approach.

First, we define the getter closure as an instance let

let getDoneButton: (XCUIApplication) -> XCUIElement = {
    $0.navigationBars.buttons["doneButton"]
}

Then we pass the value in the expectedElementGetters array

init(app: XCUIApplication = XCUIApplication()) throws {
    try super.init(
        expectedElementGetters: [ getDoneButton, getViewButton, ... ],
        app: app
    )
}

Finally, we access the element themselves by calling the closure. We don't use expectedElement because that only returns the first element.

var doneButton: XCUIElement { getDoneButton(app) }
var viewButton: XCUIElement { getViewButton(app) }

That's it. I wish I could have found a cleaner way to do this, or one that required less explanation and relied more on the type system. This is the best I could do so far. 😕


Notice that I run the UI tests and the all passed.

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 Sep 27, 2021

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

@mokagio mokagio changed the title Convert some of the UI tests screen objects from BaseScreen to ScreenObject Convert some of the UI tests screen objects from BaseScreen to ScreenObject – Part 1 of many Sep 27, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Sep 27, 2021

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

@mokagio mokagio force-pushed the convert-to-screen-objects branch from d6c926e to 620870d Compare September 28, 2021 05:12
"revision": "df8ee4983e674b583b546bb0640ec83428be2465",
"version": "0.1.0"
"revision": "2d65beae47cdcaaf21703ce1b321f382fe0d0730",
"version": "0.2.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated ScreenObject to the latest version to take advantage of the multiple-elements init method.

Comment on lines +100 to +111
extension ScreenObject {

@discardableResult
func thenTakeScreenshot(_ index: Int, named title: String) -> Self {
let mode = XCUIDevice.inDarkMode ? "dark" : "light"
let filename = "\(index)-\(mode)-\(title)"

snapshot(filename)

return self
}
}
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 a 1:1 copy of the extension on BaseScreen above and is a way to combine Fastlane's screenshot logic with a nice DSL in ScreenObject.

Eventually, we might want to add some kind of abstraction and make this part of the framework. For the moment, though, the focus is to convert all the screens, so this will do.

public init() {
super.init(element: XCUIApplication().otherElements.firstMatch)
public init(app: XCUIApplication = XCUIApplication()) throws {
try super.init(expectedElementGetters: [ { $0.otherElements.firstMatch } ])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Worth pointing out that this matches is pretty useless because it will succeed most of the time.

The aim of the current work is not to fix "issues" in the tests, but to move to ScreenObject to make it easier to fix those issues later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opened #17231 in this regard.

Copy link
Contributor

Choose a reason for hiding this comment

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

Worth pointing out that this matches is pretty useless because it will succeed most of the time.

Indeed 😄

Comment on lines 80 to 81

// returns void since return screen depends on which editor you're in
/// - Note: Returns `Void` because the return screen depends on which editor the user is in.
public func closePostSettings() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want the UITestsFoundation framework to be super easy to use for feature developers to write tests with. This little change in how the documentation is formatted means Xcode will show this information during autocompletion or if a user Alt-clicks on the method name.

Screen Shot 2021-09-28 at 3 19 06 pm

return XCUIApplication().tables["SettingsTable"].exists
return (try? EditorPostSettings().isLoaded) ?? false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rather than duplicating the logic to verify if a certain element exists, generating code that risks getting out of date, we can leverage the ScreenObject init behavior, which oden't fail the tests automatically if the screen is not loaded.

public class FeaturedImageScreen: ScreenObject {

let removeButton: XCUIElement
var removeButton: XCUIElement { expectedElement }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: add a code comment about this behavior here

@mokagio mokagio force-pushed the convert-to-screen-objects branch 4 times, most recently from 65451a1 to e8c38d7 Compare September 30, 2021 05:55
}

func testGenerateScreenshots() {
func testGenerateScreenshots() throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the ScreenObject init throws, we now have a sprinkle of try calls around the test suite.

That's good. XCTest knows how to fail tests and report errors if a try call throws.

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.

so here's the structure of a simple ScreenObject:

I wish I started the review from this PR, instead of woocommerce/woocommerce-ios/pull/5091 (which I did because it had 6 changed files less 😅 ). The overview that you provide in the PR description is very helpful! What do you think about making it a part of README.md from ScreenObject?

I executed the test locally, with no fails 👍

@mokagio
Copy link
Contributor Author

mokagio commented Oct 19, 2021

What do you think about making it a part of README.md from ScreenObject?

That's a great idea, thanks! I don't have time to tackle it just yet and I'd like the code to be used by a few more people to discover rough edges first. I opened Automattic/ScreenObject#10 to remember this.

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.

2 participants