Skip to content

Conversation

@adamraine
Copy link
Contributor

Follow up to #11539

This script significantly reduces the overhead for running Lighthouse on each URL. The input to this script is a path to a txt file rather than a single URL.

This PR also fixes an error where the content shell would crash on some pages.

@adamraine adamraine requested a review from a team as a code owner December 2, 2020 21:42
@adamraine adamraine requested review from Beytoven and removed request for a team December 2, 2020 21:42
@google-cla google-cla bot added the cla: yes label Dec 2, 2020
Copy link
Collaborator

@connorjclark connorjclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have just one script for n=1 and n=many? As I understand, the only difference would be that for the n=1 case the file would be named devtools-lhr-0.json instead of devtools-lhr.json, which is fine.

http/tests/devtools/lighthouse-run
set -e

OUTPUT_DIR="$LH_ROOT/latest-run/devtools-lhrs"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patrickhulce how do you feel about using this folder for this? Or should we stuff it in .tmp/devtools-lhrs?

Copy link
Collaborator

@connorjclark connorjclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome, thanks for reworking it so much!

echo "Creating test files for URLs"
COUNTER=0
while read url; do
[[ -z $url || ${url:0:1} == "#" ]] && continue
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice idea with the comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was necessary for our example urls.txt

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not gonna lie @connorjclark
image

Also

@adamraine adamraine changed the title misc: add script to run lighthouse on a list of urls from devtools misc: update run-devtools script to use a list of urls Dec 9, 2020
@adamraine adamraine merged commit 3892e6f into master Dec 9, 2020
@adamraine adamraine deleted the batch-test-dt branch December 9, 2020 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants