-
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
tests(devtools): fix webserver #12236
Conversation
@@ -17,7 +17,7 @@ export latest_content_shell="$LH_ROOT/.tmp/chromium-web-tests/content-shells/$la | |||
# - /inspector-sources -> the inspector resources from the content shell | |||
# - CORS (Access-Control-Allow-Origin header) | |||
|
|||
ln -s "$DEVTOOLS_PATH/out/Default/resources/inspector" "$DEVTOOLS_PATH/test/webtests/http/tests/inspector-sources" | |||
ln -s "$latest_content_shell/out/Release/resources/inspector" "$DEVTOOLS_PATH/test/webtests/http/tests/inspector-sources" |
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.
How I debugged...
I removed the output redirection of the webserver to dev/null and noticed 404s. integration_runner.js was missing. I realized line wasn't doing what the comment suggested... the inspector-sources
file is meant to come from content_shell, the reason is that the compiled devtools framework code should match the content shell binary.
I guess integration_runner.js
was being generated by devtools before, but no longer is. the incorrect symbolic link was overlooked because of that.
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.
Scratch that. DT team refactored the BUILD rules, and so we weren't building the right thing any more. And the output folder changed slightly.
@@ -2,11 +2,11 @@ Tests that exporting works. | |||
|
|||
++++++++ testExportHtml | |||
|
|||
# of .lh-audit divs (original): 137 | |||
# of .lh-audit divs (original): 136 |
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.
What changed here?
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.
file last changed with csp-xss https://github.com/GoogleChrome/lighthouse/commits/master/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-export-run-expected.txt
I don't see why this would have changed, since the audit is in experimental, so doesn't show in DT.
Fixes #12209