Skip to content

Convert UI tests screens to be a ScreenObject subclasses – 5 of many#17361

Merged
mokagio merged 7 commits intodevelopfrom
convert-to-screen-objects-p5
Nov 16, 2021
Merged

Convert UI tests screens to be a ScreenObject subclasses – 5 of many#17361
mokagio merged 7 commits intodevelopfrom
convert-to-screen-objects-p5

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Oct 25, 2021

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

I made this on top of #17360 for ease of review.

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.

@mokagio mokagio added the Testing Unit and UI Tests and Tooling label Oct 25, 2021
@mokagio mokagio added this to the 18.6 milestone Oct 25, 2021
@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.

@oguzkocer oguzkocer modified the milestones: 18.6, 18.7 Nov 1, 2021
@mokagio mokagio force-pushed the convert-to-screen-objects-p4 branch from a7206d0 to 2608300 Compare November 11, 2021 04:38
@mokagio mokagio force-pushed the convert-to-screen-objects-p5 branch from f50f89d to 9c31649 Compare November 11, 2021 05:29
expectedElementGetters: [defaultAlertButtonGetter, cancelAlertButtonGetter],
app: app
app: app,
waitTimeout: 3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default timeout for ScreenObject is 20s (which might not be a good idea)... This view is one we check for often: we don't want to wait 20s for it every time.

Comment on lines +173 to 189
// TODO: Move this to XCUITestHelpers or ScreenObject
extension XCUIElement {

func waitFor(
predicateString: String,
timeout: TimeInterval = 10
) -> XCTWaiter.Result {
XCTWaiter.wait(
for: [
XCTNSPredicateExpectation(
predicate: NSPredicate(format: predicateString),
object: self
)
],
timeout: timeout
)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extracting this is something I'll do in a followup PR (which might occur later on, or the next time I'll touch this file)

Comment on lines -46 to +67
removeSiteButton = app.cells[ElementStringIDs.removeSiteButton]
removeSiteSheet = app.sheets.buttons.element(boundBy: 0)
removeSiteAlert = app.alerts.buttons.element(boundBy: 1)
activityLogButton = app.cells[ElementStringIDs.activityLogButton]
jetpackScanButton = app.cells[ElementStringIDs.jetpackScanButton]
jetpackBackupButton = app.cells[ElementStringIDs.jetpackBackupButton]
postsButton = app.cells[ElementStringIDs.postsButton]
mediaButton = app.cells[ElementStringIDs.mediaButton]
statsButton = app.cells[ElementStringIDs.statsButton]
siteSettingsButton = app.cells[ElementStringIDs.settingsButton]
createButton = app.buttons[ElementStringIDs.createButton]
readerButton = app.buttons[ElementStringIDs.ReaderButton]
switchSiteButton = app.buttons[ElementStringIDs.switchSiteButton]
navBar = app.navigationBars[ElementStringIDs.navBarTitle]

super.init(element: navBar)
try super.init(
expectedElementGetters: [
switchSiteButtonGetter,
statsButtonGetter,
postsButtonGetter,
mediaButtonGetter,
createButtonGetter
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only kept a subset of the elements that were instantiated in the previous version to avoid spending too much time verifying whether the screen is loaded (reminder: this is something ScreenObject does automatically at init time).

Comment on lines +19 to +21
// The block editor has _many_ elements but most are loaded on-demand. To verify the screen
// is loaded, we rely only on the button to add a new block and on the navigation bar we
// expect to encase the screen.
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 also the reason for all the red lines above. Instead of instantiating the elements at init time, I've updated the code to use ScreenObject's app property and query it at runtime when a method that needs an element is called.

Also, a few of the elements were unused 🧹 🧹 🧹

@mokagio mokagio requested a review from pachlava November 11, 2021 05:36
@mokagio mokagio modified the milestones: 18.7, 18.8 Nov 11, 2021
@mokagio mokagio force-pushed the convert-to-screen-objects-p4 branch from 2608300 to a3a3c03 Compare November 15, 2021 04:42
@mokagio mokagio force-pushed the convert-to-screen-objects-p5 branch from 9c31649 to 1c4fb5c Compare November 15, 2021 04:42
Base automatically changed from convert-to-screen-objects-p4 to develop November 15, 2021 16:47
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 ran all the tests locally on iPhone 12 (iOS 14.5) and they passed. As for the changes in files: everything looks legit to me, I have no additional comments or questions 👍 Thank you for doing this thorough but quite repetitive work @mokagio!

@mokagio mokagio marked this pull request as ready for review November 16, 2021 01:59
@mokagio mokagio merged commit 85ad440 into develop Nov 16, 2021
@mokagio mokagio deleted the convert-to-screen-objects-p5 branch November 16, 2021 02:00
@mokagio
Copy link
Contributor Author

mokagio commented Nov 16, 2021

Thank you for the reviews @pachlava 🙇‍♂️

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