Reimplement waitForElementAndTap... as a typed XCUIElement method - #22322
Merged
jostnes merged 6 commits intoJan 4, 2024
Conversation
"Typed" in that it expects an `enum` instead of a `String` to define the expected element state.
mokagio
commented
Jan 3, 2024
| switch state { | ||
| case .exists: return element.exists | ||
| case .dismissed: return element.isHittable | ||
| case .selected: return element.isSelected |
Contributor
Author
There was a problem hiding this comment.
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
4 tasks
Contributor
Author
|
Update: I cancelled the build. Despite successful local run, CI got stuck finding the "Not Now" button/item: 
More work to follow... |
Contributor
Author
|
🤦♂️ Now I regret cancelling the build because I could have used the video of what the Simulator was showing... |
To follow up via #22323
🤦♂️ I didn't notice the `!` at the start of the check when porting it over.
tiagomar
approved these changes
Jan 4, 2024
| guard condition.isMet() else { | ||
| sleep(UInt32(retryInterval)) | ||
|
|
||
| return tapUntil( |
Contributor
There was a problem hiding this comment.
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) | ||
| } |
jostnes
approved these changes
Jan 4, 2024
jostnes
left a comment
jostnes
left a comment
Contributor
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
enumover aStringto "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:
And this is the CI build on the Apple Silicon test pipeline.
Regression Notes
PR submission checklist:
RELEASE-NOTES.txtif necessary. N.A.UI changes testing checklist: Not a UI PR.