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

[UI Tests] - Fix failing testAddMediaBlocks() test #21582

Merged
merged 5 commits into from
Sep 15, 2023

Conversation

jostnes
Copy link
Contributor

@jostnes jostnes commented Sep 15, 2023

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 use swipeDown() 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 created tapTopOfScreen() 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 🟢

@jostnes jostnes added the Testing Unit and UI Tests and Tooling label Sep 15, 2023
@jostnes jostnes added this to the 23.4 milestone Sep 15, 2023
@jostnes jostnes requested a review from a team as a code owner September 15, 2023 03:25
@jostnes jostnes enabled auto-merge September 15, 2023 03:39
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 15, 2023

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr21582-09d9f20
Version23.2
Bundle IDorg.wordpress.alpha
Commit09d9f20
App Center BuildWPiOS - One-Offs #7102
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 15, 2023

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr21582-09d9f20
Version23.2
Bundle IDcom.jetpack.alpha
Commit09d9f20
App Center Buildjetpack-installable-builds #6146
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@jostnes jostnes requested a review from fluiddot September 15, 2023 07:53
@@ -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.")
Copy link
Contributor

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:

  1. iPhone: Test Gallery block
  2. iPad: Test Gallery, Video, and Audio blocks

WDYT?

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor

@fluiddot fluiddot left a 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 🙇 !

@jostnes jostnes merged commit 9f34e72 into trunk Sep 15, 2023
@jostnes jostnes deleted the fix-add-media-blocks-test branch September 15, 2023 13:25
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