[Experiment] Try Playwright #34089
Closed
[Experiment] Try Playwright #34089
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WunderBart
force-pushed
the
try/playwright
branch
2 times, most recently
from
ff6604b to
b649299
Compare
gziolo
added
[Type] Automated Testing
|
The interesting part is that in this PR some of the issues with existing tests are exposed. I'm looking forward to seeing more details about the benefits of moving to Playwright. Thank you for investing your time investigating steps necessary to use Playwright instead of Puppeteer. To get us there we will have to update also |
Thanks for the heads up! I was aware of the |
WunderBart
force-pushed
the
try/playwright
branch
2 times, most recently
from
b973ea6 to
b8d0566
Compare
|
Size Change: 0 B Total Size: 1.06 MB ℹ️ View Unchanged
|
WunderBart
force-pushed
the
try/playwright
branch
4 times, most recently
from
2000d6d to
444a3d7
Compare
|
As far as I can tell CI runs tests with Puppeteer:
|
I think it just uses Puppeteer's Chromium. It would break if Puppeteer was used due to the migrated API. You can check that locally by running |
OK, I think you are correct. We have this logic that installs Chromium when the tests are run for the first time rather than during |
WunderBart
requested review from
youknowriad,
gziolo,
ockham and
fullofcaffeine
|
One of the tests started failing after the last rebase with trunk. Will take a look tomorrow. |
kevin940726
reviewed
Aug 18, 2021
| @@ -0,0 +1,27 @@ | |||
| module.exports = { | |||
| preset: 'jest-playwright-preset', | |||
There was a problem hiding this comment.
I think we'll lose the failure artifacts feature if we just use the default preset.
kevin940726
reviewed
Aug 19, 2021
- Type in directly when searching for block. - Wait for the block to be stable when focused as the layout shifts when the Inserter closes. - Remove unnecessary waitForInserterCloseAndContentFocus call because Playwright's auto-waiting does that automagically.
- Remove explicit `waits` and remove the unnecessary scrollIntoView call (all thanks to auto-waiting)
Doesn't really seem like we need it anymore?
(Removed accidentally)
It's standing in the way of the Replace button because the test image is really small.
WunderBart
force-pushed
the
try/playwright
branch
from
8e63353 to
3cdb35f
Compare
|
Regarding some concerns about how testing with CSS animations would impact the speed, here's some data I've collected from running the migrated specs:
It's hard to say how much more time will animations add when all the tests are run (124 suites in total). Basing on the numbers above, we can probably expect at least a few minutes. I'd say the trade-off would still be worth it but we can always decide later, of course. Just thought it would be interesting to see the actual numbers from this POC :) |
|
I'm gathering some more feedback on this proposal via the Make Test blog: https://make.wordpress.org/test/2021/10/04/proposal-migrate-e2e-to-playwright/ |
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Howdy! 👋
I'd like to make a case for migrating our E2E tests to Playwright. I figured the best way to do that would be to list its advantages and do a bit of refactoring so that we can actually see those advantages in action. This PR is not meant to be merged! The Playwright implementation here is very basic and just a subset of tests has been refactored for experiment/demo purposes, although all those refactors would be valid if cherry-picked into proper PR(s). So, without further ado...
Why should we drop Puppeteer in favor of Playwright?
In general, because it's easier to write stable tests with it. There are a few reasons for that, so let me start with
The API
It's almost identical. If you're used to Puppeteer you'll have no trouble switching to Playwright. <It's worth noting here that it would also mean a low-cost refactor!> . The high syntax similarity itself doesn't mean it will get easier though, so let's move to
The Auto-Waiting Mechanism
I think this is the
#1reason for the stability improvement over Puppeteer. From the documentation:For example, the
page.click( selector )method waits for the target element to be attached, visible, stable, receive events, and enabled before clicking the actual element. In practice, that means:No need to perform any additional presence checks, e.g.
Check out the demo refactor commits (i.e. 70eba4e) for more practical examples.
No need to disable CSS animation or enforce the reduced motion.
This is thanks to the stability check, which makes sure the element has a bounding box and is not moving before the action is performed. I think this is a big one because it would allow us to fully test the application, including the CSS animations. For the purpose of demonstrating that, I've deactivated the
gutenberg-test-plugin-disables-the-css-animationsplugin and removed the enforced reduced motion in this PR. At this point, you should be seeing passing Playwright tests with animations enabled in the Checks tab.And finally...
Less code / Faster scripting.
The Advanced Selectors Support
I think this one's the
#1reason for making the tests easier to write and follow. From my experience, one of the most important good practices for writing resilient E2E tests is prioritizing the user-facing attributes as they change less often than CSS classes. There isn't a more user-facing attribute than the element's inner text, and that's why the wide range of supported text selectors, together with layout-based selectors in Playwright win the game for me. For example, this is how we can fill the media URL in the Image block by combining text and layout-based selectors:...while with Puppeteer, we need to rely on CSS selectors or create complicated XPath selectors. For example, this is how we're currently selecting an active Inserter element:
`//*[contains(@class, "components-autocomplete__result") and contains(@class, "is-selected") and contains(text(), 'List')]`With Playwright it would simply be:
'.components-autocomplete__results.is-selected:text-is( "List" )'Not only it encourages good practices but it also makes the element selection more straightforward and easier to follow. Big win, if you ask me.
There's one observation I wanted to mention here. I'm not sure why it was decided to use snapshots as an expected output comparison, but I suppose that it's mostly because of how convenient they are in this regard. I think though that there's one disadvantage in how we're using them that makes this method a bit unreliable — there's no waiting for the UI update before the edited post content is taken for snapshot comparison. The most erratic suite that demonstrated this issue for me was the List block one. The UI was not keeping up, at least not in a consistent manner, and the tests were failing intermittently. Among other corrections, it needed a bit of time before the UI updates in order to make a snapshot comparison. That time can be easily applied by using the
waitForSelectormethod that would wait for the expected output. The reason why I'm mentioning it here is that with Playwright it's much easier to verify the produced content using the aforementioned selectors. If you check out the List block demo refactor, you'll see that the snapshot comparisons aren't really needed at all when we verify the content using simple text matchers. Unless of course, we do want to compare the full markup, which I'm not sure is a good idea since it can contain a lot of unrelated information, doesn't say much inside the test itself, and can be difficult to follow. Snapshots comparisons can be useful of course, e.g. for strict comparison against multiline content, but IMO it should almost always be done after the waiting for the expected UI has been done. Just a good practice in my opinion. What do you think?Let's move on to the next point.
The Browser Support
This is a short one - Playwright supports all of them. 😄
The Debugging
I think that the Playwright's built-in Inspector deserves a mention here. It's quite intuitive and has some neat features like stepping through the script or recording the actions to a script (codegen). I think it's great to have that kind of functionality OOTB.
Screen.Recording.2021-08-17.at.19.41.11.mov
You can check it out yourself by running
PWDEBUG=1 npm run test-e2eon this branch.The Documentation
Also something worth mentioning in my opinion, as it's easy to navigate and really well written. Please take a few minutes and check it out to see for yourself: https://playwright.dev/docs/intro. Maybe you'll find even more reasons to switch to Playwright. 🤷
What can I do with this branch?
You can run some tests locally in a few different ways. Before you do, make sure your wp-env is up and running and then:
See https://playwright.dev/docs/debug#run-in-debug-mode for more information on Playwright debugging.
I know it got quite lengthy, but I really wanted to make a good case for Playwright, and I hope I did. Writing good E2E tests is often a struggle, and I think that if there's a chance of improving that, especially with such a low cost, then it should be done. I would be happy to work on this task if we decide to move it forward. 🙌