-
Notifications
You must be signed in to change notification settings - Fork 4.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
E2E Test Utils: Try improved stability for transformBlockTo #18771
Conversation
Thanks for having a look. I'll take the liberty to restart the build a few times to see if it makes any difference. :) |
Unrelated intermittent e2e test failure:
|
|
Another unrelated intermittent test failure:
|
Since this does seem to have stabilized things, it gets me wondering what of the specific changes would have had the most impact here, so we can make sure to try to keep it in mind for future tests-authoring.
I don't have the answer right now, but I think it's worth keeping in mind to try to avoid future issues. |
This pull request attempts to try to resolve some intermittent E2E test failures, where test failures often occur in the
transformBlockTo
utility.Example:
While I was never able to reproduce the issue locally, I attempted to make revisions based on the above information, specifically the fact that it became stuck at the last line of the function (
waitForSelector
) means that all of the preceding interactions were successful (display the switcher, find a button with the text "Quote" and click it).The proposed changes aren't guaranteed to improve the tests, but are based on the following observations:
span
of the button on which to perform the click, not thebutton
itself where the event handler is bound. With these changes, it now matches thebutton
.span
but not registering as a click on the button?span
descendent which included the text "Quote". With these changes, the XPath selector is limited to match within the switcher popover.[contains(text(), '...')]
to match the text. With these change, the text is now matched using[.='...']
.text()
is reasonably strict about placement of text nodes. Perhaps there were some unexpected conditions where additional DOM nodes might be added? Looking at the code, it doesn't appear it should be the case. But for the purposes of the utility, it's not really concerned with strict conformance. Contrast with the proposed.=
, which is both simpler and more lenient about queried text in that it queries on the string concatenation of all text nodes in the parent (reference).I also included a change to use keyboard commands to navigate to the block toolbar and expand the switcher container, in place of the "mouse wiggle" interaction. As noted, since the function was able to successfully reach the final line of its execution, I don't expect this should have an impact one way or the other, but generally I would find the
page.mouse.move
approach to be less reliable overall.