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

Fix e2e tests failed with WC 9.1 #2457

Merged
merged 4 commits into from
Jul 12, 2024
Merged

Conversation

ianlin
Copy link
Member

@ianlin ianlin commented Jul 8, 2024

Changes proposed in this Pull Request:

Closes #2453.

This PR fixes three part of the E2E tests:

The first one is related to woocommerce/woocommerce#47860, where the tabs (<button> elements) on the product edit block has been added role="tab" attribute. So we should rewrite the Playwright query from getByRole( 'button' ) to getByRole( 'tab' ).

The second one is related to woocommerce/woocommerce#48192, where the error messages of the failure notice on the product edit block has been modified from always showing Failed to save product to different messages based on the error type. E.g.

Date error:

Screenshot 2024-07-08 at 17 42 55 .

Multipack error:

Screenshot 2024-07-08 at 17 44 10 Screenshot 2024-07-08 at 17 44 25 Screenshot 2024-07-08 at 17 43 48 .

Also, the behaviour of dismissing the error notice has been changed. Instead of clicking anywhere on the notice to dismiss it, now we need to click a specific X button.

The third one is not related to the compatibility of WC 9.1. It's in the onboarding step 2 when we want to overwrite the default WC country by fulfilling the request /wc-admin/options?options=woocommerce_default_country. Now that there are more options being added into the options query parameter, so we need to rewrite the URL matching pattern from /wc-admin\/options\?options=woocommerce_default_country\b/ to /wc-admin\/options\?options=.*woocommerce_default_country\b/.

Detailed test instructions:

  1. Check out the branch of this PR
  2. Set up E2E test
  3. Upgrade WP to 6.6-RC2
    npm run -- wp-env run tests-cli -- wp core update --version=6.6-RC2
    npm run -- wp-env run tests-cli -- wp core update-db
  4. Upgrade WC to 9.1.0-rc.1
    npm run -- wp-env run tests-cli -- wp plugin update woocommerce --version=9.1.0-rc.1
    npm run -- wp-env run tests-cli -- wp wc update
  5. Run E2E test: npm run test:e2e
  6. See the E2E test passed

Additional details:

Changelog entry

Dev - Fix E2E tests failed with WC 9.1

There were more options added in `options` query parameter, so adding a
`.*` for the URL matching pattern
@ianlin ianlin requested a review from a team July 8, 2024 09:58
@ianlin ianlin self-assigned this Jul 8, 2024
@github-actions github-actions bot added type: bug The issue is a confirmed bug. changelog: fix Took care of something that wasn't working. labels Jul 8, 2024
Copy link

codecov bot commented Jul 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.4%. Comparing base (cb7765e) to head (ddc4c23).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #2457   +/-   ##
=======================================
  Coverage     63.4%   63.4%           
=======================================
  Files          321     321           
  Lines         5027    5027           
  Branches      1219    1219           
=======================================
  Hits          3188    3188           
  Misses        1672    1672           
  Partials       167     167           
Flag Coverage Δ
js-unit-tests 63.4% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@puntope
Copy link
Contributor

puntope commented Jul 9, 2024

Not sure if related but I see this error when running E2E tests in GH https://github.com/woocommerce/google-listings-and-ads/actions/runs/9857791848/job/27217640650#step:10:219

@puntope
Copy link
Contributor

puntope commented Jul 9, 2024

In my local test when I didn't update WP and WC I got this fail:

1 failed [chromium] › product-editor/block-integration.test.js:30:6 › Product Block Editor integration › Prompt to Get Started when not yet finished onboarding

When I did the upgrade I got this one:

1 failed [chromium] › product-editor/block-integration.test.js:567:6 › Product Block Editor integration › Save all product attributes to simple product

@ianlin
Copy link
Member Author

ianlin commented Jul 10, 2024

Thanks for testing @puntope.

In my local test when I didn't update WP and WC I got this fail:
1 failed [chromium] › product-editor/block-integration.test.js:30:6 › Product Block Editor integration › Prompt to Get Started when not yet finished onboarding

I think this is expected, like I said in the description in WC 9.1.0 the tabs (<button> elements) on the product edit block has been added role="tab" attribute. So we should rewrite the Playwright query from getByRole( 'button' ) to getByRole( 'tab' ).

In WC < 9.1 getByRole( 'tab' ) doesn't exist so the test will fail. This PR can be merged after WC 9.1.0 is released.

When I did the upgrade I got this one:
1 failed [chromium] › product-editor/block-integration.test.js:567:6 › Product Block Editor integration › Save all product attributes to simple product

Not sure if related but I see this error when running E2E tests in GH https://github.com/woocommerce/google-listings-and-ads/actions/runs/9857791848/job/27217640650#step:10:219

That's weird as I tried to run e2e again in my local and all the tests passed. I also ran the E2E workflow with this branch and updated WP/WC, all tests passed as well.

https://github.com/woocommerce/google-listings-and-ads/actions/runs/9868881001/job/27251611899#step:10:209

@puntope
Copy link
Contributor

puntope commented Jul 10, 2024

That's weird as I tried to run e2e again in my local and all the tests passed. I also ran the E2E workflow with this branch and updated WP/WC, all tests passed as well.

Seems to be a flaky test:

When I've run. npm run test:e2e product-editor/block-integration.test.js few times like 6

4 were success and 2 not success

 1) [chromium] › product-editor/block-integration.test.js:609:6 › Product Block Editor integration › Save all product attributes to variation product

    Error: expect(received).toHaveLength(expected)

    Expected length: 16
    Received length: 15

await editorUtils.getAvailableProductAttributesWithTestValues();
      616 |
    > 617 | 		expect( pairs ).toHaveLength( 16 );

Maybe a race condition not sure. I will open a new issue

@puntope
Copy link
Contributor

puntope commented Jul 10, 2024

In WC < 9.1 getByRole( 'tab' ) doesn't exist so the test will fail. This PR can be merged after WC 9.1.0 is released.

Then I assume we should wait til WC/WP gets released to release this? Otherwise our E2E will fail

Copy link
Contributor

@puntope puntope left a comment

Choose a reason for hiding this comment

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

✅ Approved (left some advice about releasing this before WP and WC get released)

@ianlin
Copy link
Member Author

ianlin commented Jul 10, 2024

Thanks for testing again @puntope. WP 6.6 doesn't affect those E2E, so I will merge the PR after WC 9.1 is released.

@ianlin
Copy link
Member Author

ianlin commented Jul 12, 2024

WC 9.1 has been released so I'm merging this PR.

@ianlin ianlin merged commit 5069f65 into develop Jul 12, 2024
9 checks passed
@ianlin ianlin deleted the fix/2453-e2e-test-failed-with-wc-9.1 branch July 12, 2024 09:36
@martynmjones martynmjones mentioned this pull request Jul 24, 2024
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

E2E test failed with WC 9.1-rc.1
2 participants