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

test: Mitigate media block test flakiness #21639

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

dcalhoun
Copy link
Member

@dcalhoun dcalhoun commented Sep 27, 2023

Relates to wordpress-mobile/gutenberg-mobile#6233. Partially reverts #21582.

Mitigate media block test flakiness by pasting URLs rather than typing them. The
rapid rate at which XCTests type resulted in dropped URL characters leading to
invalid media URLs. This is undoubtedly a performance issue that could be
addressed directly, but pasting the URL feels like an appropriate solution for
resolving the test issue for now.

To test: ensure the CI tasks succeed.

Regression Notes

  1. Potential unintended areas of impact
    Other automated tests could begin failing.
  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Verifying CI tasks succeed.
  3. What automated tests I added (or what prevented me from doing so)
    Improved the robustness of media block tests.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes testing checklist:

  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

The rapid typing within XCTest occasionally results in dropped
characters in the "Insert from URL" text field. This means a valid URL
of `https://wordpress.org/video.mp4` ends up captured as
`h://wordpress.org/video.mp4`.

Addressing the underlying performance issue might be ideal, but pasting
the entire URL instead of typing is a reasonable workaround for now.
The previous predicates passed after merely adding a block, as it
asserted aspects that do not verify the act of attaching media. The new
predicates are only present if attaching media to the blocks succeeds.
This increases the coverage for the tests.
The other changes in this branch appear to resolve the issues causing
iPhone failures.
@dcalhoun dcalhoun requested a review from a team as a code owner September 27, 2023 00:16
@dcalhoun dcalhoun added Testing Unit and UI Tests and Tooling Gutenberg Editing and display of Gutenberg blocks. labels Sep 27, 2023
@dcalhoun dcalhoun self-assigned this Sep 27, 2023
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 27, 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 Numberpr21639-d0a0f2a
Version23.3
Bundle IDorg.wordpress.alpha
Commitd0a0f2a
App Center BuildWPiOS - One-Offs #7225
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 27, 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 Numberpr21639-d0a0f2a
Version23.3
Bundle IDcom.jetpack.alpha
Commitd0a0f2a
App Center Buildjetpack-installable-builds #6267
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

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.

Thanks for updating the test! And good catch finding out the reason of failure. Changes related to adding URL looks good and CI is 🟢 but I added one question related to the updated assertion that I'd like to understand before approving. Thanks!

Asserting the block position in addition to the media attachment
increases the robustness of the test.
@peril-wordpress-mobile
Copy link

Warnings
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

@dcalhoun dcalhoun added this to the 23.4 milestone Sep 28, 2023
@dcalhoun dcalhoun requested a review from jostnes September 28, 2023 12:54
@dcalhoun dcalhoun enabled auto-merge September 28, 2023 12:55
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.

Thank you for considering the feedback and making the changes! :shipit:

@dcalhoun dcalhoun merged commit 41120bd into trunk Sep 28, 2023
@dcalhoun dcalhoun deleted the test/mitigate-flaky-media-block-case branch September 28, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Editing and display of Gutenberg blocks. Testing Unit and UI Tests and Tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants