-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add infrastructure for writing Playwright E2E tests #38570
Conversation
Size Change: +1.57 kB (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
f39fab1
to
565c7b6
Compare
strategy: | ||
fail-fast: false | ||
matrix: | ||
node: ['14'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add matrix here in the future when test suites grow and take advantage of the sharding feature.
@@ -112,6 +113,7 @@ | |||
"@types/eslint": "7.28.0", | |||
"@types/estree": "0.0.50", | |||
"@types/highlight-words-core": "1.2.1", | |||
"@types/istanbul-lib-report": "3.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea why I have to add this to make tsc happy.
}; | ||
|
||
if ( isReducedMotion ) { | ||
handleTransitionEnd(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to fix a bug in one of the existing tests which surfaces itself during the migration. For some reason, transitionend
event below never fires in CI, but I couldn't reproduce it locally. Anyway, it makes sense to skip the event when the user agent prefers reduced motion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be extracted into a separate fix; if it addresses a bug with the current tests we should get it in asap. Also it's best not to mix actual code changes with the test migration 😛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep! In fact, both these two tests (customizing-widgets
and new-post
) are included here only for demonstration purposes, and they could be separated into new PRs along with the fix here. This should help minimize the changes introduced in this PR and make it easier to review.
packages/e2e-test-utils-playwright/src/click-block-toolbar-button.js
Outdated
Show resolved
Hide resolved
timeout: parseInt( process.env.TIMEOUT || '', 10 ) || 100_000, // Defaults to 100 seconds. | ||
// Don't report slow test "files", as we will be running our tests in serial. | ||
reportSlowTests: null, | ||
testDir: new URL( './specs', 'file:' + __filename ).pathname, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code might seem weird, but it makes it easier to swap into ESM (import.meta.url
) in the future if needed.
} ); | ||
} ); | ||
|
||
class WidgetsCustomizerPage { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a Page Object Model we use in this test. We can also move this to e2e-test-utils
if there are needs.
8faf1d1
to
b51dbd6
Compare
packages/e2e-tests-playwright/specs/editor/various/new-post.spec.js
Outdated
Show resolved
Hide resolved
packages/e2e-tests-playwright/specs/editor/various/new-post.spec.js
Outdated
Show resolved
Hide resolved
packages/e2e-tests-playwright/specs/widgets/customizing-widgets.spec.js
Outdated
Show resolved
Hide resolved
packages/e2e-tests-playwright/specs/widgets/customizing-widgets.spec.js
Outdated
Show resolved
Hide resolved
This looks great. Looking forward to working on playwright tests. I was hoping we would have the Playwright package in WP core itself but I think because of internal dependencies of packages it would have not been feasible. |
9851ff3
to
311d9a0
Compare
Not set in stone, feedback welcome.
Follows the previous proposal and the meeting notes.
Prior art: #34089, #38401.
Notable changes
@playwright/test
rather thanjest
+playwright
. The playwright test runner offers more features out of the box specifically for e2e testing. It's also one of the primary focuses of the playwright team (seems like they're now branding playwright as a test framework).e2e-test-utils-playwright
for all the utils used in Playwright. The new tests written in Playwright should be intest/e2e/specs
.sanity.spec.js
just to show that it works as expected. It should be removed once we have other tests migrated.Migration steps
packages/e2e-tests/specs
, rename.test.js
into.spec.js
and put it in the same folder structure insidetest/e2e/specs
.@wordpress/e2e-test-utils-playwright
:describe
,beforeAll
,beforeEach
,afterEach
andafterAll
with thetest.
prefix. For instance,describe
is turning intotest.describe
.page
andbrowser
.e2e-test-utils
, instead use the fixtures API to get thepageUtils
andrequestUtils
directly. (However,pageUtils
is not allowed inbeforeAll
andafterAll
, rewrite them usingrequestUtils
instead.)e2e-test-utils
and paste it in thepage
orrequest
folder ine2e-test-utils-playwright
depending on whether it's a page util or a request util.page
andbrowser
are available inPageUtils
'sthis.page
andthis.browser
. You can get autocomplete and type inference by adding@this {import('./').PageUtils}
to the JSDoc.this
and bound to the same instance. You can remove the internal imports and just usethis
to access them.index.ts
, and put it inside thePageUtils
/RequestUtils
class as an instance field.pageUtils
or therequestUtils
fixture intest/e2e
.Proposed best practices
Forbid
$
, uselocator
insteadIn fact, any API that returns
ElementHandle
is discouraged. This includes$
,$$
,$eval
,$$eval
, etc.Locator
is a much better API and can be used with playwright's assertions. This also works great with Page Object Model since that locator is lazy and doesn't return a promise.Use accessible selectors
This PR introduces a new selector engine role-selector. It enables us to write accessible queries without having to rely on internal implementations. It's an experimental library and could be swapped out if playwright supports it natively instead. The syntax should be straightforward and looks like this:
It can also be chained with built-in selector engines to perform complex queries:
role-selector
under the hood uses@testing-library/dom
to compute the query the elements and compute the accessible attributes.Selectors are strict by default
To encourage better practices for querying elements, selectors are strict by default, meaning that it will throw an error if the query returns more than one element.
Don't overload test-utils, inline simple utils
e2e-test-utils
are too bloated with too many utils. Most of them are simple enough to be inlined directly in tests. With the help of accessible selectors, simple utils are easier to write now. For utils that only take place on a certain page, use Page Object Model instead (with an exception of clearing states withrest
which are better placed ine2e-test-utils
). Otherwise, only create an util if the action is complex and repetitive enough.Favor Page Object Model over utils
As mentioned above, Page Object Model is the preferred way to create reusable utility functions on a certain page. (You can see an inline POM example in #38570 (comment))
The rationale behind using a POM is to group utils under namespaces to be easier to discover and use. In fact,
PageUtils in
e2e-test-utils` package is also a POM, which avoids the need for global variables, and utils can reference each other easily.Restify actions to clear or set states.
It's slow to set states manually before or after tests, especially when they're repeated multiple times between tests. It's recommended to set them via API calls. Use
requestUtils.rest
andrequestUtils.batchRest
instead to call the REST API. We should still add a test for manually setting them, but that should only be tested once.Avoid global variables
page
andbrowser
are not exposed as global variables, this makes it easier to work with when we have multiple pages/tabs in the same test, or if we want to run multiple tests in parallel.@playwright/test
has the concept of fixtures which allows us to injectpage
,browser
, and other parameters into the tests.By default,
@playwright/test
will use differentpage
instances for each test to isolate them. However, the nature of WordPress means most of our tests depend on the server state and we can't easily isolate them. We instead run the tests in serial to prevent race-condition.Make explicit assertions
We can insert as many assertions in one test as needed. It's better to make explicit assertions whenever possible. For instance, if we want to assert that a button exists before clicking on it, we can do
expect( locator ).toBeVisible()
before performinglocator.click()
. This makes the tests flow better and easier to read.Testing Instructions
Types of changes
New feature
Checklist:
*.native.js
files for terms that need renaming or removal).