Add ability to pass in test files to be ran before positional files via --file #3190

hswolff · 2018-01-03T13:37:31Z

This is to resolve #3181.

I was a little unsure how to describe this change. I think including something about how these args are ran first should be noted but I'm not exactly sure how best to do that.

Also I didn't do any file path sanitization to anything passed into --file nor did I apply --recursive to the path as right now I'm of the mind that any file passed to this argument should be explicit and not be effected by any sibling flags.

Happy to change anything that people disagree with.

Also apologies for the trailing whitespace cleanup, that was done by my editor. Happy to remove that change if you want.

Cheers! Excited to contribute! :D

jsf-clabot · 2018-01-03T13:37:32Z

All committers have signed the CLA.

coveralls · 2018-01-03T13:43:09Z

Coverage decreased (-4.9%) to 85.073% when pulling 9ea849c on hswolff:add-file-arg into a554adb on mochajs:master.

boneskull

thanks for this; I wasn't going to have time to do it myself. I've requested some changes here.

boneskull · 2018-01-03T20:13:17Z

bin/_mocha

@@ -199,7 +205,8 @@ program
  .option('--delay', 'wait for async suite definition')
  .option('--allow-uncaught', 'enable uncaught errors to propagate')
  .option('--forbid-only', 'causes test marked with only to fail the suite')
-  .option('--forbid-pending', 'causes pending tests and test marked with skip to fail the suite');
+  .option('--forbid-pending', 'causes pending tests and test marked with skip to fail the suite')
+  .option('--file <file>', 'include a file to be ran during the suite', list, []);


alas, -f is taken, so no alias is possible

I would prefer this was a repeatable flag instead of one that must be comma-delimited. I assume that's possible in commander, but am unfamiliar with its API

boneskull · 2018-01-03T20:16:27Z

bin/_mocha

 files = files.map(path => resolve(path));

 if (program.sort) {
  files.sort();
 }

+// add files given through --file to be ran first
+files = fileArgs.concat(files);


okay, so --sort doesn't work with --file, which makes sense. that should be noted somewhere if it isn't already.

Added in docs/index.md

boneskull · 2018-01-03T20:16:50Z

docs/index.md

@@ -737,6 +737,7 @@ Mocha supports the `err.expected` and `err.actual` properties of any thrown `Ass
    --debug-brk                             enable node's debugger breaking on the first line
    --globals <names>                       allow the given comma-delimited global [names]
    --es_staging                            enable all staged features
+    --file <file>                           include a file to be ran during the suite [file]


We should add a section in the documentation for this option.

Added below.

boneskull · 2018-01-03T20:17:17Z

test/integration/fixtures/options/file-alpha.fixture.js

+
+describe('alpha', function () {
+  it('should be executed first', function () {
+    if (global.beta) {


probably better to assert global.beta to be undefined?

boneskull · 2018-01-03T20:18:01Z

test/integration/options.spec.js

@@ -4,6 +4,7 @@ var path = require('path');
 var assert = require('assert');
 var run = require('./helpers').runMochaJSON;
 var directInvoke = require('./helpers').invokeMocha;
+var resolvePath = require('./helpers').resolveFixturePath;


we may as well:

var helpers = require('helpers'); var run = helpers.run; // etc

boneskull · 2018-01-03T20:18:15Z

test/integration/options.spec.js

@@ -91,6 +92,29 @@ describe('options', function () {
    });
  });

+  describe.only('--file', function () {


whoops 😉

boneskull · 2018-01-03T20:19:01Z

test/integration/options.spec.js

@@ -91,6 +92,29 @@ describe('options', function () {
    });
  });

+  describe.only('--file', function () {
+    before(function () {


a good rule of thumb is that you should always use beforeEach unless you can't.

boneskull · 2018-01-03T20:19:25Z

test/integration/options.spec.js

+      args = ['--file', resolvePath('options/file-alpha.fixture.js')];
+    });
+
+    it('should run tests passed via file first', function (done) {


boneskull · 2018-01-03T20:21:01Z

please reverse the whitespace changes; this might cause difficulty with incoming PR #3173

boneskull · 2018-01-03T20:21:12Z

coveralls failure due to use of .only

hswolff · 2018-01-04T04:30:04Z

Thank you so much for the fast review!

All PR feedback addressed. Thank for catching that .only goof :D

ljharb · 2018-01-04T04:37:23Z

bin/_mocha

+/**
+ * Parse multiple flag.
+ */
+const collect = (val, memo) => {


Could this just return memo.concat(val), or be Function.bind(Function.call, Array.prototype.concat)?

That it could. I took this straight from the commander.js page. I'm fine with it as is as it's pretty clear to any reader what the code is doing. Happy to change if people feel strongly otherwise.

yeah, I agree this would be better:

const collect = (val, memo) => memo.concat(val);

coveralls · 2018-01-04T04:43:55Z

Coverage increased (+0.05%) to 90.018% when pulling a12c310 on hswolff:add-file-arg into a554adb on mochajs:master.

boneskull · 2018-01-04T22:58:58Z

docs/index.md

@@ -25,17 +25,17 @@ Mocha is a feature-rich JavaScript test framework running on [Node.js](https://n
 - [maps uncaught exceptions to the correct test case](#browser-specific-methods)
 - [async test timeout support](#delayed-root-suite)
 - [test retry support](#retry-tests)
- [test-specific timeouts](#test-level) 
+- [test-specific timeouts](#test-level)


looks like the whitespace changes crept back in. that other PR has been merged, so if there are no conflicts then this is fine. please rebase onto master if you haven't done so yet

boneskull

thanks for the updates; was hoping you could address a few more issues I've found.

boneskull · 2018-01-04T23:05:06Z

bin/_mocha

@@ -199,7 +207,8 @@ program
  .option('--delay', 'wait for async suite definition')
  .option('--allow-uncaught', 'enable uncaught errors to propagate')
  .option('--forbid-only', 'causes test marked with only to fail the suite')
-  .option('--forbid-pending', 'causes pending tests and test marked with skip to fail the suite');
+  .option('--forbid-pending', 'causes pending tests and test marked with skip to fail the suite')
+  .option('--file [file]', 'include a file to be ran during the suite', collect, []);


the parameter to --file is required, so I think this needs to be --file <file>.

boneskull · 2018-01-04T23:08:08Z

docs/index.md

@@ -850,6 +851,10 @@ Specifies the test-case timeout, defaulting to 2 seconds. To override you may pa

 Specify the "slow" test threshold, defaulting to 75ms. Mocha uses this to highlight test-cases that are taking too long.

+### `--file <file>`
+
+Add a file you want included first in a test suite. This is useful if you have some generic setup code that must be included within the test suite. The file passed is not effected by any other flags (`--recursive` or `--sort` have no effect). Accepts multiple `--file` flags to include multiple files.


--recursive does come into play, afaict; so it's only --sort that doesn't apply.

should be "is not affected" not "is not effected"

let's mention that the order in which the flags are specified is relevant, and

it can be used in mocha.opts

I'm not seeing recursive effect. On line 476 it's taking the args passed in and then iterating over those values and applying recursive to them. Am I missing something?

boneskull · 2018-01-04T23:10:09Z

test/integration/options.spec.js

@@ -91,6 +93,29 @@ describe('options', function () {
    });
  });

+  describe('--file', function () {


can we also test multiple --file flag uses? this is basically a copy/paste of the test on L101 w/o the first parameter to runMochaJSON; instead, appending to args.

you may need to monkey with runMochaJSON to optionally accept an array as the first argument...

Added an individual additional test that should provide coverage for this.

…ia --file Fixes #3181

hswolff · 2018-01-05T01:44:46Z

RFAL.

coveralls · 2018-01-05T02:33:15Z

Coverage increased (+0.05%) to 90.018% when pulling 0008924 on hswolff:add-file-arg into a723b8f on mochajs:master.

boneskull · 2018-01-16T20:43:36Z

Awesome, thanks.

…ia --file (mochajs#3190) * Add ability to pass in test files to be ran before positional files via --file Fixes mochajs#3181

boneskull suggested changes Jan 3, 2018

View reviewed changes

boneskull added the semver-minor implementation requires increase of "minor" version number; "features" label Jan 3, 2018

ljharb approved these changes Jan 4, 2018

View reviewed changes

boneskull reviewed Jan 4, 2018

View reviewed changes

boneskull suggested changes Jan 4, 2018

View reviewed changes

hswolff added 4 commits January 4, 2018 20:44

Add ability to pass in test files to be ran before positional files v…

68b454d

…ia --file Fixes #3181

revert markdown changes

666e5f0

PR Feedback

3ba35d6

PR Feedback

0008924

boneskull approved these changes Jan 16, 2018

View reviewed changes

boneskull merged commit 50aec7a into mochajs:master Jan 16, 2018

hswolff deleted the add-file-arg branch January 16, 2018 20:50

boneskull added this to the v5.0.0 milestone Jan 17, 2018

boneskull added the type: feature enhancement proposal label Jan 17, 2018

renovate bot mentioned this pull request Feb 14, 2018

Update dependency mocha to v5 Urigo/tortilla#64

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to pass in test files to be ran before positional files via --file #3190

Add ability to pass in test files to be ran before positional files via --file #3190

hswolff commented Jan 3, 2018

jsf-clabot commented Jan 3, 2018 •

edited

Loading

coveralls commented Jan 3, 2018

boneskull left a comment

boneskull Jan 3, 2018 •

edited

Loading

boneskull Jan 3, 2018

boneskull Jan 3, 2018

hswolff Jan 4, 2018

boneskull Jan 3, 2018

hswolff Jan 4, 2018

boneskull Jan 3, 2018

boneskull Jan 3, 2018

boneskull Jan 3, 2018

hswolff Jan 4, 2018

boneskull Jan 3, 2018

boneskull Jan 3, 2018

boneskull commented Jan 3, 2018

boneskull commented Jan 3, 2018

hswolff commented Jan 4, 2018

ljharb Jan 4, 2018

hswolff Jan 4, 2018

boneskull Jan 4, 2018

coveralls commented Jan 4, 2018

boneskull Jan 4, 2018

boneskull left a comment

boneskull Jan 4, 2018

boneskull Jan 4, 2018

hswolff Jan 5, 2018

boneskull Jan 4, 2018

hswolff Jan 5, 2018

hswolff commented Jan 5, 2018

coveralls commented Jan 5, 2018

boneskull commented Jan 16, 2018

Add ability to pass in test files to be ran before positional files via --file #3190

Add ability to pass in test files to be ran before positional files via --file #3190

Conversation

hswolff commented Jan 3, 2018

jsf-clabot commented Jan 3, 2018 • edited Loading

coveralls commented Jan 3, 2018

boneskull left a comment

Choose a reason for hiding this comment

boneskull Jan 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boneskull commented Jan 3, 2018

boneskull commented Jan 3, 2018

hswolff commented Jan 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jan 4, 2018

Choose a reason for hiding this comment

boneskull left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hswolff commented Jan 5, 2018

coveralls commented Jan 5, 2018

boneskull commented Jan 16, 2018

jsf-clabot commented Jan 3, 2018 •

edited

Loading

boneskull Jan 3, 2018 •

edited

Loading