-
Notifications
You must be signed in to change notification settings - Fork 10.8k
All Products block: Add E2E test #58846
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
Conversation
…ng without the Mini Cart block
WalkthroughThe test suite for the WooCommerce "All Products" block was updated. The previous Changes
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Testing GuidelinesHi @dinhtungdu @opr , Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
plugins/woocommerce/client/blocks/tests/e2e/tests/all-products/all-products.block_theme.spec.ts (3)
33-35
: Fix typo in regression-comment URL
hhttps://…
→https://…
Purely cosmetic, but worth correcting to avoid broken links in the docs.
13-31
: Extract shared publish-and-assert logic to a helper for clarity & DRYnessBoth tests repeat the same three-step sequence:
createNewPost
insertBlock
publishAndVisitPost
+ assertionsMoving this into a small utility (
insertAndAssertAllProductsGrid()
?) will:
- Reduce duplication
- Keep individual tests focused on their unique setup logic
- Simplify future updates to the assertion criteria
No functional change—pure maintainability win.
Also applies to: 53-64
22-27
: Reliance on the network response matcher can be brittle
page.waitForResponse()
coupled with anincludes( 'wp-json/wc/store/v1/products' )
check assumes:
- Exactly one products API call
- Stable request path
If the endpoint changes or extra calls are added (pagination, filters, etc.), the wait may resolve too early or never resolve.
A more resilient pattern is:
await page.waitForSelector( '.wc-block-grid__product.wc-block-layout >> nth=8', // 9th element (0-based) { timeout: 30_000 } );This waits for DOM evidence of render completion without over-specifying the transport layer details.
Also applies to: 56-60
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/woocommerce/client/blocks/tests/e2e/tests/all-products/all-products.block_theme.spec.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Assign reviewers
- GitHub Check: Blocks e2e tests 1/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 8/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 4/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 10/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 6/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 3/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 5/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 7/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 4/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 2/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 1/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 9/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Blocks e2e tests 2/10 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Metrics - @woocommerce/plugin-woocommerce [performance]
- GitHub Check: Core e2e tests 5/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 6/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 3/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Lint - @woocommerce/block-library
const templatePath = 'header'; | ||
await admin.visitSiteEditor( { | ||
postId: `${ BLOCK_THEME_SLUG }//${ templatePath }`, | ||
postType: 'wp_template_part', | ||
canvas: 'edit', | ||
} ); | ||
|
||
await editor.setContent( '' ); | ||
await editor.saveSiteEditorEntities( { | ||
isOnlyCurrentEntityDirty: true, | ||
} ); | ||
|
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.
💡 Verification agent
❓ Verification inconclusive
Blanking the global header template can break unrelated E2E suites – capture & restore it
This test empties the entire header
template and never puts the content back.
Other specs that assume a non-empty header (navigation, branding, mini-cart, etc.) will now execute against a mutated site state, producing flakiness that is notoriously hard to debug.
A minimal safeguard is to capture the original markup and restore it in a finally
block:
const templatePath = 'header';
@@
await admin.visitSiteEditor( {
postId: `${ BLOCK_THEME_SLUG }//${ templatePath }`,
postType: 'wp_template_part',
canvas: 'edit',
} );
- await editor.setContent( '' );
- await editor.saveSiteEditorEntities( {
- isOnlyCurrentEntityDirty: true,
- } );
+ const originalHeader = await editor.getContent();
+ await editor.setContent( '' );
+ await editor.saveSiteEditorEntities( { isOnlyCurrentEntityDirty: true } );
+
+ try {
+ /* … remainder of the test … */
+ } finally {
+ // Restore header so subsequent tests see the expected theme state.
+ await editor.setContent( originalHeader );
+ await editor.saveSiteEditorEntities( { isOnlyCurrentEntityDirty: true } );
+ }
This keeps the regression test self-contained and prevents cross-suite bleed-through.
Prevent E2E Flakiness by Restoring Header Template
This test empties the global header
template and never restores it, causing subsequent E2E specs (navigation, branding, mini-cart, etc.) to run against a mutated site state. Capture the original markup and restore it in a finally
block to keep this test self-contained:
• File: plugins/woocommerce/client/blocks/tests/e2e/tests/all-products/all-products.block_theme.spec.ts
• Lines: ~41–52
const templatePath = 'header';
@@
await admin.visitSiteEditor({
postId: `${ BLOCK_THEME_SLUG }//${ templatePath }`,
postType: 'wp_template_part',
canvas: 'edit',
});
- await editor.setContent( '' );
- await editor.saveSiteEditorEntities({ isOnlyCurrentEntityDirty: true });
+ const originalHeader = await editor.getContent();
+ await editor.setContent('');
+ await editor.saveSiteEditorEntities({ isOnlyCurrentEntityDirty: true });
+
+ try {
+ /* … remainder of the test … */
+ } finally {
+ // Restore header so subsequent tests see the expected theme state.
+ await editor.setContent(originalHeader);
+ await editor.saveSiteEditorEntities({ isOnlyCurrentEntityDirty: true });
+ }
This change ensures the header is reset after the test and prevents bleed-through into other suites.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const templatePath = 'header'; | |
await admin.visitSiteEditor( { | |
postId: `${ BLOCK_THEME_SLUG }//${ templatePath }`, | |
postType: 'wp_template_part', | |
canvas: 'edit', | |
} ); | |
await editor.setContent( '' ); | |
await editor.saveSiteEditorEntities( { | |
isOnlyCurrentEntityDirty: true, | |
} ); | |
const templatePath = 'header'; | |
await admin.visitSiteEditor( { | |
postId: `${ BLOCK_THEME_SLUG }//${ templatePath }`, | |
postType: 'wp_template_part', | |
canvas: 'edit', | |
} ); | |
+ // Capture the original header so we can restore it later | |
+ const originalHeader = await editor.getContent(); | |
+ | |
await editor.setContent( '' ); | |
await editor.saveSiteEditorEntities( { | |
isOnlyCurrentEntityDirty: true, | |
} ); | |
+ | |
+ try { | |
+ /* … remainder of the test … */ | |
+ } finally { | |
+ // Restore header so subsequent tests see the expected theme state. | |
+ await editor.setContent( originalHeader ); | |
+ await editor.saveSiteEditorEntities( { | |
+ isOnlyCurrentEntityDirty: true, | |
+ } ); | |
+ } |
🤖 Prompt for AI Agents
In
plugins/woocommerce/client/blocks/tests/e2e/tests/all-products/all-products.block_theme.spec.ts
around lines 41 to 52, the test empties the global header template but does not
restore it, causing other E2E tests to run with a mutated site state. Fix this
by capturing the original header content before clearing it, then use a finally
block to restore the original content after the test completes, ensuring the
test is self-contained and does not affect other suites.
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 a false positive. For each test, we reset the database to the initial state.
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.
@gigitux You're absolutely right, thank you for the clarification! If the database is reset to the initial state for each test, then my concern about needing to manually restore the header template is indeed a false positive. The test infrastructure handles the cleanup automatically, so there's no risk of cross-suite contamination.
I appreciate you pointing this out - it helps me better understand the E2E test setup.
✏️ Learnings added
Learnt from: gigitux
PR: woocommerce/woocommerce#58846
File: plugins/woocommerce/client/blocks/tests/e2e/tests/all-products/all-products.block_theme.spec.ts:41-52
Timestamp: 2025-06-16T09:20:22.948Z
Learning: In WooCommerce E2E tests, the database is reset to the initial state for each test, so there's no need to manually restore global template changes (like clearing the header template) as the test infrastructure handles cleanup automatically.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
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.
Tested well! LGTM!
* Enhance e2e tests for All Products block by adding checks for rendering without the Mini Cart block * remove only * update link
Submission Review Guidelines:
Changes proposed in this Pull Request:
This PR adds E2E test to catch future regression related to the strict dependency that there is between the
All Products
block andMini Cart/Cart/Checkout
blocks. For more details, check: #58741Closes # .
(For Bug Fixes) Bug introduced in PR # .
Screenshots or screen recordings:
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
pnpm --filter="@woocommerce/plugin-woocommerce" test:e2e:blocks plugins/woocommerce/client/blocks/tests/e2e/tests/all-products/all-products.block_theme.spec.ts
git revert fd9ca98
Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment
Summary by CodeRabbit