-
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: add script to automatically test lighthouse on a page from devtools #11539
Conversation
I don't remember (or maybe we never knew) why this happens, but the current layout tests do mark |
IIRC it had something to do with the content shell used by layout tests, but it's been a very long time since we looked into it AFAIK. |
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.
thanks @adamraine this will be great for tracking down our PROTOCOL_TIMEOUT
nemesis (and just general explorations into DevTools errors compared to CLI 🎉 )!
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.
worked like a charm! just a few things to help us in the future, but LGTM
@@ -6,81 +6,13 @@ | |||
# Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. | |||
## | |||
|
|||
set -euo pipefail |
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.
set -u
at least still seems relevant, no?
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 Adding set -u
causes the DevTools check to fail in GitHub check. Seems to be failing on the yarn --frozen-lockfile
step: https://github.com/GoogleChrome/lighthouse/runs/1327351848.
This appears to be a failure of the script but I can't tell where. What do you make of this?
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.
Hmm maybe set -u
was not the problem?
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.
Yeah that error has been happening for the past several weeks (maybe even months?) and seems like we hit some sort of github network troubles after downloading all of chromium build tooling all the time heh 😄
I think this is unrelated to set -u
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.
@adamraine any chance you could add a brief note about this somewhere? https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/test/chromium-web-tests/README.md maybe?
(we should probably put together a "so you're trying to test/debug lighthouse in devtools..." doc soon)
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.
LGTM though a comment in the readme about our devtools suite that this thing exists like @brendankenny suggested would be cool too :)
This script is based heavily on. It's a hacky solution that executes a single DevTools web test which runs lighthouse on a page and then prints the LHR. The solution dividesrun-web-tests.sh
run-web-tests.sh
into two parts so the phony web test can be inserted after Lighthouse is rolled into DevTools.This should help test #11508 in GCP.
Currently, the script always has a
NO_SCREENSHOTS
error for Speed Index. Will this be a problem when searching forPROTOCOL_TIMEOUT
errors?Fixes #11529
Part of #6512