-
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
Site Editor: Add e2e tests for templates export #28324
Conversation
07f7f0f
to
9019132
Compare
Size Change: -101 B (0%) Total Size: 1.37 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.
This looks good and the test is passing for me locally! A couple small Qs - also maybe @youknowriad may have some input here since he has worked with the export stuff a bit more.
@@ -100,3 +100,33 @@ export const navigationPanel = { | |||
await item.click(); | |||
}, | |||
}; | |||
|
|||
export const siteEditor = { |
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.
Nice! This was a bit overdue and is good to see. 😁
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.
When things become more stable (and core merge approaches), we could start extracting some of these to e2e-test-utills.
} | ||
} | ||
|
||
function delay( time ) { |
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 just a duplication of page.waitFor it seems.
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.
page.waitFor
is deprecated, so I'd rather avoid introducing it in new tests. I'm thinking about replacing delay
with page.waitForTimeout
.
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.
We may have to update our waitFor lint rule to also target the usage of waitForTimeout
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.
Glad to see more tests being added for FSE :)
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 looks good to me! ✅
New and effected tests are passing both locally and on CI.
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 tried running these test locally but they are failing for me for some reason on: TimeoutError: waiting for selector
.edit-site-visual-editor iframe failed: timeout 30000ms exceeded
.
I rerun the tests on the PR in CircleCI to double-check and it's still passing here, so I'm assuming something is messed up with my local setup. But now some seemingly unrelated e2e tests have failed here. :[
5e0b59d
to
6cfee3b
Compare
Did a rebase, hopefully fixing CI tests. It works fine locally. I usually run |
Curiously enough, I'm getting the same error lately whenever I try to run the e2e tests locally (while they do work in CI) -- most recently for #28554.
Maybe it's my Linux box -- tho AFAIK you're on macOS @vindl, right? Anyway, let's keep in touch in case either of us finds a solution to this. |
I think I tracked it down by running e2e tests in interactive mode:
Turns out e2e tests are stuck at a (pre-login) "Your database needs and upgrade" screen. I clicked the 'Upgrade' button, closed the testing window, and restarted tests. This time, they went through 🎉 We should document and/or fix this somewhere, tho 🤔 |
Possibly https://github.com/WordPress/gutenberg/blob/master/docs/contributors/testing-overview.md |
Yes, that's correct.
Thanks, I'll try this out! (update: yep, that fixed it 🎉) |
Description
Add end-to-end tests for site editor templates export functionality.
How has this been tested?
npm run test:e2e
Types of changes
Code Quality
Checklist: