This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Fix e2e tests for price filter #10031
Merged
Conversation
This file contains 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
This commit makes two primary changes: 1. Update the Playwright configuration to capture full-page screenshots on test failures. This change will help in better debugging of failed tests by providing a full context of the application state at the time of the failure. 2. Refactor the end-to-end tests for the price filter feature of the WooCommerce Blocks plugin. The refactoring includes: - Introduction of a new `PriceFilterPage` class which encapsulates the logic for interacting with the price filter within the tests. This change improves the structure of the tests and makes them easier to understand. - The tests have been restructured to use this new class, resulting in a significant reduction in the amount of boilerplate code in the tests. - Removal of unused code and imports from the tests.
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
assets/js/atomic/blocks/product-elements/rating/block.tsx
assets/js/blocks/mini-cart/mini-cart-contents/inner-blocks/mini-cart-title-items-counter-block/block.tsx assets/js/blocks/mini-cart/mini-cart-contents/inner-blocks/mini-cart-title-label-block/block.tsx assets/js/blocks/product-tag/block.js assets/js/blocks/product-tag/edit.tsx |
|
Size Change: 0 B Total Size: 1.17 MB ℹ️ View Unchanged
|
Key changes: 1. Variable `img` was renamed to `firstProductImage` to improve clarity. 2. The method `setPrice` was renamed to `setPriceFilterRange` to better reflect its purpose. 3. Removed the function `urlSearchParamWhenFilterIsApplied` from the `blockData` object, as it was relocated to a private method inside the `PriceFilterPage` class named `generatePriceFilterQueryString`. 4. Replaced hardcoded strings 'woocommerce/filter-wrapper' and 'woocommerce/all-products' with constants WOOCOMMERCE_FILTER_WRAPPER and WOOCOMMERCE_ALL_PRODUCTS, respectively. 5. The `getAllProducts` method in `PriceFilterPage` class was renamed to `locateAllProducts` to reflect the actual operation it's performing. 6. The utility file `utils.ts` was deleted as it was no longer in use. These changes make the test scripts more readable and maintainable, removing unnecessary code and improving method names for better understanding of their functionality.
imanish003
added
skip-changelog
7 tasks
kmanijak
reviewed
Jun 30, 2023
There was a problem hiding this comment.
I reviewed the testing steps and they seem to cover the same as before. This PR really improved readability, so thanks for it!
For some reason, one test is failing for me locally:
1) [blockThemeWithGlobalSideEffects] › price-filter/price-filter.block_theme.side_effects.spec.ts:68:2 › woocommerce/price-filter Block - with PHP classic template › should show all products
Test timeout of 90000ms exceeded.
It stops at:
at price-filter/price-filter.page.ts:68
> 68 | await this.page.waitForLoadState( 'networkidle' );
Based on the output image it seems all good, but the event was not fired, so do you think it could be a race condition? 🤔
One minor thing, I know testing instructions won't be included in the release, but still worth adding a step to re-install deps to update wp-env locally before running the docker and tests.
Comment on lines
163
to
167
| if ( minPrice ) { | ||
| result += `min_price=${ minPrice }`; | ||
| } | ||
| if ( maxPrice ) { | ||
| result += `&max_price=${ maxPrice }`; |
There was a problem hiding this comment.
Detail, but in case there's no min_price query would start with an ampersand (&max_price). I don't know if that may influence the test results, but I'd keep it the closest to the real env possible. Unless you're sure that's ok.
There was a problem hiding this comment.
Based on the output image it seems all good, but the event was not fired, so do you think it could be a race condition? 🤔
I am not sure because all tests are running successfully locally for me. I will try to reproduce the issue by following these steps:
- Destroying the environment:
npm run wp-env destroy - Start the environment again using:
npm run env:start - Once the environment is up and running, re-run the tests using:
npm run test:e2e-pw
I will update you once I have more information.
There was a problem hiding this comment.
Detail, but in case there's no min_price query would start with an ampersand (&max_price). I don't know if that may influence the test results, but I'd keep it the closest to the real env possible. Unless you're sure that's ok.
Agreed, I made the changes in 8c3efcb. Now function uses URLSearchParams to generate the query string. This object automatically encodes special characters and manages the & delimiter.
|
@imanish003, an update to the previous comment. I re-run the tests locally and this time two tests failed: on the exactly same step so seems like it introduces some flakiness 🤔 Do you think that may be related to my local setup somehow? Have you faced the same problem? |
Upgrade `@playwright/test` package from version `1.32.3` to `1.35.1`.
kmanijak
reviewed
Jun 30, 2023
package.json
Outdated
| @@ -109,7 +109,7 @@ | |||
| "@bartekbp/typescript-checkstyle": "5.0.0", | |||
| "@octokit/action": "5.0.2", | |||
| "@octokit/graphql": "5.0.5", | |||
| "@playwright/test": "1.32.3", | |||
| "@playwright/test": "^1.35.1", | |||
There was a problem hiding this comment.
We should stick to the fixed version:
Suggested change
| "@playwright/test": "^1.35.1", | |
| "@playwright/test": "1.35.1", |
There was a problem hiding this comment.
I don't have any strong opinion on this as this is a dev dependency. I will go ahead and change it to fixed version 🤝
There was a problem hiding this comment.
I think that's the convention, but can't find the resource mentioning it, sorry. But I think we can assume that based on other dependencies 😅
There was a problem hiding this comment.
Made the changes in ec0c613
This commit performs a series of changes to the end-to-end tests related to the price filter: 1. Refactored the price filter e2e tests by splitting the `price-filter.block_theme.side_effects.spec.ts` test into two separate tests: `price-filter-with-all-products.block_theme.side_effects.spec.ts` and `price-filter-with-classic-template.block_theme.side_effects.spec.ts`. This provides better organization and clarity for these tests, and enables us to focus more closely on each distinct context in isolation. 2. Deleted the old `price-filter.block_theme.side_effects.spec.ts` test as its responsibilities have been taken over by the new tests. 3. Removed unnecessary `await page.waitForTimeout( 1000 );` lines from the classic-template block theme test and the products block theme test. This reduces the overall time taken to run these tests and eliminates a potential source of flakiness. 4. Made a small adjustment to the `price-filter.page.ts` by removing an unnecessary call to `this.editor.openDocumentSettingsSidebar()` and another unnecessary timeout.
This commit refactors the `getPriceFilterQuery` method in `price-filter.page.ts` to use the built-in URLSearchParams API for constructing URL parameters. Previously, the method was manually creating a string representation of URL parameters. Now, it leverages the URLSearchParams API to improve readability and handle potential edge cases better. The new approach appends 'min_price' and 'max_price' as URL parameters if they are not null. The `toString()` function is then called on the URLSearchParams object to return a string that can be used in a URL.
Here are the key changes: 1. Refactored the way the 'wp action-scheduler run' command is run in the 'global-setup.ts' script. Now, it correctly separates the wp command from the arguments passed to it. 2. Consolidated the 'price-filter-with-all-products.block_theme.side_effects.spec.ts' and 'price-filter-with-classic-template.block_theme.side_effects.spec.ts' tests into a single 'price-filter.block_theme.side_effects.spec.ts' file. The new file contains tests for both scenarios: with all products block and with PHP classic template. Also, updated the product locator function to distinguish between classic templates and products beta block. 3. Deleted the original 'price-filter-with-all-products.block_theme.side_effects.spec.ts' and 'price-filter-with-classic-template.block_theme.side_effects.spec.ts' test files as they are no longer needed. 4. Updated the 'price-filter.page.ts' file to remove the 'waitForLoadState' method call after inserting the block in the editor and added logic to handle locating products in different scenarios (classic template vs products beta block).
Soon products beta block will be replaced by Product Coll...Soon products beta block will be replaced by Product Collection block. Update this accordingly.
woocommerce-blocks/tests/e2e-pw/tests/price-filter/price-filter.page.ts Lines 158 to 169 in de415ce 🚀 This comment was generated by the automations bot based on a
|
|
Hi @kmanijak, After making some changes. I have tested it locally & all tests are passing for me. Can you please review again? As you can see in the demo video below, all tests are passing for me: Screen.Recording.2023-07-04.at.12.01.50.PM.mov |
kmanijak
approved these changes
Jul 4, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Some E2E tests related to the Price Filter block were failing. I have fixed the failing tests and made some other improvements. Here are the highlights of the changes:
Refactored E2E Tests for Price Filter: We have revamped our E2E tests in
price-filter.block_theme.side_effects.spec.ts. A key highlight of this refactoring is the introduction of a new page object model,PriceFilterPage, which consolidates the logic for interacting with the price filter block. This includes adding the price filter block to new posts, setting the price filter range, and locating the images and products on a page.Modified Playwright Configuration: Changes have been made to the Playwright configuration file
playwright.config.tsto capture full-page screenshots only on failure. This helps in better understanding the failures and improves debugging efficiency.Impact of Changes
Areas of Concern
playwright.config.tsto ensure that it aligns with our testing strategy.PriceFilterPageclass should be reviewed for potential improvements in the encapsulation of interactions with the price filter block.Part of #9875
Testing
npm run env:startornpm run env:restartnpm run test:e2e-pw.Playwright Tests / Playwright E2E testsruns without any errors or failures.WooCommerce Visibility