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

Strict mode violation in saveSiteEditorEntities when using @wordpress/e2e-test-utils #69042

Open
4 of 17 tasks
nielslange opened this issue Feb 5, 2025 · 1 comment
Open
4 of 17 tasks
Labels
[Tool] E2E Test Utils /packages/e2e-test-utils [Type] Bug An existing feature does not function as intended

Comments

@nielslange
Copy link
Contributor

Description

The saveSiteEditorEntities function in @wordpress/e2e-test-utils-playwright is causing flaky tests due to a strict mode violation when multiple snackbar notifications appear simultaneously. This affects multiple test suites in the WooCommerce repository and beyond.

Affected test suites

WooCommerce

Gutenberg

Example error message from #49353:

Error: locator.waitFor: Error: strict mode violation: getByRole('button', { name: 'Dismiss this notice' }).getByText(/(updated|published)\./) resolved to 2 elements:
    1) <div class="components-snackbar__content"></div> aka getByTestId('snackbar').first()
    2) <div class="components-snackbar__content">Template updated.</div> aka getByTestId('snackbar').nth(1)

Call log:
  - waiting for getByRole('button', { name: 'Dismiss this notice' }).getByText(/(updated|published)\./) to be visible

    at Editor.saveSiteEditorEntities (/home/runner/work/woocommerce/woocommerce/node_modules/.pnpm/@wordpress+e2e-test-utils-playwright@1.16.0_@playwright+test@1.49.1/node_modules/@wordpress/e2e-test-utils-playwright/src/editor/site-editor.ts:53:4)
    at /home/runner/work/woocommerce/woocommerce/plugins/woocommerce-blocks/tests/e2e/tests/checkout/checkout-block.merchant.block_theme.spec.ts:176:3

The strict mode violation originates from

await this.page
.getByRole( 'button', { name: 'Dismiss this notice' } )
.getByText( /(updated|published)\./ )
.waitFor();
}

Proposed solution

To ensure consistent behaviour when multiple notifications are present, explicitly select the first matching notification:

await this.page
    .getByRole('button', { name: 'Dismiss this notice' })
    .getByText(/(updated|published)\./)
    .first()
    .waitFor();

Benefits of this proposed solution

  • Resolves the strict mode violation.
  • Maintains the original functionality.

Step-by-step reproduction instructions

See the error messages of the failing e2e tests in

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes

Please confirm which theme type you used for testing.

  • Block
  • Classic
  • Hybrid (e.g. classic with theme.json)
  • Not sure
@nielslange nielslange added the [Type] Bug An existing feature does not function as intended label Feb 5, 2025
@Mamaduka Mamaduka added the [Tool] E2E Test Utils /packages/e2e-test-utils label Feb 5, 2025
@t-hamano
Copy link
Contributor

t-hamano commented Feb 5, 2025

I confirmed that multiple update snackbars sometimes appear. For example, when quickly saving site settings and templates:

Image

I can think of three approaches.


1. Click on the matching snackbar to remove it

This will prevent multiple update snackbars from appearing. However,

  • Users who already use this utility may have added their own process to remove this snackbar. In that case, those assertions may fail when you update the library.
  • We may also need to revise similar utilities to improve consistency. For example, publishPost and savePost.

2. Unify snackbar IDs

These snackbars are displayed by createSuccessNotice. If we unify the id parameter of the update-related notification, multiple update snackbars should not appear.

3. Solve it on the test code side

For example:

  • Split up the tests that perform multiple updates and refactor them so that only one update snackbar is always displayed
  • Add explicit snackbar click processing (Example)

Personally, I prefer the second or third approach, but I'd like to hear other people's opinions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Tool] E2E Test Utils /packages/e2e-test-utils [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants