Repository: ember-cli/ember-cli
Pull request #9070 (2020-02-28 05:34:25)

fail tests on unhandled rejections
prep work to address https://github.com/ember-cli/ember-cli/issues/9007

- [x] fix all failures

stefanpenner: fix

```
not ok 1233 open-editor spawns EDITOR with passed file
  Cannot read property 'on' of undefined
  TypeError: Cannot read property 'on' of undefined
      at /home/runner/work/ember-cli/ember-cli/lib/utilities/open-editor.js:19:17
      at new Promise (<anonymous>)
      at openEditor (lib/utilities/open-editor.js:18:10)
      at Context.<anonymous> (tests/unit/utilities/open-editor-test.js:27:5)
```

Patch:
diff --git a/README.md b/README.md
index f97d41138..17b4a3039 100644
--- a/README.md
+++ b/README.md
@@ -49,8 +49,8 @@ Usage
 After installation the `ember` CLI tool will be available to you. It is the
 entrypoint for all the functionality mentioned above.
 
-You can call `ember <command> --help` to find out more about [all of the
-following commands](https://cli.emberjs.com/release/basic-use/cli-commands/) or visit <https://cli.emberjs.com/release/> to read
+You can call `ember <command> --help` to find out more about all of the
+following commands or visit <https://cli.emberjs.com/release/> to read
 the in-depth documentation.
 
 
diff --git a/lib/tasks/addon-install.js b/lib/tasks/addon-install.js
index 993f5333c..3cf631bf0 100644
--- a/lib/tasks/addon-install.js
+++ b/lib/tasks/addon-install.js
@@ -16,7 +16,7 @@ class AddonInstallTask extends Task {
   run(options) {
     const chalk = require('chalk');
     let ui = this.ui;
-    let packageNames = options.packages;
+    let packageNames = options.packages || [];
     let blueprintOptions = options.blueprintOptions || {};
     let commandOptions = blueprintOptions;
 
diff --git a/lib/tasks/server/middleware/history-support/index.js b/lib/tasks/server/middleware/history-support/index.js
index 673907563..3d31dc14a 100644
--- a/lib/tasks/server/middleware/history-support/index.js
+++ b/lib/tasks/server/middleware/history-support/index.js
@@ -47,11 +47,13 @@ class HistorySupportAddon {
 
     let baseURL = options.rootURL === '' ? '/' : cleanBaseURL(options.rootURL || options.baseURL);
 
-    app.use((req, res, next) => {
-      const results = watcher.then(results => {
+    app.use(async (req, _, next) => {
+      try {
+        const results = await watcher;
         if (this.shouldHandleRequest(req, options)) {
           let assetPath = req.path.slice(baseURL.length);
           let isFile = false;
+
           try {
             isFile = fs.statSync(path.join(results.directory, assetPath)).isFile();
           } catch (err) {
@@ -61,9 +63,9 @@ class HistorySupportAddon {
             req.serveUrl = `${baseURL}index.html`;
           }
         }
-      });
-
-      Promise.resolve(results).finally(next);
+      } finally {
+        next();
+      }
     });
   }
 
diff --git a/lib/utilities/open-editor.js b/lib/utilities/open-editor.js
index a7a7a5f7d..2fa869fd5 100644
--- a/lib/utilities/open-editor.js
+++ b/lib/utilities/open-editor.js
@@ -13,14 +13,17 @@ function openEditor(file) {
 
   let editorArgs = openEditor._env().EDITOR.split(' ');
   let editor = editorArgs.shift();
-  let editProcess = openEditor._spawn(editor, [file].concat(editorArgs), { stdio: 'inherit' });
+  const args = [file].concat(editorArgs);
+  let editProcess = openEditor._spawn(editor, args, { stdio: 'inherit' });
 
   return new Promise((resolve, reject) => {
     editProcess.on('close', code => {
       if (code === 0) {
         resolve();
       } else {
-        reject();
+        reject(
+          new Error(`Spawn('${editor}', [${args.join(',')}]) exited with a non zero error status code: '${code}'`)
+        );
       }
     });
   });
diff --git a/package.json b/package.json
index 92be5e984..832ca743b 100644
--- a/package.json
+++ b/package.json
@@ -30,11 +30,11 @@
     "docs": "yuidoc",
     "lint": "eslint . --cache",
     "prepack": "yarn docs",
-    "test": "node tests/runner",
-    "test:all": "node tests/runner all",
+    "test": "node --unhandled-rejections=strict tests/runner",
+    "test:all": "node --unhandled-rejections=strict tests/runner all",
     "test:cover": "nyc node tests/runner all",
-    "test:slow": "node tests/runner slow",
-    "test:debug": "node debug tests/runner"
+    "test:slow": "node --unhandled-rejections=strict tests/runner slow",
+    "test:debug": "node --unhandled-rejections=strict debug tests/runner"
   },
   "dependencies": {
     "@babel/core": "^7.8.4",


Test patch:
diff --git a/lib/tasks/server/middleware/tests-server/index.js b/lib/tasks/server/middleware/tests-server/index.js
index bceef7698..5a87c6a84 100644
--- a/lib/tasks/server/middleware/tests-server/index.js
+++ b/lib/tasks/server/middleware/tests-server/index.js
@@ -5,7 +5,7 @@ const path = require('path');
 const fs = require('fs');
 const logger = require('heimdalljs-logger')('ember-cli:test-server');
 
-class TestsServerAddon {
+module.exports = class TestsServerAddon {
   /**
    * This addon is used to serve the QUnit or Mocha test runner
    * at `baseURL + '/tests'`.
@@ -26,44 +26,52 @@ class TestsServerAddon {
     let baseURL = options.rootURL === '' ? '/' : cleanBaseURL(options.rootURL || options.baseURL);
     let testsPath = `${baseURL}tests`;
 
-    app.use((req, res, next) => {
-      const results = watcher.then(results => {
-        let acceptHeaders = req.headers.accept || [];
-        let hasHTMLHeader = acceptHeaders.indexOf('text/html') !== -1;
-        let hasWildcardHeader = acceptHeaders.indexOf('*/*') !== -1;
+    app.use(async (req, _, next) => {
+      let results;
+      let watcherSuccess;
 
-        let isForTests = req.path.indexOf(testsPath) === 0;
+      try {
+        results = await watcher;
+        watcherSuccess = true;
+      } catch (e) {
+        watcherSuccess = false;
+        // the build has failed, the middleware can safely be skipped.
+        // build error reporting is handled by:
+        //   1. watcher production stderr
+        //   2. watcher-middleware serving the error page
+      }
 
-        logger.info('isForTests: %o', isForTests);
+      if (watcherSuccess) {
+        rewriteRequestUrlIfBasePageIsNeeded(req, testsPath, baseURL, results.directory);
+      }
 
-        if (isForTests && (hasHTMLHeader || hasWildcardHeader) && req.method === 'GET') {
-          let assetPath = req.path.slice(baseURL.length);
-          let filePath = path.join(results.directory, assetPath);
+      next();
 
-          if (!fs.existsSync(filePath) || !fs.statSync(filePath).isFile()) {
-            // N.B., `baseURL` will end with a slash as it went through `cleanBaseURL`
-            let newURL = `${baseURL}tests/index.html`;
+      if (config.finally) {
+        config.finally();
+      }
+    });
+  }
+};
 
-            logger.info(
-              'url: %s resolved to path: %s which is not a file. Assuming %s instead',
-              req.path,
-              filePath,
-              newURL
-            );
-            req.url = newURL;
-          }
-        }
-      });
+function rewriteRequestUrlIfBasePageIsNeeded(req, testsPath, baseURL, directory) {
+  let acceptHeaders = req.headers.accept || [];
+  let hasHTMLHeader = acceptHeaders.indexOf('text/html') !== -1;
+  let hasWildcardHeader = acceptHeaders.indexOf('*/*') !== -1;
 
-      Promise.resolve(results)
-        .finally(next)
-        .finally(() => {
-          if (config.finally) {
-            config.finally();
-          }
-        });
-    });
+  let isForTests = req.path.indexOf(testsPath) === 0;
+
+  logger.info('isForTests: %o', isForTests);
+
+  if (isForTests && (hasHTMLHeader || hasWildcardHeader) && req.method === 'GET') {
+    let assetPath = req.path.slice(baseURL.length);
+    let filePath = path.join(directory, assetPath);
+
+    if (!fs.existsSync(filePath) || !fs.statSync(filePath).isFile()) {
+      // N.B., `baseURL` will end with a slash as it went through `cleanBaseURL`
+      let newURL = `${baseURL}tests/index.html`;
+      logger.info('url: %s resolved to path: %s which is not a file. Assuming %s instead', req.path, filePath, newURL);
+      req.url = newURL;
+    }
   }
 }
-
-module.exports = TestsServerAddon;
diff --git a/tests/integration/models/blueprint-test.js b/tests/integration/models/blueprint-test.js
index 557002983..9711a0fa2 100644
--- a/tests/integration/models/blueprint-test.js
+++ b/tests/integration/models/blueprint-test.js
@@ -12,7 +12,7 @@ const util = require('util');
 
 const EOL = require('os').EOL;
 let root = process.cwd();
-let tmproot = path.join(root, 'tmp');
+let tempRoot = path.join(root, 'tmp');
 const SilentError = require('silent-error');
 const mkTmpDirIn = require('../../../lib/utilities/mk-tmp-dir-in');
 const td = require('testdouble');
@@ -244,7 +244,7 @@ describe('Blueprint', function() {
     let tmpdir;
 
     beforeEach(async function() {
-      const dir = await mkTmpDirIn(tmproot);
+      const dir = await mkTmpDirIn(tempRoot);
       tmpdir = dir;
       blueprint = new InstrumentedBasicBlueprint(basicBlueprint);
       ui = new MockUI();
@@ -258,8 +258,8 @@ describe('Blueprint', function() {
       };
     });
 
-    afterEach(function() {
-      return remove(tmproot);
+    afterEach(async function() {
+      await remove(tempRoot);
     });
 
     it('installs basic files', async function() {
@@ -599,7 +599,7 @@ describe('Blueprint', function() {
     }
 
     beforeEach(async function() {
-      let dir = await mkTmpDirIn(tmproot);
+      let dir = await mkTmpDirIn(tempRoot);
 
       tmpdir = dir;
       blueprint = new BasicBlueprintClass(basicBlueprint);
@@ -614,8 +614,8 @@ describe('Blueprint', function() {
       refreshUI();
     });
 
-    afterEach(function() {
-      return remove(tmproot);
+    afterEach(async function() {
+      await remove(tempRoot);
     });
 
     it('uninstalls basic files', async function() {
@@ -637,9 +637,7 @@ describe('Blueprint', function() {
 
       expect(actualFiles.length).to.equal(0);
 
-      fs.exists(path.join(tmpdir, 'test.txt'), function(exists) {
-        expect(exists).to.be.false;
-      });
+      expect(fs.existsSync(path.join(tmpdir, 'test.txt'))).to.be.false;
     });
 
     it("uninstall doesn't remove non-empty folders", async function() {
@@ -683,7 +681,7 @@ describe('Blueprint', function() {
     }
 
     beforeEach(async function() {
-      let dir = await mkTmpDirIn(tmproot);
+      let dir = await mkTmpDirIn(tempRoot);
       tmpdir = dir;
       blueprint = new InstrumentedBasicBlueprint(basicBlueprint);
       project = new MockProject();
@@ -693,7 +691,7 @@ describe('Blueprint', function() {
       };
       refreshUI();
       await blueprint.install(options);
-      await resetCalled();
+      resetCalled();
       refreshUI();
     });
 
@@ -752,8 +750,8 @@ describe('Blueprint', function() {
       };
     });
 
-    afterEach(function() {
-      return remove(tmproot);
+    afterEach(async function() {
+      await remove(tempRoot);
     });
 
     it('looks up the `npm-install` task', function() {
@@ -882,8 +880,8 @@ describe('Blueprint', function() {
       };
     });
 
-    afterEach(function() {
-      return remove(tmproot);
+    afterEach(async function() {
+      await remove(tempRoot);
     });
 
     it('looks up the `npm-uninstall` task', function() {
@@ -922,7 +920,7 @@ describe('Blueprint', function() {
     });
 
     afterEach(function() {
-      return remove(tmproot);
+      return remove(tempRoot);
     });
 
     it('looks up the `npm-uninstall` task', function() {
@@ -1098,8 +1096,8 @@ describe('Blueprint', function() {
       };
     });
 
-    afterEach(function() {
-      return remove(tmproot);
+    afterEach(async function() {
+      await remove(tempRoot);
     });
 
     it('passes a packages array for addBowerPackagesToProject', function() {
@@ -1150,8 +1148,8 @@ describe('Blueprint', function() {
       };
     });
 
-    afterEach(function() {
-      return remove(tmproot);
+    afterEach(async function() {
+      await remove(tempRoot);
     });
 
     it('looks up the `bower-install` task', function() {
@@ -1244,8 +1242,8 @@ describe('Blueprint', function() {
       blueprint = new Blueprint(basicBlueprint);
     });
 
-    afterEach(function() {
-      return remove(tmproot);
+    afterEach(async function() {
+      await remove(tempRoot);
     });
 
     it('passes a packages array for addAddonsToProject', function() {
@@ -1280,8 +1278,8 @@ describe('Blueprint', function() {
       };
     });
 
-    afterEach(function() {
-      return remove(tmproot);
+    afterEach(async function() {
+      await remove(tempRoot);
     });
 
     it('looks up the `addon-install` task', function() {
@@ -1409,7 +1407,7 @@ describe('Blueprint', function() {
     let project;
 
     beforeEach(async function() {
-      let dir = await mkTmpDirIn(tmproot);
+      let dir = await mkTmpDirIn(tempRoot);
       tmpdir = dir;
       blueprint = new Blueprint(basicBlueprint);
       project = new MockProject();
@@ -1421,8 +1419,8 @@ describe('Blueprint', function() {
       };
     });
 
-    afterEach(function() {
-      return remove(tmproot);
+    afterEach(async function() {
+      await remove(tempRoot);
     });
 
     it('can lookup other Blueprints from the project blueprintLookupPaths', function() {
diff --git a/tests/runner.js b/tests/runner.js
index 439cb29a0..203805161 100644
--- a/tests/runner.js
+++ b/tests/runner.js
@@ -1,5 +1,4 @@
 'use strict';
-
 const captureExit = require('capture-exit');
 captureExit.captureExit();
 
diff --git a/tests/unit/docs-lint-test.js b/tests/unit/docs-lint-test.js
index 4a600c5e2..4e7d35a67 100644
--- a/tests/unit/docs-lint-test.js
+++ b/tests/unit/docs-lint-test.js
@@ -1,45 +1,61 @@
 'use strict';
 
+/*
+ * Turns out, YUIDoc attached a process.on('unhandledRejection' handler which
+ * incorrectly reports any unhandledRejection as a YUIDoc error. This makes
+ * debugging of unhandledRejections in the ember-cli test suite quite painful
+ *
+ * To address YUIDocs behavior, we run YUIDoc isolated in it's own process;
+ */
+if (!process.env['IS_CHILD_THREAD']) {
+  const execa = require('execa');
+
+  describe('YUIDoc', function() {
+    it('parses without warnings', async function() {
+      await execa('node', [`--unhandled-rejections=strict`, __filename], {
+        env: {
+          IS_CHILD_THREAD: true,
+        },
+      });
+    });
+  });
+  return;
+}
+
 const Y = require('yuidocjs');
 const EOL = require('os').EOL;
 
-describe('YUIDoc', function() {
-  let options = Y.Project.init({
-    quiet: true,
-  });
-  let yuiDoc = new Y.YUIDoc(options);
-
-  it('parses without warnings', function() {
-    let json = yuiDoc.run();
+let options = Y.Project.init({
+  quiet: true,
+});
+let yuiDoc = new Y.YUIDoc(options);
 
-    let warnings = {};
-    json.warnings.forEach(function(warning) {
-      let tmp = warning.line.split(':');
-      let file = tmp[0].trim();
-      let line = tmp[1];
+let json = yuiDoc.run();
 
-      if (!warnings[file]) {
-        warnings[file] = [];
-      }
+let warnings = {};
+json.warnings.forEach(function(warning) {
+  let tmp = warning.line.split(':');
+  let file = tmp[0].trim();
+  let line = tmp[1];
 
-      warnings[file].push({
-        line,
-        message: warning.message,
-      });
-    });
+  if (!warnings[file]) {
+    warnings[file] = [];
+  }
 
-    let message = '';
-    Object.keys(warnings).forEach(function(file) {
-      message += `\t${file} – YUIDoc issues found:${EOL}${EOL}`;
-      warnings[file].forEach(function(warning) {
-        message += `\t\tline ${warning.line}: ${warning.message}${EOL}`;
-      });
-    });
+  warnings[file].push({
+    line,
+    message: warning.message,
+  });
+});
 
-    if (message.length) {
-      let error = new Error(message);
-      delete error.stack;
-      throw error;
-    }
+let message = '';
+Object.keys(warnings).forEach(function(file) {
+  message += `\t${file} – YUIDoc issues found:${EOL}${EOL}`;
+  warnings[file].forEach(function(warning) {
+    message += `\t\tline ${warning.line}: ${warning.message}${EOL}`;
   });
 });
+
+if (message.length) {
+  throw new Error(message);
+}
diff --git a/tests/unit/tasks/addon-install-test.js b/tests/unit/tasks/addon-install-test.js
index 06b0836cc..66eed5d73 100644
--- a/tests/unit/tasks/addon-install-test.js
+++ b/tests/unit/tasks/addon-install-test.js
@@ -3,14 +3,16 @@
 const AddonInstallTask = require('../../../lib/tasks/addon-install');
 const expect = require('chai').expect;
 const CoreObject = require('core-object');
+const MockUi = require('console-ui/mock');
 
 describe('addon install task', function() {
   let ui;
   let project;
 
   beforeEach(function() {
-    ui = {
-      startProgress() {},
+    ui = new MockUi();
+    project = {
+      reloadAddons() {},
     };
   });
 
diff --git a/tests/unit/tasks/git-init-test.js b/tests/unit/tasks/git-init-test.js
index 104d29bdc..251634a96 100644
--- a/tests/unit/tasks/git-init-test.js
+++ b/tests/unit/tasks/git-init-test.js
@@ -63,7 +63,6 @@ describe('git-init', function() {
     td.when(task._gitVersion()).thenReject();
 
     await task.run();
-    td.verify(task._gitVersion(), { times: 1 });
     td.verify(task._gitInit(), { times: 0 });
     td.verify(task._gitAdd(), { times: 0 });
     td.verify(task._gitCommit(td.matchers.anything()), { times: 0 });
diff --git a/tests/unit/tasks/server/livereload-server-test.js b/tests/unit/tasks/server/livereload-server-test.js
index c0bd42799..0744e04f2 100644
--- a/tests/unit/tasks/server/livereload-server-test.js
+++ b/tests/unit/tasks/server/livereload-server-test.js
@@ -60,24 +60,22 @@ describe('livereload-server', function() {
           expect(!!subject.liveReloadServer).to.equal(false);
         });
     });
-    it('informs of error during startup with custom port', function(done) {
+    it('informs of error during startup with custom port', async function() {
       let preexistingServer = net.createServer();
       preexistingServer.listen(1337);
-      subject
-        .setupMiddleware({
+
+      try {
+        await subject.setupMiddleware({
           liveReload: true,
           liveReloadPort: 1337,
           liveReloadPrefix: '/',
           port: 4200,
-        })
-        .catch(function(reason) {
-          expect(reason).to.equal(
-            `Livereload failed on http://localhost:1337.  It is either in use or you do not have permission.`
-          );
-        })
-        .finally(function() {
-          preexistingServer.close(done);
         });
+      } catch (e) {
+        expect(e.message).to.include(`Livereload failed on 'http://localhost:1337', It may be in use.`);
+      } finally {
+        await new Promise(resolve => preexistingServer.close(resolve));
+      }
     });
     it('starts with custom host, custom port', function() {
       return subject
@@ -126,28 +124,29 @@ describe('livereload-server', function() {
           expect(subject.liveReloadServer.options.cert).to.be.an.instanceof(Buffer);
         });
     });
-    it('informs of error during startup', function(done) {
+
+    it('informs of error during startup', async function() {
       let preexistingServer = net.createServer();
       preexistingServer.listen(1337);
 
-      subject
-        .setupMiddleware({
+      try {
+        await subject.setupMiddleware({
           liveReloadPort: 1337,
           liveReload: true,
           ssl: true,
           sslKey: 'tests/fixtures/ssl/server.key',
           sslCert: 'tests/fixtures/ssl/server.crt',
           port: 4200,
-        })
-        .catch(function(reason) {
-          expect(reason).to.equal(
-            `Livereload failed on https://localhost:1337.  It is either in use or you do not have permission.`
-          );
-        })
-        .finally(function() {
-          preexistingServer.close(done);
         });
+
+        expect.fail('expected rejection');
+      } catch (e) {
+        expect(e.message).to.include(`Livereload failed on 'https://localhost:1337', It may be in use.`);
+      } finally {
+        await new Promise(resolve => preexistingServer.close(resolve));
+      }
     });
+
     it('correctly runs in https mode with custom port', function() {
       return subject
         .setupMiddleware({
diff --git a/tests/unit/tasks/server/middleware/tests-server-test.js b/tests/unit/tasks/server/middleware/tests-server-test.js
index 582ec84a1..3591ef11b 100644
--- a/tests/unit/tasks/server/middleware/tests-server-test.js
+++ b/tests/unit/tasks/server/middleware/tests-server-test.js
@@ -29,7 +29,9 @@ describe('TestServerAddon', function() {
       addon.serverMiddleware({
         app,
         options: {
-          watcher: Promise.resolve(),
+          watcher: Promise.resolve({
+            directory: 'some-output-directory',
+          }),
         },
         finally() {
           try {
diff --git a/tests/unit/utilities/open-editor-test.js b/tests/unit/utilities/open-editor-test.js
index e8a0e005b..cbb3493bd 100644
--- a/tests/unit/utilities/open-editor-test.js
+++ b/tests/unit/utilities/open-editor-test.js
@@ -15,24 +15,54 @@ describe('open-editor', function() {
     td.reset();
   });
 
-  it('throws if EDITOR is not set', function() {
+  it('throws if EDITOR is not set', async function() {
     td.when(openEditor._env()).thenReturn({});
-    expect(() => {
-      openEditor('test');
-    }).to.throw('EDITOR environment variable is not set');
+    try {
+      await openEditor('test');
+      expect.fail('expected rejection');
+    } catch (e) {
+      expect(e.message).to.eql('EDITOR environment variable is not set');
+    }
   });
 
-  it('spawns EDITOR with passed file', function() {
+  it('spawns EDITOR with passed file', async function() {
     td.when(openEditor._env()).thenReturn({ EDITOR: 'vi' });
-    openEditor('test');
+    td.when(openEditor._spawn(td.matchers.anything(), td.matchers.anything(), td.matchers.anything())).thenReturn({
+      on(event, cb) {
+        cb(0);
+      },
+    });
+
+    await openEditor('test');
     td.verify(openEditor._spawn('vi', ['test'], { stdio: 'inherit' }));
   });
 
-  it('throws if no file option is provided', function() {
+  it('spawns EDITOR with passed file (rejection scenario)', async function() {
     td.when(openEditor._env()).thenReturn({ EDITOR: 'vi' });
-    expect(() => {
-      openEditor();
-    }).to.throw('No `file` option provided');
+    td.when(openEditor._spawn(td.matchers.anything(), td.matchers.anything(), td.matchers.anything())).thenReturn({
+      on(event, cb) {
+        cb(-1);
+      },
+    });
+
+    try {
+      await openEditor('test');
+      expect.fail('expected rejection');
+    } catch (e) {
+      expect(e.message).to.include(`exited with a non zero error status code: '-1'`);
+    }
+
+    td.verify(openEditor._spawn('vi', ['test'], { stdio: 'inherit' }));
+  });
+  it('throws if no file option is provided', async function() {
+    td.when(openEditor._env()).thenReturn({ EDITOR: 'vi' });
+
+    try {
+      await openEditor();
+      expect.fail('expected rejection');
+    } catch (e) {
+      expect(e.message).to.eql('No `file` option provided');
+    }
   });
 
   describe('.canEdit()', function() {