Skip to content

chore(build): use new config format for mocha v6.x #2309

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

Merged
raymondfeng merged 1 commit into from
Mar 5, 2019
Merged

chore(build): use new config format for mocha v6.x #2309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .vscode/launch.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
"program": "${workspaceRoot}/packages/build/node_modules/.bin/_mocha",
"cwd": "${workspaceRoot}",
"args": [
"--opts",
"${workspaceRoot}/test/mocha.opts",
"--config",
"${workspaceRoot}/packages/build/config/.mocharc.json",
"packages/*/dist/__tests__/**/*.js",
"-t",
"0"
Expand Down
43 changes: 22 additions & 21 deletions docs/site/todo-tutorial-scaffolding.md
View file
Open in desktop

Large diffs are not rendered by default.

14 changes: 10 additions & 4 deletions packages/build/bin/run-mocha.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,22 @@ function run(argv, options) {
const mochaOpts = argv.slice(2).map(a => a.replace(/\bDIST\b/g, dist));

const setMochaOpts =
!utils.isOptionSet(mochaOpts, '--opts') &&
!utils.mochaOptsFileProjectExists();
!utils.isOptionSet(
mochaOpts,
'--config', // mocha 6.x
'--opts', // legacy
'--package', // mocha 6.x
'--no-config', // mocha 6.x
) && !utils.mochaConfiguredForProject();

// Add default options
// Keep it backward compatible as dryRun
if (typeof options === 'boolean') options = {dryRun: options};
options = options || {};
if (setMochaOpts) {
Copy link
Member

@bajtos bajtos Feb 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

setMochaOpts is checking for presence of --opts. So IIUC the current implementation, if --opts is not provided, we add --config. Which means that lb-mocha --config my.json will add --config .mocharc.json to the command line, ending up with two --config flags. Is that desirable? I think it's not. Either way, please add a test to verify how lb-mocha behaves in this scenario.

How about backwards compatibility? If we replace --opts with --config in our check and stop detecting --opts, then I think this is a breaking change requiring a semver-major release. It may be more conventient for our users if we keep support for both --opts and --config options. Either way, please add a test to verify what happens when lb-mocha is invoked with --opts only: do we add --config or not?

Copy link
Contributor Author

@raymondfeng raymondfeng Feb 4, 2019

Choose a reason for hiding this comment

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

Maybe it's better to bump @loopback/build to the next major version. Using --opts for [email protected] seems to be add confusion.

Copy link
Member

@bajtos bajtos Feb 5, 2019

Choose a reason for hiding this comment

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

I am fine with publishing a new semver-major version of @loopback/build. See https://github.com/strongloop/loopback-next/blob/master/docs/site/DEVELOPING.md#making-breaking-changes for a check-list.

Copy link
Contributor Author

@raymondfeng raymondfeng Mar 5, 2019

Choose a reason for hiding this comment

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

It turns out that mocha 6.x continues to support mocha.opts. I have improved the PR to respect --opts and mocha.opts in addition to .mocharc.* formats. It should be now backward compatible.

const mochaOptsFile = utils.getConfigFile('mocha.opts');
mochaOpts.unshift('--opts', mochaOptsFile);
// Use the default `.mocharc.json` from `@loopback/build`
const mochaOptsFile = utils.getConfigFile('.mocharc.json');
mochaOpts.unshift('--config', mochaOptsFile);
}

const allowConsoleLogsIx = mochaOpts.indexOf('--allow-console-logs');
Expand Down
17 changes: 13 additions & 4 deletions packages/build/bin/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,18 @@ function isOptionSet(opts, ...optionNames) {
);
}

function mochaOptsFileProjectExists() {
const configFile = path.join(getPackageDir(), 'test/mocha.opts');
return fs.existsSync(configFile);
function mochaConfiguredForProject() {
const configFiles = [
'.mocharc.js',
'.mocharc.json',
'.mocharc.yaml',
'.mocharc.yml',
'test/mocha.opts',
];
return configFiles.some(f => {
const configFile = path.join(getPackageDir(), f);
return fs.existsSync(configFile);
});
}

exports.getCompilationTarget = getCompilationTarget;
Expand All @@ -228,4 +237,4 @@ exports.resolveCLI = resolveCLI;
exports.runCLI = runCLI;
exports.runShell = runShell;
exports.isOptionSet = isOptionSet;
exports.mochaOptsFileProjectExists = mochaOptsFileProjectExists;
exports.mochaConfiguredForProject = mochaConfiguredForProject;
7 changes: 7 additions & 0 deletions packages/build/config/.mocharc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"require": "source-map-support/register",
"recursive": true,
"exit": true,
"reporter": "dot"
}

4 changes: 0 additions & 4 deletions packages/build/config/mocha.opts
View file
Open in desktop

This file was deleted.

34 changes: 21 additions & 13 deletions packages/build/test/integration/scripts.integration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ describe('mocha', function() {

function cleanup() {
var run = require('../../bin/run-clean');
run(['node', 'bin/run-clean', 'test/mocha.opts']);
run(['node', 'bin/run-clean', '.mocharc.json']);
}

beforeEach(() => {
Expand All @@ -378,42 +378,50 @@ describe('mocha', function() {
process.chdir(cwd);
});

it('loads built-in mocha.opts file', () => {
it('loads built-in .mocharc.json file', () => {
var run = require('../../bin/run-mocha');
var command = run(['node', 'bin/run-mocha', '"dist/__tests__"'], true);
const builtInMochaOptsFile = path.join(
__dirname,
'../../config/mocha.opts',
'../../config/.mocharc.json',
);
assert(
command.indexOf(builtInMochaOptsFile) !== -1,
'--opts should be set by default',
'--config should be set by default',
);
});

it('honors --opts option', () => {
it('honors --config option', () => {
var run = require('../../bin/run-mocha');
var command = run(
['node', 'bin/run-mocha', '--opts custom/mocha.opts', '"dist/__tests__"'],
[
'node',
'bin/run-mocha',
'--config custom/.mocharc.json',
'"dist/__tests__"',
],
true,
);
assert(
command.indexOf('--opts custom/mocha.opts') !== -1,
'--opts custom/mocha.opts should be honored',
command.indexOf('--config custom/.mocharc.json') !== -1,
'--config custom/.mocharc.json should be honored',
);
});

it('loads mocha.opts specific project file', () => {
it('loads .mocharc.json specific project file', () => {
var run = require('../../bin/run-mocha');
const buitInMochaOptsPath = path.join(__dirname, '../../config/mocha.opts');
const destPath = path.join(__dirname, './fixtures/test/mocha.opts');
const buitInMochaOptsPath = path.join(
__dirname,
'../../config/.mocharc.json',
);
const destPath = path.join(__dirname, './fixtures/.mocharc.json');

fs.copyFileSync(buitInMochaOptsPath, destPath);

var command = run(['node', 'bin/run-mocha', '"dist/__tests__"'], true);
assert(
command.indexOf('--opts') === -1,
'should skip built-in mocha.opts file when specific project file exist',
command.indexOf('--config') === -1,
'should skip built-in .mocharc.json file when specific project file exist',
);
});
});
4 changes: 4 additions & 0 deletions packages/cli/generators/project/templates/.mocharc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"recursive": true,
"require": "source-map-support/register"
}
2 changes: 0 additions & 2 deletions packages/cli/generators/project/templates/test/mocha.opts
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion packages/cli/lib/project-generator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ module.exports = class ProjectGenerator extends BaseGenerator {
}

if (!this.projectInfo.mocha) {
this.fs.delete(this.destinationPath('test/mocha.opts'));
this.fs.delete(this.destinationPath('.mocharc.json'));
}

if (!this.projectInfo.vscode) {
Expand Down