fail tests on unhandled rejections

(cherry picked from commit d564247)
ember-cli · Mar 13, 2020 · c30ed27 · c30ed27
1 parent 3c0e529
commit c30ed27
Show file tree

Hide file tree

Showing 13 changed files with 202 additions and 140 deletions.
diff --git a/lib/tasks/addon-install.js b/lib/tasks/addon-install.js
@@ -17,7 +17,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
@@ -2,7 +2,6 @@
 
 const path = require('path');
 const fs = require('fs');
-const promiseFinally = require('promise.prototype.finally');
 const cleanBaseURL = require('clean-base-url');
 
 class HistorySupportAddon {
@@ -48,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) {
@@ -62,9 +63,9 @@ class HistorySupportAddon {
             req.serveUrl = `${baseURL}index.html`;
           }
         }
-      });
-
-      promiseFinally(Promise.resolve(results), next);
+      } finally {
+        next();
+      }
     });
   }
 

diff --git a/lib/tasks/server/middleware/tests-server/index.js b/lib/tasks/server/middleware/tests-server/index.js
@@ -4,9 +4,8 @@ const cleanBaseURL = require('clean-base-url');
 const path = require('path');
 const fs = require('fs');
 const logger = require('heimdalljs-logger')('ember-cli:test-server');
-const promiseFinally = require('promise.prototype.finally');
 
-class TestsServerAddon {
+module.exports = class TestsServerAddon {
   /**
    * This addon is used to serve the QUnit or Mocha test runner
    * at `baseURL + '/tests'`.
@@ -27,42 +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;
 
-      promiseFinally(promiseFinally(Promise.resolve(results), next), () => {
-        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/lib/utilities/open-editor.js b/lib/utilities/open-editor.js
@@ -14,14 +14,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
@@ -29,6 +29,7 @@
   "scripts": {
     "docs": "yuidoc",
     "lint": "eslint . --cache",
+    "prepack": "yarn docs",
     "test": "node tests/runner",
     "test:all": "node tests/runner all",
     "test:cover": "nyc node tests/runner all",

diff --git 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
@@ -1,5 +1,7 @@
 'use strict';
-
+// process.on('unhandledRejection', e => {
+//   throw e;
+// });
 const captureExit = require('capture-exit');
 captureExit.captureExit();