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

Always run initialization code before Playwright e2e tests #46459

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

kevin940726
Copy link
Member

What?

Discovered in #46452 by @t-hamano. The requestUtils fixture is not initialized if the test doesn't use it.

Why?

We need to run some initialization steps before any e2e tests (switching themes for instance), hence the requestUtils fixture needs to be initialize even when it's not used in tests.

How?

Add { auto: true } to the fixture options.

Testing Instructions

  1. Publish a random post in the test WP instance.
  2. On trunk, run the buttons tests only via npm run test:e2e:playwright -- buttons (it doesn't use requestUtils)
  3. The post should still be there.
  4. Apply this PR, and repeat the same. After the tests, the post should be deleted.

@kevin940726 kevin940726 added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Dec 12, 2022
@kevin940726 kevin940726 requested a review from t-hamano December 12, 2022 08:59
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

Publish a random post in the test WP instance.
On trunk, run the buttons tests only via npm run test:e2e:playwright -- buttons (it doesn't use requestUtils)
The post should still be there.
Apply this PR, and repeat the same. After the tests, the post should be deleted.

I have confirmed that the above steps returns the expected result.
At the same time, I also confirmed that Twenty Twenty One is automatically activated when the test is run.

I noticed that some test files have the following processing. Should these codes be removed?

test.afterEach( async ( { requestUtils } ) => {
	await requestUtils.deleteAllPosts();
} );

Copy link
Contributor

@alexstine alexstine left a comment

Choose a reason for hiding this comment

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

LGTM.

@kevin940726 kevin940726 force-pushed the fix/always-init-request-utils branch from 4b54f42 to fc2f9e9 Compare December 16, 2022 17:11
@kevin940726
Copy link
Member Author

I noticed that some test files have the following processing. Should these codes be removed?

I don't think so. Those are run after each test while the fixture will only run once before running the test suite.

@kevin940726 kevin940726 merged commit a10e7eb into trunk Dec 16, 2022
@kevin940726 kevin940726 deleted the fix/always-init-request-utils branch December 16, 2022 18:03
@github-actions github-actions bot added this to the Gutenberg 14.9 milestone Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants