Skip to content
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.

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

Reimplement waitForElementAndTap... as a typed XCUIElement method #22322

Merged

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Jan 3, 2024

Follows up on @tiagomar suggestion from https://github.com/wordpress-mobile/WordPress-iOS/pull/22270/files#r1439672960.

My intent with this is primary to facilitate merging #22270 and therefore officially support Xcode 15.1. Secondary is the showcase of what I think are two useful design practices:

  • Making tapping the element a method on the element itself, as opposed to a free function. This, I think, makes it clear what's being tapped as well as the existence of the method discoverable via the code-completion prompt
  • Using an enum over a String to "make inconsistent state unrepresentable" which is one of the most powerful features of a strongly typed language.

Testing

The UI tests passed for me locally:

image

And this is the CI build on the Apple Silicon test pipeline.

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.

PR submission checklist:

  • 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.

UI changes testing checklist: Not a UI PR.

"Typed" in that it expects an `enum` instead of a `String` to define the
expected element state.
@mokagio mokagio requested a review from a team as a code owner January 3, 2024 03:35
@mokagio mokagio added the Testing Unit and UI Tests and Tooling label Jan 3, 2024
@mokagio mokagio added this to the 24.0 milestone Jan 3, 2024
switch state {
case .exists: return element.exists
case .dismissed: return element.isHittable
case .selected: return element.isSelected
Copy link
Contributor Author

@mokagio mokagio Jan 3, 2024

Choose a reason for hiding this comment

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

Notice how thanks to the enum we no longer need to handle the case where the input is invalid:

default:
    XCTFail("\(condition) is invalid! Please choose 'exists', 'dismissed' or 'selected'")

It became unused in the previous commit,
b442842
@mokagio mokagio mentioned this pull request Jan 3, 2024
4 tasks
@mokagio
Copy link
Contributor Author

mokagio commented Jan 3, 2024

Update: I cancelled the build. Despite successful local run, CI got stuck finding the "Not Now" button/item:

image

More work to follow...

@mokagio
Copy link
Contributor Author

mokagio commented Jan 3, 2024

🤦‍♂️ Now I regret cancelling the build because I could have used the video of what the Simulator was showing...

Copy link
Contributor

@tiagomar tiagomar left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the issue so nicely, @mokagio!

The tests are passing locally and in the CI. :shipit:

guard condition.isMet() else {
sleep(UInt32(retryInterval))

return tapUntil(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice alternative to a conventional loop. I'd never come up with it. =D

Comment on lines +174 to +179
// FIXME: This needs to be addressed at the root but that requires more work than we have time for considering we want to support Xcode 15.1 ASAP.
// Tracked in https://github.com/wordpress-mobile/WordPress-iOS/issues/22323
XCTExpectFailure("The date conversion may fail at times, likely due to an underlying time zone inconsistency") {
XCTAssertEqual(dateComponents.month, expectedDateComponents.month)
XCTAssertEqual(dateComponents.day, expectedDateComponents.day)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@jostnes jostnes left a comment

Choose a reason for hiding this comment

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

Changes look good to me, too! Thanks for refactoring it to be clearer and simpler. I'll merge this into the branch to get rid of the unit test failure.

@jostnes jostnes merged commit cdc34d1 into ui-test-fixes-ios17.2 Jan 4, 2024
@jostnes jostnes deleted the mokagio/refactor-waitForElementAndTap branch January 4, 2024 03:14
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