Merge pull request #362 from step2yeung/master

ensure browserExitHandler is called for global errors

lib/commands/exam.js

@@ -282,7 +282,19 @@ module.exports = TestCommand.extend({
       );
     };
     const browserFailureHandler = function() {
-      if (commands.get('writeExecutionFile')) {
+      // browserFailureHandler is called for disconnect, processError or processExit events.
+      // disconnect and processExit events is fired during global error and successful test runs.
+      // On successful test runs, browserExitHandler should already be called. And is unnecessary
+      // to call it again, so we should return. This is covered by this.finish = true
+      // On global failure cases, it's possible that this.finish is also true. So we must check
+      // the timers set by onProcessExit
+      // https://github.com/testem/testem/blob/master/lib/runners/browser_test_runner.js#L266
+      // or onProcessError in testem.
+      // https://github.com/testem/testem/blob/master/lib/runners/browser_test_runner.js#L252
+      // If either timers is set, we should record the failed browser and call browserExitHandler
+      if (this.finished && (!this.onProcessExitTimer && !this.pendingTimer)) {
+        return;
+      } else if (commands.get('writeExecutionFile')) {
         const browserId = getBrowserId(this.launcher.settings.test_page);
         testemEvents.recordFailedBrowserId(browserId);
       }

node-tests/acceptance/exam-test.js

@@ -281,16 +281,25 @@ describe('Acceptance | Exam Command', function() {
   describe('Load Balance', function() {
     const unlinkFiles = [];
 
-    function assertTestExecutionJson(output) {
-      let testExecutionJsonPath = path.join(
+    function assertTestExecutionFailedBrowsers(output, numberOfFailedBrowsers) {
+      const testExecutionPath = path.join(
         process.cwd(),
         output.match(/test-execution-([0-9]*).json/g)[0]
       );
+      unlinkFiles.push(testExecutionPath);
+
       assert.ok(
-        fs.existsSync(testExecutionJsonPath),
+        fs.existsSync(testExecutionPath),
         'test execution json written to root'
       );
-      unlinkFiles.push(testExecutionJsonPath);
+
+      const testExecutionFile = fs.readJsonSync(testExecutionPath);
+
+      assert.equal(
+        testExecutionFile.failedBrowsers.length,
+        numberOfFailedBrowsers,
+        'failed browsers array is correctly recorded'
+      );
     }
 
     afterEach(() => {
@@ -326,7 +335,8 @@ describe('Acceptance | Exam Command', function() {
         '--parallel'
       ]).then(child => {
         const output = child.stdout;
-        assertTestExecutionJson(output);
+
+        assertTestExecutionFailedBrowsers(output, 0);
         assertOutput(output, 'Browser Id', [1]);
         assert.equal(
           getNumberOfTests(output),
@@ -347,7 +357,8 @@ describe('Acceptance | Exam Command', function() {
         '--write-execution-file'
       ]).then(child => {
         const output = child.stdout;
-        assertTestExecutionJson(output);
+
+        assertTestExecutionFailedBrowsers(output, 0);
         assertOutput(output, 'Browser Id', [1, 2, 3]);
         assert.equal(
           getNumberOfTests(output),
@@ -372,7 +383,8 @@ describe('Acceptance | Exam Command', function() {
         '--write-execution-file'
       ]).then(child => {
         const output = child.stdout;
-        assertTestExecutionJson(output);
+
+        assertTestExecutionFailedBrowsers(output, 0);
         assertOutput(output, 'Exam Partition', [1], [2]);
         assertOutput(output, 'Browser Id', [1, 2, 3]);
         assert.ok(
@@ -417,13 +429,15 @@ describe('Acceptance | Exam Command', function() {
           '2',
           '--write-execution-file'
         ]).then(assertExpectRejection, error => {
+          const output = error.message;
+
           assert.ok(
-            error.message.includes(
+            output.includes(
               'Error: Browser exited on request from test driver'
             ),
-            `browser exited during the test execution:\n${error.message}`
+            `browser exited during the test execution:\n${output}`
           );
-          assertTestExecutionJson(error.message);
+          assertTestExecutionFailedBrowsers(output, 1);
         });
       });
     });