misc: puppeteer script to test a page from devtools #12145

adamraine · 2021-02-26T22:42:02Z

Add a new puppeteer script to run LH from DT inspector. This script is better than the old web test one for a few reasons:

Significantly faster
Significantly less hacky (although still a little hacky)
Doesn't create any temporary files
Runs on linux, so we can run it in GCP

Still TODO:

Run on a list of URLs.
Output file(s).

Old script:
#11539
#11751

chrystianrgs

test>next<import @devtools-bot

patrickhulce

very clever, I like it! :D

patrickhulce · 2021-02-27T04:29:03Z

lighthouse-core/scripts/pptr-run-devtools.js

+  const page = await browser.newPage();
+
+  // Cut off JS for initial page navigation.
+  // This step is unnecessary for every page I tried except https://cnn.com.


What happens when you allow JS? This is rather surprising to me.

LH never starts because cnn takes too long to load. I believe the page's scripts are hogging the main thread so much that startLighthouse never runs successfully.

Every other page I tested didn't need this step, but disabling JS still decreased the startup time for those pages.

Yikes. I don't love doing this because if we can't run Lighthouse because a page is so busy, it might be indicative of our users not being able to run Lighthouse because a page is too busy.

Finding the "Lighthouse doesn't work in scenario X" is actually the point of creating a DevTools repro script, so perhaps we've already found our first bug :)

Good point, I took another look and it turns out I was overthinking the problem. The script just blocks on await page.goto(...) and never gets to the part where it tries to start LH.

Instead of disabling javascript to get the page to load faster, I just took out the await on page.goto(...).

Update: Trying to start LH immediately after navigation starts sometimes causes DT to hang on "Lighthouse is warming up..." forever.

Changed this to wait for DOMContentLoaded before trying to start LH.

lighthouse-core/scripts/pptr-run-devtools.js

patrickhulce · 2021-02-27T04:33:59Z

lighthouse-core/scripts/pptr-run-devtools.js

+
+  fs.writeFileSync('latest-run/lhr.json', remoteLhrResponse.result.value);
+
+  await session.send('Runtime.disable');


does this have a particular effect you noticed? fine to leave in, I just wouldn't expect it to do anything if we're shutting down the entire browser immediately after :)

No effect that I noticed.

FWIW I plan to update this script to test multiple urls. Script would open a new page for each test and close the page after, but browser remains open the entire time. Do you think this line would have an effect then?

If you're still closing the page I wouldn't expect it to have an effect. I suppose it's slightly more plausible there's some memory leak in Chromium browser process no one has discovered before, but still seems unlikely to affect us.

package.json

lighthouse-core/scripts/pptr-run-devtools.js

connorjclark · 2021-03-03T19:18:54Z

lighthouse-core/scripts/pptr-run-devtools.js

+    const pageSession = await page.target().createCDPSession();
+    await pageSession.send('Page.enable');
+    pageSession.once('Page.domContentEventFired', resolve);
+    page.goto(process.argv[2]).catch(err => err);


Can you process args at the top of the file, and leave a comment (like in the other file) on how to run this?

Added usage comment, wound up removing the usage of cli args in favor of stdin.

lighthouse-core/scripts/pptr-run-devtools.js

k

brendankenny · 2021-03-22T21:54:35Z

It might be a bit annoying to hook this up to an existing Chrome session with remote debugging enabled. Could we add an extra parameter to this script to launch Chrome with a specific DT inspector build?

Yeah, that seems workable. We can think about it at some point.

brendankenny · 2021-03-22T22:21:14Z

lighthouse-core/scripts/pptr-run-devtools.js

+
+const startLighthouse = `
+(() => {
+  UI.ViewManager.instance().showView('lighthouse');


is this working for you locally still? When I run it it seems like this line isn't succeeding (it's stuck on the elements panel)

It's working fine for me on the latest Canary, but I was having issues earlier with a custom DT frontend.

Can you try opening DT on DT and running this line manually?

Maybe we need to add an extra ViewManager?

https://source.chromium.org/chromium/chromium/src/+/master:third_party/devtools-frontend/src/front_end/test_runner/TestRunner.js;l=264-270?q=TestRunner.js&ss=chromium%2Fchromium%2Fsrc

Can you do const ViewManager = UI.ViewManager.ViewManager || UI.ViewManager; ... so it works in stable and canary?

Update: The issue here was that CHROME_PATH wasn't set, so puppeteer defaulted to M78 which uses the "Audits" panel instead of the "Lighthouse" panel.

I've upgraded our puppeteer dep: #12284

patrickhulce

bulk of this seems as good as I remember! :)

patrickhulce · 2021-04-12T19:12:03Z

lighthouse-core/scripts/pptr-run-devtools.js

+  .option('o', {
+    type: 'string',
+    default: 'latest-run/devtools-lhrs',
+  }).alias('o', 'output-dir')


not a big deal, but I find it to be more readable for non-authors when the option is the typical JS camelCase version and the alias is the letter. You still get the same support for -o, --output-dir and --outputDir but it's much clearer in the JS when you use argv.outputDir and it matches the explicit option :)

I'm a little conflicted here.

I think you're right that the readability is better, but I would also prefer to keep --custom-devtools-frontend in kebab case because it matches the corresponding flag used on the chrome instance. It's probably better to keep the case for these two flags the same.

I think I will leave this as as for now.

that's fine. fwiw I'd still take argv['custom-devtools-frontend'] over argv.d 😉

lighthouse-core/scripts/pptr-run-devtools.js

adamraine added 7 commits February 22, 2021 16:15

start

a7c9b7c

better

2bfab03

arg

5600580

rename

f1f482c

package

f55af1a

fix

2610184

shift

eb46ce9

google-cla bot added the cla: yes label Feb 26, 2021

adamraine marked this pull request as ready for review February 26, 2021 22:42

adamraine requested a review from a team as a code owner February 26, 2021 22:42

adamraine requested review from patrickhulce and removed request for a team February 26, 2021 22:42

devtools-bot assigned patrickhulce Feb 26, 2021

devtools-bot added the waiting4reviewer label Feb 26, 2021

chrystianrgs approved these changes Feb 27, 2021

View reviewed changes

patrickhulce reviewed Feb 27, 2021

View reviewed changes

patrickhulce added waiting4committer and removed waiting4reviewer labels Feb 27, 2021

adamraine added 4 commits March 1, 2021 11:52

fix

c02565e

review

64db3dd

rm old

031aff0

license

92bf401

vercel bot deployed to Preview March 1, 2021 17:22 View deployment

wait

327aad7

vercel bot deployed to Preview March 1, 2021 17:57 View deployment

connorjclark previously requested changes Mar 3, 2021

View reviewed changes

adamraine added 4 commits March 5, 2021 14:41

url list

e388a0b

usage comment

f9cb0ab

organize

9362eb2

l

e789cf6

vercel bot deployed to Preview March 19, 2021 23:54 View deployment

brendankenny reviewed Mar 20, 2021

View reviewed changes

lighthouse-core/scripts/pptr-run-devtools.js Outdated Show resolved Hide resolved

change

44ef3c2

vercel bot deployed to Preview March 22, 2021 17:33 View deployment

brendankenny reviewed Mar 22, 2021

View reviewed changes

update usage

eff36a1

vercel bot deployed to Preview March 22, 2021 23:21 View deployment

connorjclark mentioned this pull request Mar 23, 2021

deps(puppeteer): upgrade to 9.1.1 #12284

Merged

adamraine added 3 commits March 23, 2021 11:21

Merge branch 'master' into new-run-dt-script

e9cbda1

args

ee3aad5

correction

f1789cc

vercel bot deployed to Preview March 30, 2021 20:16 View deployment

brendankenny mentioned this pull request Apr 9, 2021

cli(asset-saver): print one devtoolsLog event per line #12348

Merged

patrickhulce approved these changes Apr 12, 2021

View reviewed changes

patrickhulce added waiting4committer and removed waiting4reviewer labels Apr 12, 2021

adamraine added 2 commits April 12, 2021 16:48

Merge branch 'master' into new-run-dt-script

cc269a0

review

c01ce58

vercel bot deployed to Preview April 12, 2021 22:08 View deployment

lint

92a3b56

vercel bot deployed to Preview April 13, 2021 17:44 View deployment

adamraine added land-when-ci-is-green and removed waiting4committer labels Apr 13, 2021

Merge branch 'master' into new-run-dt-script

ff8ba75

vercel bot deployed to Preview April 13, 2021 20:29 View deployment

devtools-bot merged commit db76ab3 into master Apr 13, 2021

devtools-bot deleted the new-run-dt-script branch April 13, 2021 20:51

devtools-bot removed the land-when-ci-is-green label Apr 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

misc: puppeteer script to test a page from devtools #12145

misc: puppeteer script to test a page from devtools #12145

adamraine commented Feb 26, 2021 •

edited

Loading

chrystianrgs left a comment

patrickhulce left a comment

patrickhulce Feb 27, 2021

adamraine Feb 28, 2021

patrickhulce Feb 28, 2021

adamraine Mar 1, 2021

adamraine Mar 1, 2021

patrickhulce Feb 27, 2021

adamraine Feb 28, 2021

patrickhulce Feb 28, 2021

connorjclark Mar 3, 2021

adamraine Mar 5, 2021

brendankenny commented Mar 22, 2021

brendankenny Mar 22, 2021

adamraine Mar 22, 2021

adamraine Mar 22, 2021

connorjclark Mar 22, 2021

adamraine Mar 22, 2021

connorjclark Mar 23, 2021

patrickhulce left a comment

patrickhulce Apr 12, 2021

adamraine Apr 12, 2021

patrickhulce Apr 12, 2021


		fs.writeFileSync('latest-run/lhr.json', remoteLhrResponse.result.value);

		await session.send('Runtime.disable');

misc: puppeteer script to test a page from devtools #12145

misc: puppeteer script to test a page from devtools #12145

Conversation

adamraine commented Feb 26, 2021 • edited Loading

chrystianrgs left a comment

Choose a reason for hiding this comment

patrickhulce left a 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

brendankenny commented Mar 22, 2021

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

patrickhulce left a 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

adamraine commented Feb 26, 2021 •

edited

Loading