-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
misc: update run-devtools script to use a list of urls #11751
Conversation
There was a problem hiding this 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.
lighthouse-core/test/chromium-web-tests/batch-test-pages-devtools.sh
Outdated
Show resolved
Hide resolved
lighthouse-core/test/chromium-web-tests/batch-test-pages-devtools.sh
Outdated
Show resolved
Hide resolved
http/tests/devtools/lighthouse-run | ||
set -e | ||
|
||
OUTPUT_DIR="$LH_ROOT/latest-run/devtools-lhrs" |
There was a problem hiding this comment.
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
?
lighthouse-core/test/chromium-web-tests/batch-test-pages-devtools.sh
Outdated
Show resolved
Hide resolved
lighthouse-core/test/chromium-web-tests/batch-test-pages-devtools.sh
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
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.