-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[UI Tests] - Fix failing testAddMediaBlocks() test #21582
Conversation
|
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr21582-09d9f20 | |
Version | 23.2 | |
Bundle ID | org.wordpress.alpha | |
Commit | 09d9f20 | |
App Center Build | WPiOS - One-Offs #7102 |
|
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr21582-09d9f20 | |
Version | 23.2 | |
Bundle ID | com.jetpack.alpha | |
Commit | 09d9f20 | |
App Center Build | jetpack-installable-builds #6146 |
@@ -100,6 +100,8 @@ class EditorGutenbergTests: XCTestCase { | |||
} | |||
|
|||
func testAddMediaBlocks() throws { | |||
try XCTSkipIf(!XCUIDevice.isPad, "Test currently fails on iPhone in CI - Tapping on the coordinate location results in a blank screen. Skipping test on iPhone.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This problem only affects the Video and Audio blocks due to inserting media via URL. So I'm wondering if we could only skip those. Maybe we could have two different UI tests depending on the device:
- iPhone: Test Gallery block
- iPad: Test Gallery, Video, and Audio blocks
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't recommend breaking it into two tests, especially if the blocks should behave the same on both iPhone and iPad, I also believe that this should be a temporary and not a permanent skip.
I understand the concern around test coverage, though. So to handle this, I moved the skip after adding the image for the iPhone test so the test would still test up to add image, if the image fails to load, the test would still fail. I made the changes in 09d9f20
wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't recommend breaking it into two tests, especially if the blocks should behave the same on both iPhone and iPad, I also believe that this should be a temporary and not a permanent skip.
Yeah, good point.
I understand the concern around test coverage, though. So to handle this, I moved the skip after adding the image for the iPhone test so the test would still test up to add image, if the image fails to load, the test would still fail. I made the changes in 09d9f20
Great idea, I'll take a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎊 ! Thanks @jostnes for addressing the UI test failure 🙇 !
Description
Test failed because of a UI change when adding media blocks via URL. In the updated design, the test no longer needs to tap on a specific button. A swipe-down gesture or a tap anywhere on the screen should now dismiss the view.
The fix for this is to useswipeDown()
instead. I also removed the button and getter since it's no longer referenced anywhere on the screen.The
swipeDown()
method didn't work when the test ran in CI. I createdtapTopOfScreen()
to tap a specific coordinate on the screen to dismiss the view instead. However, this also doesn't work on iPhone in CI for reasons I don't understand yet. So, the test is only set to run on iPad until there's a better solution.Testing
CI should be 🟢