Use playwright instead of puppeteer #2712

jmbockhorst · 2020-02-10T23:00:05Z

Use playwright instead of puppeteer and adds support for yarn test-api --browser [chromium|firefox|webkit]. Closes #2700.

Requires Node >=10.15.0.

jerch · 2020-02-11T14:51:35Z

@jmbockhorst Very nice 👍

About the failing mouse tests - my guess is that my hack on the puppeteer protocol for the wheel events fail here. I had to do this, because puppeteer does not expose those with enough settings in the official API (like setting the modifier keys).

Tyriar · 2020-02-12T13:07:16Z

package.json

@@ -50,7 +51,7 @@
    "mustache": "^3.0.1",
    "node-pty": "^0.9.0",
    "nyc": "13",
-    "puppeteer": "^1.15.0",
+    "playwright": "^0.10.0",


We need to remote @types/puppeteer, are the types for playwright included in the package?

Yes, the types for playwright are included.

Tyriar · 2020-02-12T13:07:29Z

test/api/TestUtils.ts

@@ -3,10 +3,11 @@
 * @license MIT
 */

-import * as puppeteer from 'puppeteer';
+// import * as playwright from 'playwright';


test/api/TestUtils.ts

Tyriar · 2020-02-12T13:10:22Z

test/api/InputHandler.api.ts

