Skip to content
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

[Blocks] Migrate rating filter to playwright #46583

Merged
merged 47 commits into from
Apr 26, 2024
Merged

Conversation

gigitux
Copy link
Contributor

@gigitux gigitux commented Apr 15, 2024

Warning

This PR is blocked by #46125

Submission Review Guidelines:

Changes proposed in this Pull Request:

This PR migrates the Rating Filter tests to the Playwright infrastructure.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. Ensure that E2E tests are green.

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Comment

Migrate Rating filter tests to Playwright

WunderBart and others added 29 commits April 2, 2024 15:43
That part is overriding the logged in user state and wipes out the nonce and rootUrl fields.
* Load local pickup enabled setting as bool not string

* Add changelog

* Remove unused import

* Update plugins/woocommerce-blocks/assets/js/extensions/shipping-methods/pickup-location/utils.ts

Co-authored-by: Niels Lange <info@nielslange.de>

* Use strict equality test

* try now

* try now

* add a new sql file

* remove default.sql

* fix welcome guide tour

* fix E2E tests

* Fix the template revert tests
...where the template is unreachable due to pagination.

* Add changelog entry

* improve logic to closeWelcomeGuideModal

* fix cart checkout tests

* improve flakiness

* improve flakiness

* fix flaky test

* fix company field

* fix E2E test

* fix E2E tests

---------

Co-authored-by: Thomas Roberts <thomas.roberts@automattic.com>
Co-authored-by: Thomas Roberts <5656702+opr@users.noreply.github.com>
Co-authored-by: Niels Lange <info@nielslange.de>
Co-authored-by: Bart Kalisz <bartlomiej.kalisz@gmail.com>
@gigitux gigitux added the focus: e2e tests Issues related to e2e tests label Apr 15, 2024
@gigitux gigitux force-pushed the try/filter-product-by-rating branch from 5b8ac55 to 3a583aa Compare April 15, 2024 17:03
@gigitux gigitux force-pushed the try/filter-product-by-rating branch from 3a583aa to ffac689 Compare April 16, 2024 07:20
Copy link
Contributor

@WunderBart WunderBart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments; otherwise, it looks good! Thanks!

Comment on lines 256 to 261
await page.addInitScript( () => {
document.addEventListener( 'DOMContentLoaded', () => {
// eslint-disable-next-line dot-notation
window[ '__DOMContentLoaded__' ] = true;
} );
} );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this?

Copy link
Contributor Author

@gigitux gigitux Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to ensure that the page is refreshed when the user clicks on the Apply button, so add an eventListener that sets a value at the DOMContentLoaded event.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we base the test on any UI change or the URL instead? There has to be some change since we're reloading the page, no?

Comment on lines 268 to 273
await page.waitForEvent( 'domcontentloaded' );

