-
Notifications
You must be signed in to change notification settings - Fork 4.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
E2E Tests: Fix failing image e2e test by waiting for required element #36982
Conversation
Size Change: 0 B Total Size: 1.1 MB ℹ️ View Unchanged
|
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 also tested 23db42264d1029361fa93cc7ec3fb3520492872f, which reinstates the test from #36446 They're passing locally for me. 🤞 for the CI results. |
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 👍
The latest waitForImage
approach tests well for me and passes reliably locally. For what it's worth, the previous waitForSelector
approach failed consistently for me locally.
b7a6274
to
cdbe93a
Compare
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.
Thanks for this!
Thanks for working on this, @glendaviesnz 🌟 I'm going to restart e2e tests and merge this once all checks are green. Then, we can address other flaky tests separately. |
@andrewserong and I have been testing this locally and finding the remaining image e2es are failing intermittently. Every other run to be precise. We might need a little luck to get all green checks 🤞 |
Or we could use administrator privileges and merge it anyways. It only will make the current status of e2e tests better 😄 |
This error is the current reason for test failure. I've seen it a dozen times in the last couple of days but cannot reproduce it locally. Error
|
I wasn't looking to push that option given locally I'm getting more failures on this branch than before. Given the inconsistency, if merging this lets us narrow the investigation, I'm fine with that 🙂 |
I think I have an idea for what could be causing the failing I'll push a commit shortly. Edit: pushed in 2eb00f2 The hunch is that there's a couple of things at play. I suspect that doing things like the following might cause an issue for React controlled components (because we're updating the value of input field from outside of React):
So, it could be more appropriate for us to try to clear fields as a user would? Then, the other thing at play is how quickly the UI will let us type. Adding in a delay appears to help reliability when running the tests locally, but as Aaron mentions, we're still experiencing intermittent failures 🤔 |
Initially, this looked like it fixed the issue with I also still got the |
await inputField.click( { clickCount: 3, delay: 100 } ); | ||
await page.keyboard.press( 'Backspace', { delay: 100 } ); |
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.
Keeping the total delay overall the same but just using all of it to ensure the selection of the input field text has the e2es passing more reliably for me.
await inputField.click( { clickCount: 3, delay: 100 } ); | |
await page.keyboard.press( 'Backspace', { delay: 100 } ); | |
await inputField.click( { clickCount: 3, delay: 200 } ); |
@Mamaduka I haven't been able to get the checks green despite passing e2es locally. We might need to merge this with admin privileges after all so at least the PHP unit tests are fixed. I've run out of time today but I've dropped a comment pointing to this PR in the |
Sounds good, @aaronrobertshaw 👍 |
I ran the E2E tests locally on fresh
|
This is curious. Seems to be the whole test suite failing to run. Is it some kind of issue with authentication since |
That's also my best guess, maybe issue with auth cookies. |
describe( 'autocomplete mentions', () => { | ||
// Something in WP 5.9 RC has broken the creation of new users so disabling until | ||
// that is fixed | ||
describe.skip( 'autocomplete mentions', () => { |
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 is passing in #36987. Might be worth enabling it again 😄
The unsual thing is that it's only that image test and I see nothing special about it. Other than it possibly being the first test run in the series. |
2eb00f2
to
bcb9f40
Compare
I merged the PHP test fix separately in #36987. Also rebased this and re-enabled the autocomplete tests as they now seem to be passing in trunk. |
bcb9f40
to
82727b6
Compare
… buttons, and by clicking image block wrapper to delete block
c7f6dd6
to
1902936
Compare
* Fix failing image e2e test by waiting for required element before attempting click * Switch to waiting for image instead of the image resize buttons in image block e2e tests * Add delay when clearing input field and typing replacement url in image e2e tests * Add in wait for user input box to prevent username being partially entered in mentions e2e test * Add phpunit-polyfills to fix running of php-unit tests locally - related to https://make.wordpress.org/core/2021/09/27/changes-to-the-wordpress-core-php-test-suite/ Co-authored-by: Glen Davies <glen.davies@a8c.com> Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Co-authored-by: Daniel Richards <daniel.richards@automattic.com>
Description
The image block e2e tests are consistently failing due to element not being visible when attempting to click. This fix adds a
page.waitForSelector
to ensure element is visible before attempting the click.There is also an update to the test for
Gutenberg_REST_Templates_Controller_Test
that updates thetest_create_item
test to expect the author id in the response to match the id used to create the resource (self::$admin_id
).How has this been tested?
Run
npm run test-e2e specs/editor/blocks/image.test.js
locally and make sure tests pass.