Tests: Use REST API to delete templates and template parts #38524
Conversation
|
Size Change: 0 B Total Size: 1.15 MB ℹ️ View Unchanged
|
Mamaduka
force-pushed
the
try/rest-api-e2e-template-utils
branch
from
7edbd31 to
7b53d16
Compare
Mamaduka
added
[Type] Technical Prototype
|
Update: This test is currently skipped and we can update setup methods when it's enabled back. gutenberg/packages/e2e-tests/specs/site-editor/multi-entity-editing.test.js Lines 190 to 193 in 7b53d16 |
| method: 'DELETE', | ||
| path: `/wp/v2/posts/${ template.wp_id }?force=true`, |
There was a problem hiding this comment.
This should be faster than using the templates endpoint.
There was a problem hiding this comment.
This is not working properly in my tests
There was a problem hiding this comment.
Interesting. I'll have a look tomorrow or Monday.
Are you testing this util separately, or e2e tests are failing?
There was a problem hiding this comment.
I was debugging the template-part test in my PR and noticed that the template parts were not removed between runs.
There was a problem hiding this comment.
One limitation of this util (and in some parts of templates endpoint) is that it can only delete templates for the current theme.
But I will see if there are any other issues. If util isn't reliable, we can always revert to my original PR - #38486.
|
One of the tests was flagged as flaky from this PR, btw - #38583. |
|
Thanks, Dan. I will look into it. |
|
Hey @Mamaduka , what's the status of this PR? Currently @chad1008 has an open PR with a flaky test, and @talldan said that this PR could potentially contain the fix to unblock it (see #37535 (comment)) Thank you for working on this! |
|
Hey, @ciampo. It's funny you asked; I was just working to make this ready for review 😄 |
Mamaduka
force-pushed
the
try/rest-api-e2e-template-utils
branch
from
7b53d16 to
fe72b2f
Compare
Mamaduka
changed the title
There was a problem hiding this comment.
Awesome, thank you!
Ideally though, for every rest util we add, there should be a manual test for that action to prevent decreasing the test coverage. We should add a test for manually deleting templates and assert that it works, but we only have to do it once.
I think this can be in another PR though 👍 (or do we already have one?).
|
Thanks for the review, @kevin940726.
Is there an example of this for Widget utils? Then, I can use it as a foundation. |
We have this test to test deleting a widget and undoing it in the widgets screen. But it's really only testing undo/redo though. I guess we should also add it to widgets but they behave more like blocks and should already have tests for them somewhere. I'd imagine the test to be visiting the site editor, selecting a template in the templates list page, deleting it, and verifying it. It also would make sense to do this later though given that the screen is still technically in beta and could change in the future. |
|
Thanks for the clarification, @kevin940726. Using template list page was my first thought as well, I just wanted to be sure :)
Let's do it later when the list page screen is more stable. Meanwhile, I think this test can serve a similar purpose - fe72b2f3d606d4777578d37254927c518f57ec09. |
Mamaduka
force-pushed
the
try/rest-api-e2e-template-utils
branch
from
fe72b2f to
f2d3f4e
Compare
| } ); | ||
| afterAll( async () => { | ||
| await trashAllPosts( 'wp_template' ); | ||
| await trashAllPosts( 'wp_template_part' ); |
There was a problem hiding this comment.
Should we switch trashAllPosts to REST API too?
There was a problem hiding this comment.
We can. I don't see a reason for visiting post list table before every test.
There was a problem hiding this comment.
Not saying we should do so, just replacing the implementation of the current util.
There was a problem hiding this comment.
ICYMI, there's an ongoing project to migrate e2e tests to Playwright, which follows the best practices of using requestUtils to set/clear states between tests.
|
In case you missed it #38524 (comment) In my tests, the template parts are not being removed. |
|
@youknowriad, I am thinking to revert this to my original solution - #38486. While the REST API method works in most cases, it's still flaky. For example, this method can delete only the template for the active theme. What do you think? |
|
@Mamaduka I think if we can get the REST API to be stable and quick and use it in all "resets" (not just templates and template parts which are the small thing here), it might reduce the time it takes to run our workflows. That said it's just theory right, I'm totally fine with the alternative especially if we don't see any impact on job length. |
I think we can still do this for standard post types and maybe later for templates when they're not attached to the theme. I will create a new PR to see how template change affects e2e test run time. |
Description
Follow-up for #38486.
PR introduced a new e2e test utility function to handle template deletion. The new function uses REST API to delete templates.
Previously these were deleted using post type list UI, but this isn't possible after recent changes in the core.
Testing Instructions
All tests should pass.
Types of changes
Enhancement
Checklist:
*.native.jsfiles for terms that need renaming or removal).