const domContentLoaded = await page.evaluate(
// eslint-disable-next-line dot-notation
() => window[ '__DOMContentLoaded__' ] === true
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part looks like it might not be necessary. Can we make sure we absolutely need this and why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, I check that the DOMContentLoaded event has been triggered.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to check it, though? Do we not trust Playwright? 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good point! page.waitForEvent( 'dom content loaded' ) is enough! I will update all the E2E tests! Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need the page.waitForEvent( 'domcontentloaded' ). By default, navigation changes in Playwright wait for the load event to kick in, which is used to detect a fully loaded page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I want to ensure that the "navigation" is triggered. How can I do it?

gigitux and others added 3 commits April 16, 2024 11:00
Co-authored-by: Bart Kalisz <bartlomiej.kalisz@gmail.com>
…/woocommerce into try/filter-product-by-rating
* Fix ESLint errors

* Add changefile(s) from automation for the following project(s): woocommerce-blocks

---------

Co-authored-by: github-actions <github-actions@github.com>
@gigitux gigitux force-pushed the try/filter-product-by-rating branch from 27d762c to eace94d Compare April 16, 2024 13:06
@gigitux gigitux marked this pull request as ready for review April 16, 2024 13:22
@woocommercebot woocommercebot requested a review from a team April 16, 2024 13:28
Copy link
Contributor

@WunderBart WunderBart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@opr opr removed their request for review April 17, 2024 18:29
Base automatically changed from try/e2e-db-reset to trunk April 26, 2024 09:39
@gigitux gigitux merged commit 24c2832 into trunk Apr 26, 2024
34 checks passed
@gigitux gigitux deleted the try/filter-product-by-rating branch April 26, 2024 10:53
@github-actions github-actions bot added this to the 9.0.0 milestone Apr 26, 2024
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Apr 26, 2024
samueljseay pushed a commit that referenced this pull request Apr 29, 2024
* try db reset in page teardown

* move reset to setup step

* Use wp db cli

* Fix global setup
That part is overriding the logged in user state and wipes out the nonce and rootUrl fields.

* Try importing instead from a generated dump

* Revert "Try importing instead from a generated dump"

This reverts commit 987dc47.

* Revert "Revert "Try importing instead from a generated dump""

This reverts commit c8d008c.

* Don't bypass visitSiteEditor so that the Welcome Guide is closed

* use createNewPost

* Revert "Revert "Revert "Try importing instead from a generated dump"""

This reverts commit 2684273.

* [Blocks]: Fix E2E tests (#46344)

* Load local pickup enabled setting as bool not string

* Add changelog

* Remove unused import

* Update plugins/woocommerce-blocks/assets/js/extensions/shipping-methods/pickup-location/utils.ts

Co-authored-by: Niels Lange <info@nielslange.de>

* Use strict equality test

* try now

* try now

* add a new sql file

* remove default.sql

* fix welcome guide tour

* fix E2E tests

* Fix the template revert tests
...where the template is unreachable due to pagination.

* Add changelog entry

* improve logic to closeWelcomeGuideModal

* fix cart checkout tests

* improve flakiness

* improve flakiness

* fix flaky test

* fix company field

* fix E2E test

* fix E2E tests

---------

Co-authored-by: Thomas Roberts <thomas.roberts@automattic.com>
Co-authored-by: Thomas Roberts <5656702+opr@users.noreply.github.com>
Co-authored-by: Niels Lange <info@nielslange.de>
Co-authored-by: Bart Kalisz <bartlomiej.kalisz@gmail.com>

* Replace beforeAll w/ beforeEach + remove all after* hooks

* Fix broken hooks

* Activate plugins via requestUtils API

* [Blocks - E2E]: Add `playwright/no-hooks` ESlint rule (#46432)

add ESLint configuration

* Clean up console logs

* Remove obsolete language setup steps

* Remove more unnecessary setup steps

* Remove even more obsolete setup steps

* tmp: add the LYS fix

* Try stabilizing the company field test

* Use canvas param instead of manually entering edit mode

* Remove double site editor redirect

* Blocks: Migrate rating filter to playwright

* Add changefile(s) from automation for the following project(s): woocommerce-blocks, woocommerce

* remove not necessary changelog

* Revert "Use canvas param instead of manually entering edit mode"

This reverts commit 5e6cc17.

* Revert "Remove double site editor redirect"

This reverts commit 69a57a8.

* Fix flaky products sorting test

* Blocks: Fix ESLint errors (#46595)

fix eslint error

* improve tests

* use visitSiteEditor util

Co-authored-by: Bart Kalisz <bartlomiej.kalisz@gmail.com>

* use locator.fill

* Fix ESLint errors (#46626)

* Fix ESLint errors

* Add changefile(s) from automation for the following project(s): woocommerce-blocks

---------

Co-authored-by: github-actions <github-actions@github.com>

* address feedback

* remove not necessary changelog

---------

Co-authored-by: Bart Kalisz <bartlomiej.kalisz@gmail.com>
Co-authored-by: Thomas Roberts <thomas.roberts@automattic.com>
Co-authored-by: Thomas Roberts <5656702+opr@users.noreply.github.com>
Co-authored-by: Niels Lange <info@nielslange.de>
Co-authored-by: github-actions <github-actions@github.com>
@rodelgc rodelgc added status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: blocks plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris team: Kirigami & Origami WC Store Editing (FSE)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants