Skip to content

misc: update run-devtools script to use a list of urls - #11751

Merged
adamraine merged 5 commits into
masterfrom
batch-test-dt
Dec 9, 2020
Merged

misc: update run-devtools script to use a list of urls#11751
adamraine merged 5 commits into
masterfrom
batch-test-dt

Conversation

@adamraine

Copy link
Copy Markdown
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

@connorjclark connorjclark left a comment

Copy link
Copy Markdown
Collaborator

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.

Comment thread lighthouse-core/test/chromium-web-tests/batch-test-pages-devtools.sh Outdated
Comment thread lighthouse-core/test/chromium-web-tests/batch-test-pages-devtools.sh Outdated
http/tests/devtools/lighthouse-run
set -e

OUTPUT_DIR="$LH_ROOT/latest-run/devtools-lhrs"

Copy link
Copy Markdown
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?

Comment thread lighthouse-core/test/chromium-web-tests/batch-test-pages-devtools.sh Outdated
Comment thread lighthouse-core/test/chromium-web-tests/batch-test-pages-devtools.sh Outdated

@connorjclark connorjclark left a comment

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Comment thread lighthouse-core/test/chromium-web-tests/test-page-devtools.sh Outdated
Comment thread lighthouse-core/test/chromium-web-tests/test-page-devtools.sh Outdated
@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