fix bad error message when incorrectly using ESM

- re-enable parallel-mode root hook plugin tests - refactor root hook plugin tests to avoid flake Signed-off-by: Christopher Hiller <boneskull@boneskull.com>
mochajs · Jun 5, 2020 · 9881ace · 9881ace
1 parent 63eb80b
commit 9881ace
Show file tree

Hide file tree

Showing 4 changed files with 189 additions and 40 deletions.
diff --git a/lib/cli/cli.js b/lib/cli/cli.js
@@ -49,10 +49,10 @@ exports.main = (argv = process.argv.slice(2)) => {
       'Commands:': 'Commands'
     })
     .fail((msg, err, yargs) => {
-      debug(err);
+      debug('caught error sometime before command handler: %O', err);
       yargs.showHelp();
       console.error(`\n${symbols.error} ${ansi.red('ERROR:')} ${msg}`);
-      process.exit(1);
+      yargs.exit(1);
     })
     .help('help', 'Show usage information & exit')
     .alias('help', 'h')

diff --git a/lib/cli/run.js b/lib/cli/run.js
@@ -7,6 +7,8 @@
  * @private
  */
 
+const symbols = require('log-symbols');
+const ansi = require('ansi-colors');
 const Mocha = require('../mocha');
 const {
   createUnsupportedError,
@@ -333,14 +335,21 @@ exports.builder = yargs =>
 
       return true;
     })
-    .middleware(async argv => {
-      // load requires first, because it can impact "plugin" validation
-      const rawRootHooks = await handleRequires(argv.require);
-      validatePlugin(argv, 'reporter', Mocha.reporters);
-      validatePlugin(argv, 'ui', Mocha.interfaces);
+    .middleware(async (argv, yargs) => {
+      // currently a failing middleware does not work nicely with yargs' `fail()`.
+      try {
+        // load requires first, because it can impact "plugin" validation
+        const rawRootHooks = await handleRequires(argv.require);
+        validatePlugin(argv, 'reporter', Mocha.reporters);
+        validatePlugin(argv, 'ui', Mocha.interfaces);
 
-      if (rawRootHooks.length) {
-        argv.rootHooks = await loadRootHooks(rawRootHooks);
+        if (rawRootHooks && rawRootHooks.length) {
+          argv.rootHooks = await loadRootHooks(rawRootHooks);
+        }
+      } catch (err) {
+        // this could be a bad --require, bad reporter, ui, etc.
+        console.error(`\n${symbols.error} ${ansi.red('ERROR:')}`, err);
+        yargs.exit(1);
       }
     })
     .array(types.array)

diff --git a/test/integration/fixtures/options/require/root-hook-defs-esm-broken.fixture.js b/test/integration/fixtures/options/require/root-hook-defs-esm-broken.fixture.js
@@ -0,0 +1,8 @@
+export const mochaHooks = {
+  beforeAll() {
+    console.log('mjs beforeAll');
+  },
+  afterAll() {
+    console.log('mjs afterAll');
+  },
+};
diff --git a/test/integration/options/require.spec.js b/test/integration/options/require.spec.js
@@ -3,11 +3,38 @@
 var invokeMochaAsync = require('../helpers').invokeMochaAsync;
 var utils = require('../../../lib/utils');
 
+/**
+ * Extracts root hook log messages from run results
+ * `root-hook-defs-*` fixtures are root hook plugins which call `console.log()`
+ * for verification that they have been run.
+ * @param {RawResult} res - result of invokeMochaAsync()
+ */
+function extractHookOutputFromResult(res) {
+  return res.output
+    .trim()
+    .split('\n')
+    .filter(function(line) {
+      // every line that begins with whitespace (e.g., the test name) should be ignored;
+      // we just want the console.log messages
+      return /^\S/.test(line);
+    })
+    .sort();
+}
+
+/**
+ * Helper to call Mocha and pipe the result through `extractHookOutputFromResult`
+ * @param {*} args - args for invokeMochaAsync
+ * @param {*} opts - opts for invokeMochaAsync
+ */
+function runMochaForHookOutput(args, opts) {
+  return invokeMochaAsync(args, opts)[1].then(extractHookOutputFromResult);
+}
+
 describe('--require', function() {
   describe('when mocha run in serial mode', function() {
     it('should run root hooks when provided via mochaHooks object export', function() {
       return expect(
-        invokeMochaAsync([
+        runMochaForHookOutput([
           '--require=' +
             require.resolve(
               '../fixtures/options/require/root-hook-defs-a.fixture.js'
@@ -19,16 +46,28 @@ describe('--require', function() {
           require.resolve(
             '../fixtures/options/require/root-hook-test.fixture.js'
           )
-        ])[1],
-        'when fulfilled',
-        'to contain output',
-        /beforeAll[\s\S]+?beforeAll array 1[\s\S]+?beforeAll array 2[\s\S]+?beforeEach[\s\S]+?beforeEach array 1[\s\S]+?beforeEach array 2[\s\S]+?afterEach[\s\S]+?afterEach array 1[\s\S]+?afterEach array 2[\s\S]+?afterAll[\s\S]+?afterAll array 1[\s\S]+?afterAll array 2/
+        ]),
+        'to be fulfilled with',
+        [
+          'afterAll',
+          'afterAll array 1',
+          'afterAll array 2',
+          'afterEach',
+          'afterEach array 1',
+          'afterEach array 2',
+          'beforeAll',
+          'beforeAll array 1',
+          'beforeAll array 2',
+          'beforeEach',
+          'beforeEach array 1',
+          'beforeEach array 2'
+        ]
       );
     });
 
     it('should run root hooks when provided via mochaHooks function export', function() {
       return expect(
-        invokeMochaAsync([
+        runMochaForHookOutput([
           '--require=' +
             require.resolve(
               '../fixtures/options/require/root-hook-defs-c.fixture.js'
@@ -40,21 +79,33 @@ describe('--require', function() {
           require.resolve(
             '../fixtures/options/require/root-hook-test.fixture.js'
           )
-        ])[1],
-        'when fulfilled',
-        'to contain output',
-        /beforeAll[\s\S]+?beforeAll array 1[\s\S]+?beforeAll array 2[\s\S]+?beforeEach[\s\S]+?beforeEach array 1[\s\S]+?beforeEach array 2[\s\S]+?afterEach[\s\S]+?afterEach array 1[\s\S]+?afterEach array 2[\s\S]+?afterAll[\s\S]+?afterAll array 1[\s\S]+?afterAll array 2/
+        ]),
+        'to be fulfilled with',
+        [
+          'afterAll',
+          'afterAll array 1',
+          'afterAll array 2',
+          'afterEach',
+          'afterEach array 1',
+          'afterEach array 2',
+          'beforeAll',
+          'beforeAll array 1',
+          'beforeAll array 2',
+          'beforeEach',
+          'beforeEach array 1',
+          'beforeEach array 2'
+        ]
       );
     });
 
-    describe('support ESM when style=module or .mjs extension', function() {
+    describe('support ESM when type=module or .mjs extension', function() {
       before(function() {
         if (!utils.supportsEsModules()) this.skip();
       });
 
       it('should run root hooks when provided via mochaHooks', function() {
         return expect(
-          invokeMochaAsync(
+          runMochaForHookOutput(
             [
               '--require=' +
                 require.resolve(
@@ -79,22 +130,55 @@ describe('--require', function() {
                 ? []
                 : '--experimental-modules'
             )
+          ),
+          'to be fulfilled with',
+          [
+            'afterAll',
+            'afterEach',
+            'beforeAll',
+            'beforeEach',
+            'esm afterEach',
+            'esm beforeEach',
+            'mjs afterAll',
+            'mjs beforeAll'
+          ]
+        );
+      });
+    });
+
+    describe('support ESM via .js extension w/o type=module', function() {
+      before(function() {
+        if (!utils.supportsEsModules()) this.skip();
+      });
+
+      it('should fail due to ambiguous file type', function() {
+        return expect(
+          invokeMochaAsync(
+            [
+              '--require=' +
+                require.resolve(
+                  // as object
+                  '../fixtures/options/require/root-hook-defs-esm-broken.fixture.js'
+                )
+            ].concat(
+              +process.versions.node.split('.')[0] >= 13
+                ? []
+                : '--experimental-modules'
+            ),
+            'pipe'
           )[1],
           'when fulfilled',
           'to contain output',
-          /mjs beforeAll[\s\S]+?beforeAll[\s\S]+?esm beforeEach[\s\S]+?beforeEach[\s\S]+?esm afterEach[\s\S]+?afterEach[\s\S]+?mjs afterAll[\s\S]+?afterAll/
+          /SyntaxError: Unexpected token/
         );
       });
     });
   });
 
   describe('when mocha in parallel mode', function() {
-    before(function() {
-      this.skip(); // TODO: remove when #4245 lands
-    });
     it('should run root hooks when provided via mochaHooks object exports', function() {
       return expect(
-        invokeMochaAsync([
+        runMochaForHookOutput([
           '--require=' +
             require.resolve(
               '../fixtures/options/require/root-hook-defs-a.fixture.js'
@@ -107,16 +191,28 @@ describe('--require', function() {
           require.resolve(
             '../fixtures/options/require/root-hook-test.fixture.js'
           )
-        ])[1],
-        'when fulfilled',
-        'to contain output',
-        /beforeAll[\s\S]+?beforeAll array 1[\s\S]+?beforeAll array 2[\s\S]+?beforeEach[\s\S]+?beforeEach array 1[\s\S]+?beforeEach array 2[\s\S]+?afterEach[\s\S]+?afterEach array 1[\s\S]+?afterEach array 2[\s\S]+?afterAll[\s\S]+?afterAll array 1[\s\S]+?afterAll array 2/
+        ]),
+        'to be fulfilled with',
+        [
+          'afterAll',
+          'afterAll array 1',
+          'afterAll array 2',
+          'afterEach',
+          'afterEach array 1',
+          'afterEach array 2',
+          'beforeAll',
+          'beforeAll array 1',
+          'beforeAll array 2',
+          'beforeEach',
+          'beforeEach array 1',
+          'beforeEach array 2'
+        ]
       );
     });
 
     it('should run root hooks when provided via mochaHooks function export', function() {
       return expect(
-        invokeMochaAsync([
+        runMochaForHookOutput([
           '--require=' +
             require.resolve(
               '../fixtures/options/require/root-hook-defs-c.fixture.js'
@@ -129,17 +225,29 @@ describe('--require', function() {
           require.resolve(
             '../fixtures/options/require/root-hook-test.fixture.js'
           )
-        ])[1],
-        'when fulfilled',
-        'to contain output',
-        /beforeAll[\s\S]+?beforeAll array 1[\s\S]+?beforeAll array 2[\s\S]+?beforeEach[\s\S]+?beforeEach array 1[\s\S]+?beforeEach array 2[\s\S]+?afterEach[\s\S]+?afterEach array 1[\s\S]+?afterEach array 2[\s\S]+?afterAll[\s\S]+?afterAll array 1[\s\S]+?afterAll array 2/
+        ]),
+        'to be fulfilled with',
+        [
+          'afterAll',
+          'afterAll array 1',
+          'afterAll array 2',
+          'afterEach',
+          'afterEach array 1',
+          'afterEach array 2',
+          'beforeAll',
+          'beforeAll array 1',
+          'beforeAll array 2',
+          'beforeEach',
+          'beforeEach array 1',
+          'beforeEach array 2'
+        ]
       );
     });
 
     describe('when running multiple jobs', function() {
       it('should run root hooks when provided via mochaHooks object exports for each job', function() {
         return expect(
-          invokeMochaAsync([
+          runMochaForHookOutput([
             '--require=' +
               require.resolve(
                 '../fixtures/options/require/root-hook-defs-a.fixture.js'
@@ -155,10 +263,34 @@ describe('--require', function() {
             require.resolve(
               '../fixtures/options/require/root-hook-test-2.fixture.js'
             )
-          ])[1],
-          'when fulfilled',
-          'to contain output',
-          /(?:beforeAll[\s\S]+?beforeAll array 1[\s\S]+?beforeAll array 2[\s\S]+?beforeEach[\s\S]+?beforeEach array 1[\s\S]+?beforeEach array 2[\s\S]+?afterEach[\s\S]+?afterEach array 1[\s\S]+?afterEach array 2[\s\S]+?afterAll[\s\S]+?afterAll array 1[\s\S]+?afterAll array 2[\s\S]+?){2}/
+          ]),
+          'to be fulfilled with',
+          [
+            'afterAll',
+            'afterAll',
+            'afterAll array 1',
+            'afterAll array 1',
+            'afterAll array 2',
+            'afterAll array 2',
+            'afterEach',
+            'afterEach',
+            'afterEach array 1',
+            'afterEach array 1',
+            'afterEach array 2',
+            'afterEach array 2',
+            'beforeAll',
+            'beforeAll',
+            'beforeAll array 1',
+            'beforeAll array 1',
+            'beforeAll array 2',
+            'beforeAll array 2',
+            'beforeEach',
+            'beforeEach',
+            'beforeEach array 1',
+            'beforeEach array 1',
+            'beforeEach array 2',
+            'beforeEach array 2'
+          ]
         );
       });
     });