#3128 replace phantomjs with chrome for browser tests

[akrawchyk]

Description of the Change

Replaces PhantomJS with Puppeteer and updates Karma to use ChromeHeadless browser.

Alternate Designs

The issue #3128 describes using karma-headless-chrome-launcher for this.

However, the project already has karma-chrome-launcher as a dependency, see #3152 (comment). which has documentation for setting up ChromeHeadless, see https://github.com/karma-runner/karma-chrome-launcher#headless-chromium-with-puppeteer. I thought this would be preferred, so I configured Karma to use Puppeteer.

Also, during my testing the karma-headless-chrome-launcher was unable to locate Chromium. I'm not sure it's supported by the library used to locate Chrome for that plugin, see hughsk/chrome-location#7.

karma-chrome-launcher does not locate Chromium by default either. Both Travis and Appveyor use Google Chrome, but you can use Chromium to run tests locally by setting the CHROME_BIN env var:

CHROME_BIN=$(which chromium-browser) make test-browser

Why should this be in core?

#3128 (comment)

Benefits

#3128 (comment)

Possible Drawbacks

Puppeteer downloads a usable Chrome binary on install. This download is currently >95MB in size compared to PhantomJS <20MB, almost 5x bigger download. This is undesirable for obvious reasons.

[coveralls]

Coverage increased (+0.05%) to 90.014% when pulling c5a128b on akrawchyk:issue/3128 into 6fc2d9e on mochajs:master.

[akrawchyk]

The Node v4 Travis tests failed: https://travis-ci.org/mochajs/mocha/jobs/316017544

Looks like puppeteer requires Node >= v6.4.0. If this is the desired path, it would have to wait on #3149.

[boneskull]

Puppeteer v0.13.0 won't run in Node.js v4.x.

Unfortunately, there's no such thing as an "optional dev dependency". This would be nice, since we don't actually need it, since we only run the browser tests from Node.js v8.x.

Would downgrading to a version that supports Node.js v4.x work? Happy to hear other suggestions as well.

[boneskull]

the commit message seems to have a typo

[akrawchyk]

@boneskull thanks for the feedback, I fixed the commit message.

Unfortunately, there's no such thing as an "optional dev dependency". This would be nice since we don't actually need it, since we only run the browser tests from Node.js v8.x.

Agreed, but it is nice to be able to depend on puppeteer's binary instead of adding dependencies to resolve the paths to Chrome.

Would downgrading to a version that supports Node.js v4.x work? Happy to hear other suggestions as well.

I looked through the commit history for the project and I don't think this is viable since the project makes heavy use of ES6 features (like destructuring and default function params) that are not supported on older versions of Node, unfortunately.

I'm going to follow this guide and tinker some more with the Karma config to try and get this working without puppeteer as a dependency.

[coveralls]

Coverage increased (+0.05%) to 90.014% when pulling e623926 on akrawchyk:issue/3128 into 9150e34 on mochajs:master.

[akrawchyk]

@boneskull I removed puppeteer since karma-chrome-launcher appears to work out of the box (see thread at karma-runner/karma#2489 as well as the guide I linked above).

I updated the Travis config based on https://docs.travis-ci.com/user/chrome but I don't think we need to do the additional setup for Chrome headless here since Karma takes care of launching Chrome.

The Appveyor config doesn't need to be updated since Chrome is included in the build worker images (see https://www.appveyor.com/docs/build-environment/#test-runners).

[coveralls]

Coverage increased (+0.05%) to 90.014% when pulling aa02506 on akrawchyk:issue/3128 into ae3712c on mochajs:master.

[akrawchyk]

@boneskull let me know your feedback once you get a chance to review the changes I pushed, thanks!

[boneskull]

thanks

[boneskull]

I'm going to pull this down and try it out; I'll address the conflict in package-lock.json as well.

[boneskull]

awesome. merged via e54370e

thanks @akrawchyk