-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Gutenberg: Trigger E2E suite for v9.9.x upgrade #49599
Conversation
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
84ead44
to
f320ac6
Compare
f320ac6
to
336ccf8
Compare
Rebased and running with 9.9.0 stable on edge. |
336ccf8
to
7662e0d
Compare
Rebased and running with 9.9.1 stable on edge. |
7662e0d
to
357bf2a
Compare
357bf2a
to
fd2e54e
Compare
Rebased and running with 9.9.2 stable on edge. |
d7ad0c9
to
7fab6d4
Compare
initsWithChildFocus ? 'has-child-selected' : 'is-selected' | ||
}[aria-label*='${ ariaLabel }']`; | ||
|
||
let insertedBlockSelector = By.css( insertedBlockSelectorStr ); |
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.
One of the things I wanted to refactor at some point - the By
interface has "locator strategies" that return locator
s, not selector
s. A selector
in this case is actually the string passed to the locator strategy (css()
in this case). We use the selector naming for locators everywhere, so we should keep doing that to stay consistent, IMO. Just thought I'd mention the "locators" for future reference :)
https://www.selenium.dev/documentation/en/getting_started_with_webdriver/locating_elements/
Because of this PR that just got merged (planned for v10.1), this might end up not being a workaround at all:
We need to keep changes from that PR in mind as they can cause more failed specs in our E2E GB suites. edit Looks like we have a fix already! :D WordPress/gutenberg#28962 |
…by the main inserter Workaround for WordPress/gutenberg#28932. To be reverted once we get a fix from GB core.
751d8e5
to
d8269e4
Compare
Summary of failing mobile specs:✅ Currently, there are no failing Mailosaur-related (ignore, see p1612456065018600-slack-C1A1EKDGQ for context):
Non-Gutenberg-edge-related:
Gutenberg-related:
Summary of failing desktop specs:✅ Currently, there are no failing Mailosaur-related (ignore, see p1612456065018600-slack-C1A1EKDGQ for context):
Non-Gutenberg-edge-related:
Overall, I don't see any specs that are related to Gutenberg v9.9. @fullofcaffeine's fix is working nicely, so I think we can ship this one and move on with the upgrade. cc @griffbrad |
This reverts commit 30f1c62.
* Revert "Gutenberg: Trigger E2E suite for v9.9.x upgrade (#49599)" This reverts commit 30f1c62. * Improve the main options menu selector It fails in the mobile viewport because it matches the block context menu, which is returned instead. * Close the welcome modal dialog before trying to select a page template in the mobile screen size * Use clickIfPresent to close the welcome dialog, as it might not show in all scenarios Co-authored-by: Marcelo Serpa <81248+fullofcaffeine@users.noreply.github.com>
Tracking issue: #49534.
Includes fix for failing tests where inserted block focus is stolen by the restore focus hook. See WordPress/gutenberg#28932 for more details.
ToDo:
A fix for the focus issue mentioned above has been already merged and will be included in the v10.0 release. This PR might need to be reverted when we upgrade to that version.
What about failing specs?
See the summary here: #49599 (comment)