Improve 'can add custom product attributes' flaky test #59762
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6f0db36 to
100b181
Compare
Testing GuidelinesHi @kmanijak , 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:
|
📝 WalkthroughWalkthroughA synchronization step was added to the end-to-end test for creating product attributes in WooCommerce, ensuring the UI is ready before interacting with the "Attributes" tab. Additionally, a changelog entry documents this patch as a fix for a flaky test. No functional code changes were made outside the test. Changes
Possibly related PRs
✨ 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
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugins/woocommerce/changelog/fix-53449-flaky-test (1)
1-6: LGTM! Changelog entry follows WooCommerce conventions.The changelog entry correctly documents the test fix as a patch-level development change. The format aligns with WooCommerce's changelog structure in the
plugins/woocommerce/changelog/directory.Minor formatting suggestion:
Significance: patch Type: dev Comment: Improve 'can add custom product attributes' flaky test - -
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/woocommerce/changelog/fix-53449-flaky-test(1 hunks)plugins/woocommerce/tests/e2e-pw/tests/product/create-product-attributes.spec.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{php,js,ts,jsx,tsx}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/generate-pr-description.mdc:0-0
Timestamp: 2025-06-30T09:26:55.361Z
Learning: Provide clear, step-by-step instructions for how to test the changes in the PR description.
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.981Z
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.
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.815Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in `plugins/woocommerce/changelog/` directory and are processed during releases, not at the repository root level.
Learnt from: ralucaStan
PR: woocommerce/woocommerce#58782
File: plugins/woocommerce/client/blocks/assets/js/base/utils/render-frontend.tsx:0-0
Timestamp: 2025-06-16T16:12:12.148Z
Learning: For WooCommerce checkout blocks, lazy loading was removed in favor of direct imports to prevent sequential "popping" effects during component loading. This approach prioritizes user experience over code splitting, with minimal bundle size impact and improved performance (1.7s to 1.1s speed score improvement). The checkout flow benefits from having all components load together rather than incrementally.
Learnt from: NeosinneR
PR: woocommerce/woocommerce#0
File: :0-0
Timestamp: 2025-06-26T14:25:08.421Z
Learning: In WooCommerce transactional emails, product images have historically had display issues due to lazy loading attributes being applied, which email clients cannot process since they don't execute JavaScript. This issue existed in both old and new email templates, but became more visible with the new email template system. The fix involves preventing lazy loading on attachment images specifically during email generation by adding skip classes and data attributes.
Learnt from: vladolaru
PR: woocommerce/woocommerce#58784
File: plugins/woocommerce/client/admin/client/settings-payments/onboarding/components/stepper/index.tsx:57-67
Timestamp: 2025-06-20T13:08:44.017Z
Learning: For WooCommerce Payments onboarding step tracking in useEffect hooks, only include the active step in the dependency array, not the context object. The sessionEntryPoint is static throughout the onboarding session and tracking should only fire on actual step navigation, not context changes.
plugins/woocommerce/changelog/fix-53449-flaky-test (7)
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.815Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in `plugins/woocommerce/changelog/` directory and are processed during releases, not at the repository root level.
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.981Z
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.
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/woo-phpunit.mdc:0-0
Timestamp: 2025-06-30T09:27:17.200Z
Learning: Applies to plugins/woocommerce/tests/**/*.php : Run WooCommerce PHPUnit tests for specific files or directories using the command: pnpm run test:php:env {relative_path} --verbose, and ensure the command is run in the plugins/woocommerce directory.
Learnt from: opr
PR: woocommerce/woocommerce#0
File: :0-0
Timestamp: 2025-06-20T17:38:16.565Z
Learning: WooCommerce legacy JavaScript files in plugins/woocommerce/client/legacy/js/ must use older JavaScript syntax and cannot use modern features like optional chaining (?.) due to browser compatibility requirements. Explicit null checking with && operators should be used instead.
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/woo-build.mdc:0-0
Timestamp: 2025-06-30T09:27:12.787Z
Learning: To watch for changes, use `pnpm --filter=@woocommerce/plugin-woocommerce watch:build`
Learnt from: samueljseay
PR: woocommerce/woocommerce#58716
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/iapi-frontend.ts:83-101
Timestamp: 2025-06-17T07:07:53.443Z
Learning: In WooCommerce blocks, when porting existing code patterns that have known issues (like parseInt truncating decimal money values), maintain consistency with existing implementation rather than making isolated fixes. The preference is for systematic refactoring approaches (like broader Dinero adoption) over piecemeal changes.
Learnt from: lysyjan
PR: woocommerce/woocommerce#59632
File: packages/js/email-editor/src/layouts/flex-email.tsx:116-122
Timestamp: 2025-07-14T10:41:46.150Z
Learning: In WooCommerce projects, formatting suggestions should respect the project's Prettier configuration and linting rules. Changes that would break the lint job should be avoided, even if they appear to improve readability.
plugins/woocommerce/tests/e2e-pw/tests/product/create-product-attributes.spec.js (2)
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.981Z
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.
Learnt from: gigitux
PR: woocommerce/woocommerce#58899
File: plugins/woocommerce/tests/e2e-pw/tests/customize-store/assembler/font-picker.spec.js:250-265
Timestamp: 2025-06-17T08:14:15.688Z
Learning: In Playwright E2E tests, page.waitForResponse() should be called BEFORE the triggering action (like clicks) to ensure the response is not missed. This is the recommended pattern per Playwright documentation: const responsePromise = page.waitForResponse(...); await triggerAction(); await responsePromise;
🔇 Additional comments (1)
plugins/woocommerce/tests/e2e-pw/tests/product/create-product-attributes.spec.js (1)
178-186: Excellent synchronization fix for the flaky test!The added wait condition effectively addresses the race condition by ensuring the UI state is stable before interacting with the "Attributes" tab. The approach is well-documented with clear comments explaining the rationale.
Key strengths:
- Waits for a specific UI state change (Variations tab becoming hidden)
- Uses proper Playwright locator chaining with
.attribute_tabcontainer- Clear comments explain the timing issue and solution
- Follows E2E testing best practices for handling dynamic UI updates
This should eliminate the flakiness described in issue #53449.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this Pull Request:
Closes #53449.
I think the test was flaky because we were clicking on the Attributes button before it was interactive. This PR waits for the tabs to update before clicking on it.
How to test the changes in this Pull Request: