-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix failing tests and compatibility with 5.9. #36368
Conversation
This reverts commit fdf7185.
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.
// Wait for media modal to appear and upload image. | ||
await page.waitForSelector( '.media-modal input[type=file]' ); | ||
const inputElement = await page.$( '.media-modal input[type=file]' ); | ||
const inputElement = await page.waitForSelector( |
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 couldn't tell why this test was failing. The image upload just wasn't happening, but it works when testing manually. I've modified it to upload the image in a different way, using the image block placeholder's Upload button. This is actually more succinct, though reduces test coverage for the media modal.
It could be related to this core bug or other changes to the media modal - https://core.trac.wordpress.org/ticket/54411
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.
Though it might actually be better to have test coverage for the placeholder upload button, given there are other tests for using the media modal. I think it's worth keeping this change.
Size Change: +4 B (0%) Total Size: 1.09 MB
ℹ️ View Unchanged
|
A lot of tests are failing because the Menus REST API hasn't been updated to use This was resulting in a 404 error that basically seems to have caused every site editor test to fail. This will need to be backported to core so that core isn't using the Possibly the same for navigation areas eventually too when https://core.trac.wordpress.org/ticket/54393 is patched. |
83a8a55
to
5299541
Compare
5299541
to
d9ded0b
Compare
All the remaining e2e test failures are like the other image upload failure mentioned here - #36368 (comment) Still unsure why they would be failing 🤔 |
Ok, have gotten to the bottom of this with some help. It seems the change in https://core.trac.wordpress.org/changeset/52059 is causing an issue with our e2e tests that interact with the media modal. Our tests usually use a hidden I've pushed a hopeful fix, but if it doesn't work I'd suggest skipping those tests until the issue in core is resolved (https://core.trac.wordpress.org/ticket/54411). |
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.
Changes look good to me 👍
I'm going to merge this and keep an eye on image e2e tests.
Thank you so much @talldan! |
This is currently a draft PR, it reverts #36333. I'll push fixes here that are required for getting tests passing against WordPress 5.9.
A summary of the changes: