Speed-up local browsers detection (Windows 7)

[mazswojejzony]

On my machine (Core-i5 + SSD) browser detection takes approx. 1min 5s. The reason is that Chrome is installed in "c:\Program Files (x86)\Google\Chrome\Application\chrome.exe" folder (i.e. not in the AppData sub-folder). Also I do not have Opera nor phantomjs installed.

So what happens is launchpad tries to find those browsers using pathQuery e.g. dir /s /b opera.exe command which takes lots of time to complete.

Are there any plans to provide means to change platform browsers configuration so that users could define what local browsers they want to use and where they are installed?

[daffl]

The current version should only look for a browser if you are actually trying to start it or want to list them all using launcher.browsers (which is not necessary to start a specific browser). It probably still takes a while to look for just Chrome though.

We could add the ability for Launchpad to look for environment variables (e.g. LAUNCHPAD_OPERA="false" or LAUNCHPAD_CHROME=<path\to\chrome.exe>).

[mazswojejzony]

This is exactly what I was thinking about, i.e. to check env variables first, then try pre-configured default locations. Although this is a good idea it does not solve a whole issue. Thing is launchpad is used by wct-local which is used by web-component-tester. And yes, it uses the browsers method you mentioned. :-)

So it seems to me that to fully resolve this problem the following needs to be done:

1. Add the ability for Launchpad to look for environment variables:
   1. LAUNCHPAD_CHROME=<path\to\chrome.exe>, LAUNCHPAD_OPERA=<path/to/opera>, etc. Defaults taken from platform/<platform.js>
   2. LAUNCHPAD_ENABLED_BROWSERS=chrome,firefox,ie - this would limit the launcher.browsers to detect browsers listed in this env variable; default = detect all browsers
2. Also, I think it would make sense to extend defaultLocation param to accept an array of strings/paths. Then it would be possible to update platform/windows.js to look for Chrome in both AppData and Program Files folders; in such a case the above mentioned LAUNCHPAD_CHROME could be used as a last resort only.

[mazswojejzony]

BTW: I am happy to work on a PR if you think this idea makes sense.

[daffl]

That would be fantastic. Probably two separate PRs for 1) and 2). For 1) I'd make LAUNCHPAD_ENABLED_BROWSERS a little shorter and rename it to LAUNCHPAD_BROWSERS. Let me know if you need anything.

[daffl]

For 1, the discovery is happening at https://github.com/ekryski/launchpad/blob/master/lib/local/browser.js#L18 which is probably the place to check for the command line parameter. Maybe something like

var envSetting = process.env['LAUNCHPAD_' + browser.name.toUpperCase()];
var location = envSetting || browser.defaultLocation;
if (location && fs.existsSync(location)) {
  debug('Found browser ' + current.name + ' in location', location);
  return setPath(current, browser.defaultLocation);
}

if(envSetting === 'false') {
  return null;
}

That might almost be it. Testing it might be a little more tricky especially in CI.

[mazswojejzony]

Looks like you almost did it. 👍 I hope to find some time to finish this up in next few days.

[MeTaNoV]

status? 😄

[mazswojejzony]

I've got both LAUNCHPAD_ & LAUNCHPAD_BROWSERS implemented. See mazswojejzony@a63e468

The downside is I find it really hard to add new tests for those features. I tried to use grunt-env (to change env settings for separate grunt test tasks) + mockery (to ensure the launchpad local module is reinitialized after env settings are changed) but I failed to find a working solution.

Any thoughts?

[daffl]

We could always run the tests with environment variables set and install the local PhantomJS NPM package in development mode. Then you'd just have to change the npm test script to something like LAUNCHPAD_PHANTOMJS=./node_modules/.bin/phantomjs grunt test.

[mazswojejzony]

That would kind of work for LAUNCHPAD_<browser> setting.

What about the LAUNCHPAD_BROWSERS? Would it be OK if it was set to LAUNCHPAD_BROWSERS=chrome,firefox,phantomjs? Then we could change the following assert from:

assert.ok(browsers.length, 'Found at least one browser');

to:

assert.ok(browsers.length > 0 && browsers.length <= 3, 'Found at least one and no more than three browsers');

The downside is we would not test the default path when no env vars are set.

[mazswojejzony]

I've found a way to test this using decache to re-init the local module. See PR for details.