Skip to content
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

Added --experimental.dumpChromeNetlog #274

Merged
merged 3 commits into from
Mar 17, 2017

Conversation

worenga
Copy link
Contributor

@worenga worenga commented Feb 17, 2017

No description provided.

@worenga
Copy link
Contributor Author

worenga commented Feb 17, 2017

There seems to be an issue that the log is not entirely written and truncated.

@@ -124,6 +124,15 @@ class Engine {
options.postScript.unshift(...videoPostScripts);
}

if(options.experimental.dumpChromeNetlog) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you change that to use lodash.get or add an extra if, yargs doesn't populate experimental so it will try .dumpChromeNetlog on an undefined object if you don't send any experimental arguments. same in chrome.js.

@soulgalore
Copy link
Member

There seems to be an issue that the log is not entirely written and truncated.

Ok so it's unusable?

@worenga
Copy link
Contributor Author

worenga commented Feb 17, 2017

Yes, it seems to be an issue with the chromedriver, waiting for replies here: https://groups.google.com/forum/#!topic/chromedriver-users/T5uxtJ3kKuU

@worenga
Copy link
Contributor Author

worenga commented Feb 21, 2017

@soulgalore
Copy link
Member

soulgalore commented Feb 22, 2017

Maybe we can fix it temporary by sending all the default settings to Chromedriver as the answer in your bug report just so you get the info you need?

@worenga
Copy link
Contributor Author

worenga commented Mar 3, 2017

there is now a fix available, i guess it will land in chromedriver 2.28. There still seems to be an related issue with windows, but im not sure if that is concerning us at all. If i manage to find some time im gonna recompile chromium and test the patch it with browsertime over the weekend

@soulgalore
Copy link
Member

@worenga we use the 2.28 in master, do you have time to test if it works?

@worenga
Copy link
Contributor Author

worenga commented Mar 17, 2017

yes, the log ins no longer truncated in the new chromedriver, rebased pr

@tobli
Copy link
Member

tobli commented Mar 17, 2017

🎉 🎉 🎉
Merging!

@tobli tobli merged commit cfde5a1 into sitespeedio:master Mar 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants