cli flag to specify chromedriver port #1620
Conversation
|
I am new in this org as a contributor, let me know what else needs to be added. |
svkrclg
changed the title
|
Hi @svkrclg looks great, thanks the addition! I'm on vacation, that's why the late merge. |
lib/chrome/webdriver/builder.js
Outdated
| @@ -25,6 +25,9 @@ module.exports.configureBuilder = function(builder, baseDir, options) { | |||
| } | |||
|
|
|||
| const serviceBuilder = new chrome.ServiceBuilder(chromedriverPath); | |||
| if (options.chrome.chromedriverPort) { | |||
There was a problem hiding this comment.
This need either use lodash.get or you can add a check like:
if (options.chrome && options.chrome.chromedriverPort) ...
The reason is that even when you run Chrome, we aren't 100% you have set any options for Chrome (that's the way it works right now at least).
|
Also, do you think it is a good idea to check whether the port is free or not before starting chromedriver? |
|
@svkrclg no I think that could up to the developer that starts the driver just as long as we push an error to the log if it fails. |
|
Verified it, in case of an invalid port, the Chromedriver error is pushed to |
|
Thank you @svkrclg !!! |
sitespeedio/sitespeed.io#3427