[E2E][QIT] Migrate to Test Packages infrastructure #11175
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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the QIT E2E tests infrastructure from a custom Docker-based setup to the recommended Test Packages approach. The migration modernizes the test infrastructure with better isolation, simpler configuration, and alignment with QIT best practices.
Key changes:
- Adopts Test Packages structure with dedicated test package directory (
tests/qit/test-package/) - Removes legacy custom E2E infrastructure (Docker volumes, custom runner scripts,
qit.yml) - Implements QIT-native configuration with
qit.jsonandqit-test.jsonmanifests - Updates test utilities to use the new
@qit/helpersmodule - Fixes race condition in WC Blocks checkout save payment checkbox
Reviewed changes
Copilot reviewed 31 out of 42 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tests/qit/test-package/** |
New test package infrastructure with Playwright tests, utilities, fixtures, and configuration |
tests/qit/test-package/bootstrap/*.sh |
Bootstrap scripts for WordPress/WooCommerce setup and Jetpack connection |
tests/qit/test-package/qit-helpers/** |
QIT helper module for WP-CLI operations and test utilities |
tests/qit/qit.json |
Main QIT configuration defining SUT, environments, and test packages |
tests/qit/qit.yml |
Removed legacy QIT configuration for custom tests |
tests/qit/e2e-runner.sh |
Removed legacy custom test runner script |
tests/qit/e2e/utils/helpers.ts |
Removed legacy helper functions replaced by test package utilities |
tests/qit/e2e/.eslintrc.js |
Removed legacy ESLint configuration |
package.json |
Updated npm scripts to use QIT CLI directly with new configuration |
composer.json |
Updated QIT CLI to dev-trunk for test packages support |
.eslintignore, .prettierignore |
Added test-package directory to ignore lists |
tests/qit/README.md |
Updated documentation with test packages approach |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Contributor
|
Size Change: 0 B Total Size: 877 kB ℹ️ View Unchanged
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 41 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
tests/qit/test-package/bootstrap/setup.sh:109
- The user creation commands reference emails that don't match those in the configuration file. For example, line 95 creates a user with email
[email protected], but the config file (tests/qit/test-package/config/users.json) specifies[email protected]. This inconsistency could lead to issues if the tests expect specific email addresses. The same issue exists for subscriptions-customer (line 102) and editor (line 109).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Migrates 8 shopper test specs to QIT: My Account: - My account payment methods add failures - My account saved cards management Multicurrency & Pay for Order: - Multi-currency checkout - Multi-currency widget - Pay for order flow Alternative Payment Methods: - Alipay checkout purchase - Klarna checkout purchase - BNPLs (Buy Now Pay Later) checkout Tests cover shopper flows for account management, currency switching, and alternative payment methods.
- Alipay: Disable multi-currency before tests (like Klarna does) - Alipay: Use URL check for redirect instead of page text - Pay for order: Handle both 'card was declined' and 'payment was not processed' errors
Alipay payments require specific Stripe account configuration that is not available in the QIT test environment. The payment method can be enabled in settings but checkout fails with 'Invalid or missing payment details'.
The QIT devtools implementation uses WP-CLI to set options directly, but the card testing protection feature relies on filters/hooks from the Dev Tools plugin (option_wcpay_account_data, woocommerce_payments_account_refreshed) that aren't available in the QIT environment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The dev-trunk version of qit-cli requires PHP 7.4+, which conflicts with the config.platform.php setting of 7.3. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
mgascam
force-pushed
the
dev/qit-use-test-packages
branch
from
52eeeb6 to
3597932
Compare
mgascam
changed the base branch from
develop
to
dev/qit-e2e-shopper-remaining-specs
Copilot finished reviewing on behalf of
mgascam
December 3, 2025 19:37
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 49 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (4)
tests/qit/test-package/tests/woopayments/shopper/shopper-multi-currency-widget.spec.ts:25
- The timeout is set within
test.beforeAllwhich only affects the setup hook, not the actual tests. The comment on line 7-8 indicates the intention is to increase the "per-test timeout", but this placement only affects thebeforeAllhook timeout. To set the test timeout for all tests in the suite, movetest.setTimeout(60000)outside thebeforeAllhook, placing it at the suite level (e.g., right after thetest.describecall).
tests/qit/test-package/tests/woopayments/shopper/shopper-wc-blocks-checkout-failures.spec.ts:33 - The error messages use different regex patterns (
/Your card.s expiration year is in the past\./and/Your card.s security code is incomplete\./) which escape the apostrophe with.sinstead of's. This is likely intended to handle both straight apostrophes (') and curly apostrophes ('), but the pattern.swould also match other characters likeas,bs, etc. Consider using a more specific pattern like['']sto only match apostrophe characters, or verify if the literal string matching was working before and if the regex is actually necessary.
tests/qit/test-package/bootstrap/setup.sh:11 - The comment states "Ensure environment is marked as development so dev-only CLI commands are available." but ends with a period before "so", making it grammatically inconsistent with the pattern used in other comments. For consistency, either remove the period or rephrase to use a full sentence structure.
tests/qit/test-package/utils/shopper-navigation.ts:32 - The
ensureAuthAfterNavigationfunction re-authenticates and navigates again if the login form is shown, but there's no protection against infinite loops if the re-authentication itself fails or the navigation continues to show the login form. Consider adding a retry limit or better error handling to detect authentication failures and fail gracefully instead of potentially looping indefinitely.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+23
to
+30
| * @param {string} hostname - The hostname or full URL of the server. | ||
| * @param {string} path - The path of the endpoint. | ||
| * @param {Object} data - The data to send in the POST request. | ||
| * @returns {Promise<Object>} - Resolves with the response object. | ||
| */ | ||
| async function makePostRequest(hostname, path, data) { | ||
| try { | ||
| let finalHost = hostname; |
There was a problem hiding this comment.
[nitpick] The function makePostRequest has a potential issue: when hostname is not provided and getDomain() is called, if getDomain() returns a URL with a protocol (like http://localhost:8080), the logic at lines 36-37 will correctly identify it has a protocol. However, the variable name hostname is misleading since it can actually be a full URL. Consider renaming the parameter to hostOrUrl for clarity, or add a comment explaining that this parameter can be either a hostname or a full URL.
Suggested change
| * @param {string} hostname - The hostname or full URL of the server. | |
| * @param {string} path - The path of the endpoint. | |
| * @param {Object} data - The data to send in the POST request. | |
| * @returns {Promise<Object>} - Resolves with the response object. | |
| */ | |
| async function makePostRequest(hostname, path, data) { | |
| try { | |
| let finalHost = hostname; | |
| * @param {string} hostOrUrl - The hostname or full URL of the server. | |
| * @param {string} path - The path of the endpoint. | |
| * @param {Object} data - The data to send in the POST request. | |
| * @returns {Promise<Object>} - Resolves with the response object. | |
| */ | |
| async function makePostRequest(hostOrUrl, path, data) { | |
| try { | |
| let finalHost = hostOrUrl; |
| if (typeof response.body.status === 'undefined' || typeof response.body.stdout === 'undefined' || typeof response.body.stderr === 'undefined') { | ||
| console.error(command); | ||
| console.error(response); | ||
| throw new Error('Invalid response: "status", "stdout" or "stderr" not found.'); |
There was a problem hiding this comment.
The error message states "Invalid response: 'status', 'stdout' or 'stderr' not found." but uses quotes that could be clearer. Consider using backticks for the field names to improve readability: Invalid response: 'status', 'stdout', or 'stderr' not found. Also, add the Oxford comma before "or" for grammatical correctness.
Suggested change
| throw new Error('Invalid response: "status", "stdout" or "stderr" not found.'); | |
| throw new Error('Invalid response: `status`, `stdout`, or `stderr` not found.'); |
| # Run specific test file | ||
| npm run test:qit-e2e tests/qit/e2e/specs/woopayments/shopper/shopper-checkout-purchase.spec.ts | ||
| # Run specific test file (passthrough to Playwright) | ||
| E2E_JP_SITE_ID='<value>' E2E_JP_BLOG_TOKEN='<value>' E2E_JP_USER_TOKEN='<value>' npm run test:qit-e2e -- -- shopper-checkout-purchase.spec.ts |
There was a problem hiding this comment.
[nitpick] The command shown uses double dashes -- -- which may be confusing. The first -- is npm's argument separator to pass arguments to the underlying script, and the second -- is likely Playwright's separator. Consider adding a brief explanation or simplifying the example to make it clearer for users, such as:
# Run specific test file
npm run test:qit-e2e -- -- shopper-checkout-purchase.spec.ts
# The first -- passes args to npm script, second -- passes to Playwright
Base automatically changed from
dev/qit-e2e-shopper-remaining-specs
to
develop
December 4, 2025 13:09
|
@mgascam Did you add the binary from the root of https://github.com/woocommerce/qit-cli? It seems its still set to use the latest released version which doesn't support test packages yet. |
MrJnrman
reviewed
Dec 5, 2025
| "./bootstrap/setup.sh" | ||
| ], | ||
| "setup": [ | ||
| "npm ci", |
There was a problem hiding this comment.
This throws an error because there's no package lock. We should either generate one or switch to npm install
|
I ended up running it with the binary from trunk and only got 7 failed tests |
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.
Summary
This PR migrates the QIT E2E tests infrastructure to use Test Packages, the recommended approach for QIT. This replaces the previous custom E2E setup and adds comprehensive shopper test coverage.
Key changes:
tests/qit/e2e/,qit.yml,e2e-runner.sh)qit.json,qit-test.json)Test Coverage
Basic Checkout
WC Blocks Checkout
Multi-Currency
My Account
Alternative Payment Methods
Key Improvements
Auth Persistence Fix
Added
ensureAuthAfterNavigationhelper inshopper-navigation.tsthat automatically re-authenticates customers when WooCommerce session expires during long test runs. This prevents flaky test failures due to auth state becoming stale.Test Utilities
Testing
Refer to tests/qit/README.md for setup instructions.
All tests should pass ✅
Context
This is part of the ongoing effort to migrate E2E tests from a custom Docker-based environment to QIT. Previous work established the QIT foundation (#11133, #11146). After QIT released the Test Packages feature, we paused migration to adopt this new approach which offers better isolation and simpler configuration.
🤖 Generated with Claude Code