-        it('Pm = 1003, Set Use All Motion (any event) Mouse Tracking', async() => {
-          const coords = await page.evaluate(`
+        it('Pm = 1003, Set Use All Motion (any event) Mouse Tracking', async () => {
+          const coords: any = await page.evaluate(`


{ left: number, top: number, bottom: number, right: number }

Tyriar · 2020-02-12T13:10:58Z

test/api/InputHandler.api.ts

@@ -443,7 +443,7 @@ async function getCursor(): Promise<{col: number, row: number}> {
 }

 async function getDimensions(): Promise<any> {
-  const dim = await page.evaluate(`term._core._renderService.dimensions`);
+  const dim: any = await page.evaluate(`term._core._renderService.dimensions`);


Can we reference IRenderDimensions from here?

Tyriar · 2020-02-12T13:12:12Z

addons/xterm-addon-attach/src/AttachAddon.api.ts

    await page.setViewport({ width, height });
  });

  after(async () => {
    await browser.close();
  });

-  beforeEach(async function(): Promise<any> {
+  beforeEach(async function (): Promise<any> {


I've noticed in some PRs the space goes away and some it gets added. Is this prettier acting up or something? I prefer: async function() { and async () => {

Hehe, this is quite a sensible topic for some ppl. There are discussions about this in linter blueprint / style guide projects, some argue, that the whitespace should stay, since the official form is function SP <name>( and if name is empty the SP would still be there. Well JS allows both, I personally favour the short form w'o the space as well. (But also couldnt care less here, I would be more concerned if someone starts to remove all ; in the code 👿 )

I normally don't mention it but in #2711 the oppposite change happened:

I was going to ask about this. I’m just using VS Code’s default TypeScript formatting settings. Then I would see my formatter would change any entire file just with formatting so I would revert it and try to match the settings. There is some inconsistencies throughout the project so it becomes a mess sometimes and it’s easy to miss things. Maybe we can set some standardized workplace format settings or something?

Sounds good to me #2717

Tyriar · 2020-02-12T13:13:05Z

addons/xterm-addon-web-links/src/WebLinksAddon.api.ts

@@ -3,25 +3,25 @@
 * @license MIT
 */

-import * as puppeteer from 'puppeteer';
+import * as playwright from 'playwright';


It's probably best to change this to import { Page, Browser } on each file to be consistent

Yeah, I can do that after #2711 once I can remove the getBrowserTypes() function from the addon files (which needs the namespace import).

package.json

test/api/InputHandler.api.ts

…ywright

Firefox takes longer to start up

See microsoft/playwright#1032

jmbockhorst · 2020-02-15T12:27:48Z

A new playwright version came out, so upping that might help stuff too.

Tyriar · 2020-02-15T13:03:27Z

@jmbockhorst cool, just been working through all the failures on the other browsers 😃

Tyriar · 2020-02-15T13:12:11Z

Tests take longer due to download again, I think we need to use playwright-core and download the browser on first run instead of PUPPETEER_SKIP_CHROMIUM_DOWNLOAD microsoft/playwright#823 (comment)

https://bugzilla.mozilla.org/show_bug.cgi?id=1372998

Tyriar · 2020-02-15T15:14:43Z

Doesn't look like Windows tests works yet, maybe they'll work after #2714

xtermjs/xterm.js@c5593bd...21b490f Link provider fix - Merge pull request xtermjs/xterm.js#2710 from jmbockhorst/linkFix Leaked reference - Merge pull request xtermjs/xterm.js#2767 from JavaCS3/fix/addDisposableDomListener-leak Missing cursor fix - Merge pull request xtermjs/xterm.js#2731 from jerch/fix_2729 Accessibility fix - Merge pull request xtermjs/xterm.js#2814 from Tyriar/role_list Fixes #93480 Showcase Merge pull request xtermjs/xterm.js#2761 from slel/patch-1 Merge pull request xtermjs/xterm.js#2723 from UziTech/patch-1 Test, doc, build improvements Merge pull request xtermjs/xterm.js#2766 from Tyriar/readme Merge pull request xtermjs/xterm.js#2786 from Tyriar/eslint Merge pull request xtermjs/xterm.js#2799 from Tyriar/enum_like_upper_case Merge pull request xtermjs/xterm.js#2730 from Tyriar/ts38 Merge pull request xtermjs/xterm.js#2753 from jerch/fix_vtfeatures_template Merge pull request xtermjs/xterm.js#2754 from jerch/fix_vtfeatures_2 Merge pull request xtermjs/xterm.js#2722 from Tyriar/linux_tests Merge pull request xtermjs/xterm.js#2712 from jmbockhorst/playwright Merge pull request xtermjs/xterm.js#2800 from Tyriar/upgrade_mac Many dependency updates Merge pull request xtermjs/xterm.js#2758 from Tyriar/acorn Merge pull request xtermjs/xterm.js#2770 from xtermjs/dependabot/npm_and_yarn/deep-eq… Merge pull request xtermjs/xterm.js#2779 from xtermjs/dependabot/npm_and_yarn/webpack… Merge pull request xtermjs/xterm.js#2760 from xtermjs/dependabot/npm_and_yarn/acorn-5… Merge pull request xtermjs/xterm.js#2772 from xtermjs/dependabot/npm_and_yarn/https-p… Merge pull request xtermjs/xterm.js#2783 from Tyriar/deps Merge pull request xtermjs/xterm.js#2795 from xtermjs/dependabot/npm_and_yarn/types/w… Merge pull request xtermjs/xterm.js#2801 from xtermjs/dependabot/npm_and_yarn/types/w… Merge pull request xtermjs/xterm.js#2780 from xtermjs/dependabot/npm_and_yarn/types/w… Merge pull request xtermjs/xterm.js#2792 from xtermjs/dependabot/npm_and_yarn/mocha-7… Merge pull request xtermjs/xterm.js#2803 from Tyriar/more_deps Merge pull request xtermjs/xterm.js#2805 from xtermjs/dependabot/npm_and_yarn/mustach… Merge pull request xtermjs/xterm.js#2810 from xtermjs/dependabot/npm_and_yarn/deep-eq… Merge pull request xtermjs/xterm.js#2815 from xtermjs/dependabot/npm_and_yarn/nyc-15.0.1 Merge pull request xtermjs/xterm.js#2820 from xtermjs/dependabot/npm_and_yarn/typescr… Merge pull request xtermjs/xterm.js#2819 from xtermjs/dependabot/npm_and_yarn/typescr…

xtermjs/xterm.js@c5593bd...21b490f Link provider fix - Merge pull request xtermjs/xterm.js#2710 from jmbockhorst/linkFix Leaked reference - Merge pull request xtermjs/xterm.js#2767 from JavaCS3/fix/addDisposableDomListener-leak Missing cursor fix - Merge pull request xtermjs/xterm.js#2731 from jerch/fix_2729 Accessibility fix - Merge pull request xtermjs/xterm.js#2814 from Tyriar/role_list Fixes microsoft#93480 Showcase Merge pull request xtermjs/xterm.js#2761 from slel/patch-1 Merge pull request xtermjs/xterm.js#2723 from UziTech/patch-1 Test, doc, build improvements Merge pull request xtermjs/xterm.js#2766 from Tyriar/readme Merge pull request xtermjs/xterm.js#2786 from Tyriar/eslint Merge pull request xtermjs/xterm.js#2799 from Tyriar/enum_like_upper_case Merge pull request xtermjs/xterm.js#2730 from Tyriar/ts38 Merge pull request xtermjs/xterm.js#2753 from jerch/fix_vtfeatures_template Merge pull request xtermjs/xterm.js#2754 from jerch/fix_vtfeatures_2 Merge pull request xtermjs/xterm.js#2722 from Tyriar/linux_tests Merge pull request xtermjs/xterm.js#2712 from jmbockhorst/playwright Merge pull request xtermjs/xterm.js#2800 from Tyriar/upgrade_mac Many dependency updates Merge pull request xtermjs/xterm.js#2758 from Tyriar/acorn Merge pull request xtermjs/xterm.js#2770 from xtermjs/dependabot/npm_and_yarn/deep-eq… Merge pull request xtermjs/xterm.js#2779 from xtermjs/dependabot/npm_and_yarn/webpack… Merge pull request xtermjs/xterm.js#2760 from xtermjs/dependabot/npm_and_yarn/acorn-5… Merge pull request xtermjs/xterm.js#2772 from xtermjs/dependabot/npm_and_yarn/https-p… Merge pull request xtermjs/xterm.js#2783 from Tyriar/deps Merge pull request xtermjs/xterm.js#2795 from xtermjs/dependabot/npm_and_yarn/types/w… Merge pull request xtermjs/xterm.js#2801 from xtermjs/dependabot/npm_and_yarn/types/w… Merge pull request xtermjs/xterm.js#2780 from xtermjs/dependabot/npm_and_yarn/types/w… Merge pull request xtermjs/xterm.js#2792 from xtermjs/dependabot/npm_and_yarn/mocha-7… Merge pull request xtermjs/xterm.js#2803 from Tyriar/more_deps Merge pull request xtermjs/xterm.js#2805 from xtermjs/dependabot/npm_and_yarn/mustach… Merge pull request xtermjs/xterm.js#2810 from xtermjs/dependabot/npm_and_yarn/deep-eq… Merge pull request xtermjs/xterm.js#2815 from xtermjs/dependabot/npm_and_yarn/nyc-15.0.1 Merge pull request xtermjs/xterm.js#2820 from xtermjs/dependabot/npm_and_yarn/typescr… Merge pull request xtermjs/xterm.js#2819 from xtermjs/dependabot/npm_and_yarn/typescr…

jmbockhorst added 3 commits February 10, 2020 16:56

Use playwright instead of puppeteer

7515ed6

Update CI to node 10

e7b3910

Fix some mouse tests

5209183

jmbockhorst added 2 commits February 11, 2020 13:13

Fix mouse tests for chromium

8fb6ded

Revert some formatting and remove puppeteer

391313c

Tyriar requested changes Feb 12, 2020

View reviewed changes

Clean up some stuff based on feedback

2442dc0

Tyriar referenced this pull request in microsoft/vscode Feb 12, 2020

@types/debug required by playwright

8daf3f2

jmbockhorst added 3 commits February 12, 2020 23:53

Merge branch 'master' of https://github.com/xtermjs/xterm.js into pla…

258b084

…ywright

Remove duplicate code and clean up after merge

383e2ed

Fix tests

b53c2e1

jmbockhorst marked this pull request as ready for review February 13, 2020 06:44

Tyriar and others added 4 commits February 15, 2020 03:26

Merge branch 'master' into playwright

1d0cceb

Enable tests on other browsers

502452f

Fix firefox tests FitAddon tests

e6e181e

Fix serialize tests on firefox

6082c00

Firefox takes longer to start up

Tyriar force-pushed the playwright branch from 2c11f4f to 6082c00 Compare February 15, 2020 11:58

Only enable webgl tests on chromium and non-headless firefox

93026bc

See microsoft/playwright#1032

Tyriar added 5 commits February 15, 2020 04:29

Movee api test timeout into CLI args

f3eb9f7

Skip paste test when not Chromium

a9356fd

Skip webgl tests instead of not registering

3f15a72

Skip mouse wheel-related tests on non-chromium

7400b88

Fix pipelines file

3b51565

Only start server once in pipelines

2b70802

Tyriar added 2 commits February 15, 2020 05:13

Fix lint

7bd4f38

[email protected]

4ae25e5

Tyriar added 17 commits February 15, 2020 05:18

Adopt playwright breaking changes

4ebb61b

Move to playwright-core, download browser if needed

ffc5e2f

Fix typo in firefox job

03c6b0c

Download browser at start of test

47dc137

Cache yarn modules on integration test jobs

cb1353c

Change browserType back to sync

477b161

Move playwright-core back to dev

2ac0c0a

Update macOS version

ed23194

Up timeout for slower CI agents

cd1b251

Try firefox deps

579a135

https://bugzilla.mozilla.org/show_bug.cgi?id=1372998

Fix syntax error

445a801

Disable firefox on linux

446f8f9

Disable webkit on Linux

1a6ec0d

Add a Windows test job

eb82346

Tweak windows server start

83794c4

script -> pwsh

c36a468

Disable windows tests

c73adf0

Tyriar approved these changes Feb 15, 2020

View reviewed changes

Tyriar added this to the 4.5.0 milestone Feb 15, 2020

Tyriar merged commit 708c6bd into xtermjs:master Feb 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use playwright instead of puppeteer #2712

Use playwright instead of puppeteer #2712

jmbockhorst commented Feb 10, 2020 •

edited

Loading

jerch commented Feb 11, 2020

Tyriar Feb 12, 2020

jmbockhorst Feb 12, 2020

Tyriar Feb 12, 2020

Tyriar Feb 12, 2020

Tyriar Feb 12, 2020

Tyriar Feb 12, 2020

jerch Feb 12, 2020

Tyriar Feb 12, 2020

jmbockhorst Feb 12, 2020

Tyriar Feb 12, 2020

Tyriar Feb 12, 2020

jmbockhorst Feb 12, 2020

jmbockhorst commented Feb 15, 2020

Tyriar commented Feb 15, 2020

Tyriar commented Feb 15, 2020

Tyriar commented Feb 15, 2020

Use playwright instead of puppeteer #2712

Use playwright instead of puppeteer #2712

Conversation

jmbockhorst commented Feb 10, 2020 • edited Loading

jerch commented Feb 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmbockhorst commented Feb 15, 2020

Tyriar commented Feb 15, 2020

Tyriar commented Feb 15, 2020

Tyriar commented Feb 15, 2020

jmbockhorst commented Feb 10, 2020 •

edited

Loading