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.

Sign up for GitHub

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

Jump to bottom

Update Mocha to v8, drop Phantomic, run browser tests in Chromium #35

Merged
merged 5 commits into from May 23, 2021
Merged

Update Mocha to v8, drop Phantomic, run browser tests in Chromium #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5588e29
Update mocha to v8, adjust assertions
m90 May 15, 2021
c321c00
Port browser tests to use puppeteer
m90 May 15, 2021
df8ffea
Pass in custom reporters in setup to work around dynamic require limi…
m90 May 16, 2021
9b4e1b2
Ensure brout is required before mocha
m90 May 16, 2021
3bd112a
Remove phantomic references from README and update engine requirements
m90 May 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions .travis.yml
View file
Open in desktop
@@ -1,5 +1,5 @@
language: node_js
node_js:
- "6"
- "8"
- "10"
- "12"
- "14"
- "16"
20 changes: 1 addition & 19 deletions README.md
View file
Open in desktop
Expand Up @@ -19,26 +19,9 @@ with a headless browser, on a Selenium grid, in the cloud with SauceLabs or
generates a standalone HTML page to run the tests. The underlying modules can
also be used as Browserify plugins.

- [Phantomic][] for headless browser testing
- [min-webdriver][] for Selenium and SauceLabs support
- [Consolify][] to generate a standalone HTML page

## Headless browser testing

Browserify a test and run in a Phantom.JS with [Phantomic][]:

```
$ browserify -p mocaccino test.js | phantomic --brout
```

### Code coverage with headless browser

Use the [Coverify][] transform and [Phantomic][]:

```
$ browserify -p mocaccino -t coverify test.js | phantomic --brout | coverify
```

### Code coverage with node

Use the [Coverify][] transform and node:
Expand Down Expand Up @@ -76,7 +59,7 @@ I/O to happen. It's ignored if `--node` is given.

## Compatibility

- Node 0.10 or later
- Node 12 or later
- Browserify 5.9 or later (since version 1.0.0)
- Browserify 4.x (before 1.0.0)

Expand All @@ -90,7 +73,6 @@ MIT
[Mocha]: http://mochajs.org/
[Browserify]: http://browserify.org
[Mochify]: https://github.com/mantoni/mochify.js
[Phantomic]: https://github.com/mantoni/phantomic
[min-webdriver]: https://github.com/mantoni/min-webdriver
[Consolify]: https://github.com/mantoni/consolify
[Coverify]: https://github.com/substack/coverify
19 changes: 10 additions & 9 deletions lib/mocaccino.js
View file
Open in desktop
Expand Up @@ -44,6 +44,11 @@ module.exports = function (b, opts) {
return { main : pkg.browser };
}
});
var broutPath = path.relative(process.cwd(), broutFile);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is not strictly necessary, but it's a leftover from moving things around and I felt the order is now easier to understand. It could also just be reverted again.

b.require('./' + broutPath.replace(/\\/g, '/'), {
expose : 'brout'
});

var mochaReporters = require(opts.mochaPath
? path.resolve(process.cwd(), opts.mochaPath, 'lib', 'mocha')
: 'mocha').reporters;
Expand All @@ -56,10 +61,6 @@ module.exports = function (b, opts) {
expose: reporter
});
}
var broutPath = path.relative(process.cwd(), broutFile);
b.require('./' + broutPath.replace(/\\/g, '/'), {
expose : 'brout'
});

var mochaPath = opts.mochaPath
? resolve.sync('mocha.js', { paths: [opts.mochaPath] })
Expand All @@ -78,8 +79,8 @@ module.exports = function (b, opts) {
|| 80;
var setupContent = content
.replace('{{WINDOW_WIDTH}}', JSON.stringify(windowWidth))
.replace('{{REPORTER}}', reporter)
.replace('\'{{REPORTER_OPTIONS}}\'', JSON.stringify(reporterOptions))
.replace(/{{REPORTER}}/g, reporter)
.replace(/'{{REPORTER_OPTIONS}}'/g, JSON.stringify(reporterOptions))
.replace('{{YIELDS}}', yields)
.replace('{{UI}}', ui)
.replace('\'{{TIMEOUT}}\'', timeout)
Expand All @@ -94,7 +95,6 @@ module.exports = function (b, opts) {
b.transform(function () {
var s = '';
return through(function (chunk, enc, next) {
/*jslint unparam: true*/
s += chunk;
var p = s.lastIndexOf('\n');
if (p !== -1) {
Expand Down Expand Up @@ -129,7 +129,6 @@ module.exports = function (b, opts) {

function apply() {
b.pipeline.get('deps').push(through.obj(function (row, enc, next) {
/*jslint unparam: true*/
if (row.entry) {
row.deps['mocaccino-setup'] = setupFile;
row.order++;
Expand All @@ -139,7 +138,9 @@ module.exports = function (b, opts) {
var self = this;
listener.then(function (err, res) {
if (!err) {
setupSource = (res.mocha || '') + res.setup;
setupSource = (opts.node ? '' : 'require(\'brout\');\n')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mocha 6.2.0 starts adding behavior to console.log mochajs/mocha#3725 so it's now needed to require brout before requiring mocha. I'm not sure if this ad-hoc string doing it here for us is the most elegant way to do it, but then I also didn't want to add a single line file. Any opinions?

Copy link
Owner

Choose a reason for hiding this comment

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

My opinion is that any hack that get's us there is fine 😆

+ (res.mocha || '')
+ res.setup;
self.push(createSetupEntry());
}
next();
Expand Down
12 changes: 8 additions & 4 deletions lib/setup-browser.js
View file
Open in desktop
Expand Up @@ -8,8 +8,6 @@
*/
'use strict';

require('brout');

Mocha.reporters.Base.window.width = JSON.parse('{{WINDOW_WIDTH}}');
Mocha.reporters.Base.symbols.dot = '.';
var grep = '{{GREP}}';
Expand All @@ -21,10 +19,16 @@ var mocha = new Mocha({
if ('{{INVERT}}' === true) { // eslint-disable-line no-constant-condition
mocha.invert();
}
mocha.reporter('{{REPORTER}}', '{{REPORTER_OPTIONS}}');

if (Mocha.reporters['{{REPORTER}}']) {
mocha.reporter('{{REPORTER}}', '{{REPORTER_OPTIONS}}');
} else {
mocha.reporter(require('{{REPORTER}}'), '{{REPORTER_OPTIONS}}');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mocha is now bundled using Rollup which does not support dynamic requires for CommonJS. This way we pass in the reporter module instead of its name so that Mocha doesn't have to do any module resolution anymore.

}

mocha.ui('{{UI}}');
mocha.timeout('{{TIMEOUT}}');
mocha.useColors('{{USE_COLORS}}');
mocha.color('{{USE_COLORS}}');
mocha.suite.emit('pre-require', window, '', mocha);
var t = new Date().getTime();
var y = Number('{{YIELDS}}');
Expand Down
2 changes: 1 addition & 1 deletion lib/setup-node.js
View file
Open in desktop
Expand Up @@ -22,7 +22,7 @@ if ('{{INVERT}}' === true) { // eslint-disable-line no-constant-condition
}
_mocha.reporter('{{REPORTER}}', '{{REPORTER_OPTIONS}}');
_mocha.timeout('{{TIMEOUT}}');
_mocha.useColors('{{USE_COLORS}}');
_mocha.color('{{USE_COLORS}}');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

useColors seems to have been deprecated a while ago and was removed in Mocha 8 mochajs/mocha#4178

_mocha.suite.emit('pre-require', global, '', _mocha);

setTimeout(function () {
Expand Down