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

fix bundling; closes #3091 #3145

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 12 additions & 14 deletions Makefile
@@ -1,14 +1,14 @@
BROWSERIFY := "node_modules/.bin/browserify"
BROWSERIFY := "scripts/build.js"
KARMA := "node_modules/.bin/karma"
MOCHA := "bin/mocha"
NYC := "node_modules/.bin/nyc"

ifdef COVERAGE
define test_node
$(NYC) --no-clean --report-dir coverage/reports/$(1) $(MOCHA)
$(NYC) --no-clean --report-dir coverage/reports/$(1) $(MOCHA)
Copy link
Member Author

@boneskull boneskull Dec 12, 2017

Choose a reason for hiding this comment

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

from what I can tell the leading tab here is unnecessary

endef
else
test_node := $(MOCHA)
test_node := $(MOCHA)
Copy link
Member Author

Choose a reason for hiding this comment

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

and here

endif

TM_BUNDLE = JavaScript\ mocha.tmbundle
Expand All @@ -20,13 +20,7 @@ all: mocha.js
mocha.js: $(SRC) browser-entry.js
@printf "==> [Browser :: build]\n"
mkdir -p ${@D}
$(BROWSERIFY) ./browser-entry \
--require buffer/:buffer \
--plugin ./scripts/dedefine \
--ignore 'fs' \
--ignore 'glob' \
--ignore 'path' \
--ignore 'supports-color' > $@
$(BROWSERIFY) > $@

clean:
@printf "==> [Clean]\n"
Expand All @@ -38,7 +32,7 @@ lint:

test-node: test-bdd test-tdd test-qunit test-exports test-unit test-integration test-jsapi test-compilers test-requires test-reporters test-only test-global-only

test-browser: clean mocha.js test-browser-unit test-browser-bdd test-browser-qunit test-browser-tdd test-browser-exports
test-browser: clean mocha.js test-browser-unit test-browser-bdd test-browser-qunit test-browser-tdd test-browser-esm

test: lint test-node test-browser

Expand All @@ -48,15 +42,19 @@ test-browser-unit:

test-browser-bdd:
@printf "==> [Test :: Browser :: BDD]\n"
MOCHA_UI=bdd $(MAKE) test-browser-unit
MOCHA_TEST=bdd $(MAKE) test-browser-unit

test-browser-qunit:
@printf "==> [Test :: Browser :: QUnit]\n"
MOCHA_UI=qunit $(MAKE) test-browser-unit
MOCHA_TEST=qunit $(MAKE) test-browser-unit

test-browser-tdd:
@printf "==> [Test :: Browser :: TDD]\n"
MOCHA_UI=tdd $(MAKE) test-browser-unit
MOCHA_TEST=tdd $(MAKE) test-browser-unit

test-browser-esm:
@printf "==> [Test :: Browser :: ESM]\n"
MOCHA_TEST=esm $(MAKE) test-browser-unit

test-jsapi:
@printf "==> [Test :: JS API]\n"
Expand Down
104 changes: 63 additions & 41 deletions karma.conf.js
Expand Up @@ -4,6 +4,9 @@ var fs = require('fs');
var path = require('path');
var mkdirp = require('mkdirp');
var baseBundleDirpath = path.join(__dirname, '.karma');
var builder = require('./scripts/build');
Copy link
Member Author

Choose a reason for hiding this comment

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

this allows us to reuse the config in karma

var build = builder.build;
var bundlerOptions = builder.options;

var browserPlatformPairs = {
'chrome@latest': 'Windows 8',
Expand Down Expand Up @@ -32,23 +35,24 @@ module.exports = function (config) {
preprocessors: {
'test/**/*.js': ['browserify']
},
browserify: {
browserify: Object.assign({
insertGlobalVars: bundlerOptions.insertGlobalVars
}, {
debug: true,
configure: function configure (b) {
b.ignore('glob')
.ignore('fs')
.ignore('path')
.ignore('supports-color')
.require(path.join(__dirname, 'node_modules', 'buffer'), {expose: 'buffer'})
build(b)
.on('bundled', function (err, content) {
if (!err && bundleDirpath) {
if (err) {
throw err;
Copy link
Member Author

Choose a reason for hiding this comment

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

apparently we were eating errors before... oops.

}
if (bundleDirpath) {
// write bundle to directory for debugging
fs.writeFileSync(path.join(bundleDirpath,
'bundle.' + Date.now() + '.js'), content);
fs.writeFileSync(path.join(bundleDirpath, 'mocha.' + Date.now() +
Copy link
Member Author

Choose a reason for hiding this comment

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

I changed this filename

'.js'), content);
}
});
}
},
}),
reporters: ['mocha'],
colors: true,
browsers: ['PhantomJS'],
Expand All @@ -63,18 +67,8 @@ module.exports = function (config) {
}
};

// see https://github.com/saucelabs/karma-sauce-example

// We define the browser to run on the Saucelabs Infrastructure
// via the environment variables BROWSER and PLATFORM.
// PLATFORM is e.g. "Windows"
// BROWSER is expected to be in the format "<name>@<version>",
// e.g. "MicrosoftEdge@latest"
// See https://wiki.saucelabs.com/display/DOCS/Platform+Configurator#/
// for available browsers.

// TO RUN LOCALLY, execute:
// `CI=1 SAUCE_USERNAME=<user> SAUCE_ACCESS_KEY=<key> BROWSER=<browser> PLATFORM=<platform> make test-browser`
// TO RUN AGAINST SAUCELABS LOCALLY, execute:
// `CI=1 SAUCE_USERNAME=<user> SAUCE_ACCESS_KEY=<key> make test-browser`
var env = process.env;
var sauceConfig;

Expand All @@ -86,8 +80,8 @@ module.exports = function (config) {
if (env.SAUCE_USERNAME && env.SAUCE_ACCESS_KEY) {
// correlate build/tunnel with Travis
sauceConfig = {
build: 'TRAVIS #' + env.TRAVIS_BUILD_NUMBER +
' (' + env.TRAVIS_BUILD_ID + ')',
build: 'TRAVIS #' + env.TRAVIS_BUILD_NUMBER + ' (' +
env.TRAVIS_BUILD_ID + ')',
tunnelIdentifier: env.TRAVIS_JOB_NUMBER,
startConnect: false
};
Expand All @@ -96,8 +90,7 @@ module.exports = function (config) {
console.error('No SauceLabs credentials present');
}
} else if (env.APPVEYOR) {
console.error('AppVeyor detected');
bundleDirpath = path.join(baseBundleDirpath, process.env.APPVEYOR_BUILD_ID);
throw new Error('no browser tests should run on AppVeyor!');
} else {
console.error('Local/unknown environment detected');
bundleDirpath = path.join(baseBundleDirpath, 'local');
Expand All @@ -124,21 +117,50 @@ module.exports = function (config) {
addSauceTests(cfg);
}

// the MOCHA_UI env var will determine if we're running interface-specific
// tests. since you can only load one at a time, each must be run separately.
// each has its own set of acceptance tests and a fixture.
// the "bdd" fixture is used by default.
var ui = env.MOCHA_UI;
if (ui) {
if (cfg.sauceLabs) {
cfg.sauceLabs.testName = 'Interface "' + ui + '" integration tests';
}
cfg.files = [
'test/browser-fixtures/' + ui + '.fixture.js',
'test/interfaces/' + ui + '.spec.js'
];
} else if (cfg.sauceLabs) {
cfg.sauceLabs.testName = 'Unit Tests';
/* the MOCHA_TEST env var will be set for "special" cases of tests.
* these may require different interfaces or other setup which make
* them unable to be batched w/ the rest.
*/
var MOCHA_TEST = env.MOCHA_TEST;
switch (MOCHA_TEST) {
case 'bdd':
case 'tdd':
case 'qunit':
if (cfg.sauceLabs) {
cfg.sauceLabs.testName =
'Interface "' + MOCHA_TEST + '" Integration Tests';
}
cfg.files = [
'test/browser-fixtures/' + MOCHA_TEST + '.fixture.js',
'test/interfaces/' + MOCHA_TEST + '.spec.js'
];
break;

case 'esm':
// for now we will only run against Chrome to test this.
if (cfg.sauceLabs) {
cfg.sauceLabs.testName = 'ESM Integration Tests';
cfg.browsers = ['chrome@latest'];
var launcher = cfg.customLaunchers['chrome@latest'];
Copy link
Member Author

Choose a reason for hiding this comment

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

sorry this is awkward; we need to update the dev scripts to ES2015+.

cfg.customLaunchers = {
'chrome@latest': launcher
};
} else if (!env.TRAVIS) {
cfg.browsers = ['Chrome'];
Copy link
Member Author

@boneskull boneskull Dec 12, 2017

Choose a reason for hiding this comment

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

what other browsers currently support ES modules? @mochajs/core

Copy link
Contributor

Choose a reason for hiding this comment

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

in accordance to MDN

This feature is only just beginning to be implemented in browsers natively at this time. It is implemented in many transpilers, such as TypeScript and Babel, and bundlers such as Rollup and Webpack.

image

} else {
console.error(
'skipping ESM tests & exiting; no SauceLabs nor local run detected');
process.exit(0);
}
cfg.files = [
'test/browser-fixtures/esm.fixture.html',
Copy link
Member Author

Choose a reason for hiding this comment

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

can we force Karma to load a .js file as a module instead of script? I have no idea.

'test/browser-specific/esm.spec.js'
];
break;
default:
if (cfg.sauceLabs) {
cfg.sauceLabs.testName = 'Unit Tests';
}
}

config.set(cfg);
Expand Down
57 changes: 57 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Expand Up @@ -321,6 +321,7 @@
"supports-color": "4.4.0"
},
"devDependencies": {
"aliasify": "^2.1.0",
"assert": "^1.4.1",
"browserify": "^14.4.0",
"buffer": "^4.9.1",
Expand Down
46 changes: 46 additions & 0 deletions scripts/build.js
@@ -0,0 +1,46 @@
#!/usr/bin/env node
'use strict';

/**
* Mocha's build script which is sadly too complex to manage from the command
* line
* @type {Browserify}
*/

const browserify = require('browserify');
const path = require('path');
const dedefine = require('./dedefine');
const aliasify = require('aliasify');

const options = {
basedir: path.join(__dirname, '..'),
entries: ['./browser-entry.js'],
insertGlobalVars: {
Buffer (file, basedir) {
const filepath = path.join(path.relative(path.dirname(file), basedir),
'node_modules',
'buffer');
return `require('${filepath}').Buffer`;
}
}
};

const build = (b) => b.ignore('fs')
.ignore('glob')
.ignore('path')
.ignore('supports-color')
.transform(aliasify, {
replacements: {
'^buffer/?': () => require.resolve('buffer/index.js')
},
global: true
})
.plugin(dedefine);

exports.build = build;
exports.options = options;

if (require.main === module) {
build(browserify(options)).bundle()
.pipe(process.stdout);
}
7 changes: 7 additions & 0 deletions test/browser-fixtures/esm.fixture.html
@@ -0,0 +1,7 @@
<script>
delete window.require;
Copy link
Member Author

Choose a reason for hiding this comment

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

this ensures that breakage will occur if the suffix on the distfile is bad.

Copy link
Member Author

Choose a reason for hiding this comment

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

see #3139 for what I mean

</script>
<script type="module">
import '/base/mocha.js';
window.MOCHA_IS_OK = true;
</script>
5 changes: 5 additions & 0 deletions test/browser-specific/esm.spec.js
@@ -0,0 +1,5 @@
'use strict';

it('should register a global if it did not fail', function () {
expect(window.MOCHA_IS_OK).to.be.ok();
});