Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mokagio merged 8 commits into from
Dec 7, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
686b6e6
Convert `AztecEditorScreen` to be a `ScreenObject` subclass
mokagio Oct 25, 2021
cd2654a
Convert `PasswordScreen` to be a `ScreenObject` subclass
mokagio Oct 25, 2021
01ec106
Convert `WelcomeScreen` to be a `ScreenObject` subclass
mokagio Oct 25, 2021
c0ec006
Convert `LoginPasswordScreen` to be a `ScreenObject` subclass
mokagio Oct 25, 2021
8206938
Convert `LinkOrPasswordScreen` to be a `ScreenObject` subclass
mokagio Oct 25, 2021
b0724fc
Convert `LoginCheckMagicLinkScreen` to be a `ScreenObject` subclass
mokagio Oct 25, 2021
8a7f206
Convert `LoginSiteAddressScreen` to be a `ScreenObject` subclass
mokagio Oct 25, 2021
e284434
Remove a double comment start syntax
mokagio Dec 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 63 additions & 75 deletions WordPress/UITestsFoundation/Screens/Editor/AztecEditorScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import ScreenObject
import XCTest

public class AztecEditorScreen: BaseScreen {
public class AztecEditorScreen: ScreenObject {

enum Mode {
case rich
case html
Expand All @@ -11,63 +13,44 @@ public class AztecEditorScreen: BaseScreen {
}

let mode: Mode
var textView: XCUIElement

private var richTextField = "aztec-rich-text-view"
private var htmlTextField = "aztec-html-text-view"

let editorCloseButton = XCUIApplication().navigationBars["Azctec Editor Navigation Bar"].buttons["Close"]
let publishButton = XCUIApplication().buttons["Publish"]
let publishNowButton = XCUIApplication().buttons["Publish Now"]
let moreButton = XCUIApplication().buttons["more_post_options"]
let uploadProgressBar = XCUIApplication().progressIndicators["Progress"]

let titleView = XCUIApplication().textViews["Title"]
let contentPlaceholder = XCUIApplication().staticTexts["aztec-content-placeholder"]

let mediaButton = XCUIApplication().buttons["format_toolbar_insert_media"]
let insertMediaButton = XCUIApplication().buttons["insert_media_button"]
let headerButton = XCUIApplication().buttons["format_toolbar_select_paragraph_style"]
let boldButton = XCUIApplication().buttons["format_toolbar_toggle_bold"]
let italicButton = XCUIApplication().buttons["format_toolbar_toggle_italic"]
let underlineButton = XCUIApplication().buttons["format_toolbar_toggle_underline"]
let strikethroughButton = XCUIApplication().buttons["format_toolbar_toggle_strikethrough"]
let blockquoteButton = XCUIApplication().buttons["format_toolbar_toggle_blockquote"]
let listButton = XCUIApplication().buttons["format_toolbar_toggle_list_unordered"]
let linkButton = XCUIApplication().buttons["format_toolbar_insert_link"]
let horizontalrulerButton = XCUIApplication().buttons["format_toolbar_insert_horizontal_ruler"]
let sourcecodeButton = XCUIApplication().buttons["format_toolbar_toggle_html_view"]
let moreToolbarButton = XCUIApplication().buttons["format_toolbar_insert_more"]

let unorderedListOption = XCUIApplication().buttons["Unordered List"]
let orderedListOption = XCUIApplication().buttons["Ordered List"]

// Action sheets
let actionSheet = XCUIApplication().sheets.element(boundBy: 0)
let postSettingsButton = XCUIApplication().sheets.buttons["Post Settings"]
let keepEditingButton = XCUIApplication().sheets.buttons["Keep Editing"]
let postHasChangesSheet = XCUIApplication().sheets["post-has-changes-alert"]
Comment on lines -19 to -49
Copy link
Contributor Author

@mokagio mokagio Nov 30, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@jostnes jostnes Dec 7, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@mokagio mokagio Dec 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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.


init(mode: Mode) {
var textField = ""
private(set) var textView: XCUIElement

private let richTextField = "aztec-rich-text-view"
private let htmlTextField = "aztec-html-text-view"

var mediaButton: XCUIElement { app.buttons["format_toolbar_insert_media"] }
var sourcecodeButton: XCUIElement { app.buttons["format_toolbar_toggle_html_view"] }

private let textViewGetter: (String) -> (XCUIApplication) -> XCUIElement = { identifier in
return { app in
Comment on lines +24 to +25
Copy link
Contributor Author

@mokagio mokagio Nov 30, 2021

Choose a reason for hiding this comment

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

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.

var textView = app.textViews[identifier]

if textView.exists == false {
if app.otherElements[identifier].exists {
textView = app.otherElements[identifier]
}
}

return textView
}
}

init(mode: Mode, app: XCUIApplication = XCUIApplication()) throws {
self.mode = mode
let textField: String
switch mode {
case .rich:
textField = richTextField
case .html:
textField = htmlTextField
}

let app = XCUIApplication()
textView = app.textViews[textField]

if !textView.exists {
if app.otherElements[textField].exists {
textView = app.otherElements[textField]
}
}

super.init(element: textView)
try super.init(
expectedElementGetters: [ textViewGetter(textField) ],
app: app
)

showOptionsStrip()
}
Expand All @@ -87,11 +70,12 @@ public class AztecEditorScreen: BaseScreen {

@discardableResult
func addList(type: String) -> AztecEditorScreen {
let listButton = app.buttons["format_toolbar_toggle_list_unordered"]
tapToolbarButton(button: listButton)
if type == "ul" {
unorderedListOption.tap()
app.buttons["Unordered List"].tap()
} else if type == "ol" {
orderedListOption.tap()
app.buttons["Ordered List"].tap()
}

return self
Expand All @@ -114,6 +98,7 @@ public class AztecEditorScreen: BaseScreen {
*/
@discardableResult
func tapToolbarButton(button: XCUIElement) -> AztecEditorScreen {
let linkButton = app.buttons["format_toolbar_insert_link"]
let swipeElement = mediaButton.isHittable ? mediaButton : linkButton

if !button.exists || !button.isHittable {
Expand Down Expand Up @@ -147,22 +132,12 @@ public class AztecEditorScreen: BaseScreen {
return self
}

/**
Switches between Rich and HTML view.
*/
func switchContentView() -> AztecEditorScreen {
tapToolbarButton(button: sourcecodeButton)


return AztecEditorScreen(mode: mode.toggle())
}

/**
Common method to type in different text fields
*/
@discardableResult
func enterText(text: String) -> AztecEditorScreen {
contentPlaceholder.tap()
public func enterText(text: String) -> AztecEditorScreen {
app.staticTexts["aztec-content-placeholder"].tap()
textView.typeText(text)
return self
}
Expand All @@ -171,7 +146,8 @@ public class AztecEditorScreen: BaseScreen {
Enters text into title field.
- Parameter text: the test to enter into the title
*/
func enterTextInTitle(text: String) -> AztecEditorScreen {
public func enterTextInTitle(text: String) -> AztecEditorScreen {
let titleView = app.textViews["Title"]
titleView.tap()
titleView.typeText(text)

Expand Down Expand Up @@ -222,10 +198,14 @@ public class AztecEditorScreen: BaseScreen {

// Inject the first picture
try MediaPickerAlbumScreen().selectImage(atIndex: 0)
insertMediaButton.tap()
app.buttons["insert_media_button"].tap()

// Wait for upload to finish
waitFor(element: uploadProgressBar, predicate: "exists == false", timeout: 10)
let uploadProgressBar = app.progressIndicators["Progress"]
XCTAssertEqual(
uploadProgressBar.waitFor(predicateString: "exists == false", timeout: 10),
.completed
)

return self
}
Expand All @@ -234,19 +214,23 @@ public class AztecEditorScreen: BaseScreen {
public func closeEditor() {
XCTContext.runActivity(named: "Close the Aztec editor") { (activity) in
XCTContext.runActivity(named: "Close the More menu if needed") { (activity) in
let actionSheet = app.sheets.element(boundBy: 0)
if actionSheet.exists {
if XCUIDevice.isPad {
app.otherElements["PopoverDismissRegion"].tap()
} else {
keepEditingButton.tap()
app.sheets.buttons["Keep Editing"].tap()
}
}
}

let editorCloseButton = app.navigationBars["Azctec Editor Navigation Bar"].buttons["Close"]

editorCloseButton.tap()

XCTContext.runActivity(named: "Discard any local changes") { (activity) in

let postHasChangesSheet = app.sheets["post-has-changes-alert"]
let discardButton = XCUIDevice.isPad ? postHasChangesSheet.buttons.lastMatch : postHasChangesSheet.buttons.element(boundBy: 1)

if postHasChangesSheet.exists && (discardButton?.exists ?? false) {
Expand All @@ -255,13 +239,16 @@ public class AztecEditorScreen: BaseScreen {
}
}

let editorClosed = waitFor(element: editorCloseButton, predicate: "isEnabled == false")
XCTAssert(editorClosed, "Aztec editor should be closed but is still loaded.")
XCTAssertEqual(
editorCloseButton.waitFor(predicateString: "isEnabled == false"),
.completed,
"Aztec editor should be closed but is still loaded."
)
}
}

func publish() throws -> EditorNoticeComponent {
publishButton.tap()
public func publish() throws -> EditorNoticeComponent {
app.buttons["Publish"].tap()

try confirmPublish()

Expand All @@ -272,13 +259,14 @@ public class AztecEditorScreen: BaseScreen {
if FancyAlertComponent.isLoaded() {
try FancyAlertComponent().acceptAlert()
} else {
publishNowButton.tap()
app.buttons["Publish Now"].tap()
}
}

public func openPostSettings() throws -> EditorPostSettings {
moreButton.tap()
postSettingsButton.tap()
app.buttons["more_post_options"].tap()

app.sheets.buttons["Post Settings"].tap()

return try EditorPostSettings()
}
Expand All @@ -298,7 +286,7 @@ public class AztecEditorScreen: BaseScreen {
return textView.value as! String
}

static func isLoaded() -> Bool {
return XCUIApplication().navigationBars["Azctec Editor Navigation Bar"].buttons["Close"].exists
static func isLoaded(mode: Mode = .rich) -> Bool {
(try? AztecEditorScreen(mode: mode).isLoaded) ?? false
}
}
36 changes: 17 additions & 19 deletions WordPress/UITestsFoundation/Screens/Login/LinkOrPasswordScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,34 @@
import ScreenObject
import XCTest

// TODO: remove when unifiedAuth is permanent.

private struct ElementStringIDs {
static let passwordOption = "Use Password"
static let linkButton = "Send Link Button"
}

public class LinkOrPasswordScreen: BaseScreen {
let passwordOption: XCUIElement
let linkButton: XCUIElement
public class LinkOrPasswordScreen: ScreenObject {

init() {
passwordOption = XCUIApplication().buttons[ElementStringIDs.passwordOption]
linkButton = XCUIApplication().buttons[ElementStringIDs.linkButton]
let passwordOptionGetter: (XCUIApplication) -> XCUIElement = {
$0.buttons["Use Password"]
}
let linkButtonGetter: (XCUIApplication) -> XCUIElement = {
$0.buttons["Send Link Button"]
}

super.init(element: passwordOption)
init(app: XCUIApplication = XCUIApplication()) throws {
try super.init(expectedElementGetters: [passwordOptionGetter, linkButtonGetter], app: app)
}

func proceedWithPassword() -> LoginPasswordScreen {
passwordOption.tap()
func proceedWithPassword() throws -> LoginPasswordScreen {
passwordOptionGetter(app).tap()

return LoginPasswordScreen()
return try LoginPasswordScreen()
}

public func proceedWithLink() -> LoginCheckMagicLinkScreen {
linkButton.tap()
public func proceedWithLink() throws -> LoginCheckMagicLinkScreen {
linkButtonGetter(app).tap()

return LoginCheckMagicLinkScreen()
return try LoginCheckMagicLinkScreen()
}

public static func isLoaded() -> Bool {
return XCUIApplication().buttons[ElementStringIDs.passwordOption].exists
(try? LinkOrPasswordScreen().isLoaded) ?? false
}
}
35 changes: 18 additions & 17 deletions WordPress/UITestsFoundation/Screens/Login/LoginCheckMagicLinkScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,27 @@
import ScreenObject
import XCTest

private struct ElementStringIDs {
static let passwordOption = "Use Password"
static let mailButton = "Open Mail Button"
}

public class LoginCheckMagicLinkScreen: BaseScreen {
let passwordOption: XCUIElement
let mailButton: XCUIElement
public class LoginCheckMagicLinkScreen: ScreenObject {

init() {
let app = XCUIApplication()
passwordOption = app.buttons[ElementStringIDs.passwordOption]
mailButton = app.buttons[ElementStringIDs.mailButton]
let passwordOptionGetter: (XCUIApplication) -> XCUIElement = {
$0.buttons["Use Password"]
}

super.init(element: mailButton)
init(app: XCUIApplication = XCUIApplication()) throws {
try super.init(
expectedElementGetters: [
passwordOptionGetter,
// swiftlint:disable:next opening_brace
{ $0.buttons["Open Mail Button"] }
],
app: app
)
}

func proceedWithPassword() -> LoginPasswordScreen {
passwordOption.tap()
func proceedWithPassword() throws -> LoginPasswordScreen {
passwordOptionGetter(app).tap()

return LoginPasswordScreen()
return try LoginPasswordScreen()
}

public func openMagicLoginLink() -> LoginEpilogueScreen {
Expand All @@ -30,6 +31,6 @@ public class LoginCheckMagicLinkScreen: BaseScreen {
}

public static func isLoaded() -> Bool {
return XCUIApplication().buttons[ElementStringIDs.mailButton].exists
(try? LoginCheckMagicLinkScreen().isLoaded) ?? false
}
}
8 changes: 4 additions & 4 deletions WordPress/UITestsFoundation/Screens/Login/LoginEmailScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,18 @@ public class LoginEmailScreen: ScreenObject {
)
}

public func proceedWith(email: String) -> LinkOrPasswordScreen {
public func proceedWith(email: String) throws -> LinkOrPasswordScreen {
emailTextField.tap()
emailTextField.typeText(email)
nextButton.tap()

return LinkOrPasswordScreen()
return try LinkOrPasswordScreen()
}

func goToSiteAddressLogin() -> LoginSiteAddressScreen {
func goToSiteAddressLogin() throws -> LoginSiteAddressScreen {
app.buttons["Self Hosted Login Button"].tap()

return LoginSiteAddressScreen()
return try LoginSiteAddressScreen()
}

static func isLoaded() -> Bool {
Expand Down
Loading