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

Improve stability and coverage of CT (and for other puppeteer reliance) #1273

Closed
samreid opened this issue Jul 6, 2022 · 5 comments
Closed
Assignees

Comments

@samreid
Copy link
Member

samreid commented Jul 6, 2022

Quarterly goal: Investigate Playwright (prototype, feasibility test, bring back to dev); Address memory bug

@samreid
Copy link
Member Author

samreid commented Jul 6, 2022

Here is a star chart comparing puppeteer and playwright:

image

The APIs seem very compatible and I was able to swap out puppeteer for playwright and run phet-io API generation without a problem.

It's also nice that the browsers are downloaded and cached, and not part of node_modules. This will solve one of the big hassles with puppeteer. https://playwright.bootcss.com/python/docs/installation#managing-browser-binaries

~/apache-document-root/main$ cd ~/Library/Caches/ms-playwright
~/Library/Caches/ms-playwright$ ls -la
total 16
drwxr-xr-x    8 samreid  staff   256 Jul  6 10:56 .
drwx------+ 150 samreid  staff  4800 Jul  6 14:57 ..
-rw-r--r--@   1 samreid  staff  6148 Jul  6 10:56 .DS_Store
drwxr-xr-x    4 samreid  staff   128 Jul  5 19:45 .links
drwxr-xr-x    4 samreid  staff   128 Jul  5 19:37 chromium-1012
drwxr-xr-x    5 samreid  staff   160 Jul  5 19:37 ffmpeg-1007
drwxr-xr-x    4 samreid  staff   128 Jul  5 19:37 firefox-1327
drwxr-xr-x   17 samreid  staff   544 Jul  6 10:56 webkit-1668
Index: .gitignore
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.gitignore b/.gitignore
--- a/.gitignore	(revision 22b3962ed4f3f9db6a90d2ebcadb3fade7166d02)
+++ b/.gitignore	(date 1657071952033)
@@ -11,3 +11,6 @@
 build
 dist
 transpile/cache/status.json
+/test-results/
+/playwright-report/
+/playwright/.cache/
Index: package.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/package.json b/package.json
--- a/package.json	(revision 22b3962ed4f3f9db6a90d2ebcadb3fade7166d02)
+++ b/package.json	(date 1657072082811)
@@ -42,6 +42,7 @@
     "node-html-encoder": "^0.0.2",
     "pngjs": "^6.0.0",
     "puppeteer": "~13.5.2",
+    "playwright": "1.23.1",
     "qunit": "^2.16.0",
     "taffydb": "^2.7.3",
     "terser": "^4.6.4",
Index: js/phet-io/generatePhetioMacroAPI.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/phet-io/generatePhetioMacroAPI.js b/js/phet-io/generatePhetioMacroAPI.js
--- a/js/phet-io/generatePhetioMacroAPI.js	(revision 22b3962ed4f3f9db6a90d2ebcadb3fade7166d02)
+++ b/js/phet-io/generatePhetioMacroAPI.js	(date 1657073169723)
@@ -11,7 +11,7 @@
 
 const http = require( 'http' );
 const fs = require( 'fs' );
-const puppeteer = require( 'puppeteer' );
+const { chromium, firefox, webkit } = require( 'playwright' );
 const _ = require( 'lodash' ); // eslint-disable-line
 const assert = require( 'assert' );
 
@@ -67,9 +67,14 @@
   server.listen( 0 );
 
   const port = server.address().port;
-  const browser = await puppeteer.launch( {
-    timeout: 120000
+
+  // Or 'firefox' or 'webkit'.
+  const browser = await chromium.launch( {
+    timeout: 12000
   } );
+  // const browser = await puppeteer.launch( {
+  //   timeout: 120000
+  // } );
   const chunks = _.chunk( repos, options.chunkSize );
 
   const macroAPI = {};
Index: test/test-playwright.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/test/test-playwright.js b/test/test-playwright.js
new file mode 100644
--- /dev/null	(date 1657072245067)
+++ b/test/test-playwright.js	(date 1657072245067)
@@ -0,0 +1,9 @@
+const { chromium, firefox, webkit } = require( 'playwright' );
+
+( async () => {
+  const browser = await chromium.launch();  // Or 'firefox' or 'webkit'.
+  const page = await browser.newPage();
+  await page.goto( 'http://example.com' );
+  await page.screenshot( { path: 'screenshot.png' } );
+  await browser.close();
+} )();
\ No newline at end of file

@samreid
Copy link
Member Author

samreid commented Jul 7, 2022

See related issue phetsims/aqua#150

@samreid
Copy link
Member Author

samreid commented Jul 7, 2022

There was a severe logic bug in phetsims/aqua#150. I pushed a fix that may help a lot in this area.

@samreid
Copy link
Member Author

samreid commented Jul 10, 2022

While investigating deno, I observed that puppeteer has a deno port but playwright does not yet have one. Also, deno would address one of the shortcomings in puppeteer about eliminating the need for npm update and caching puppeteer versions.

The fix in phetsims/aqua#150 (comment) seems to have corrected the puppeteer problem.

Here are the remaining reasons to investigate playwright:

@samreid
Copy link
Member Author

samreid commented Aug 8, 2022

The quarterly goal of improving stability was accomplished through a bugfix in phetsims/aqua#150 (comment). The remaining work is lower priority and I have moved it to separate issues (currently unassigned), which can be investigated at our discretion. Closing.

@samreid samreid closed this as completed Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant