Fix HTML reporter regression causing duplicate error output (#2112)

Fixes #2083.

Author: danielstjules

lib/reporters/html.js

@@ -128,88 +128,82 @@ function HTML(runner) {
     stack.shift();
   });
 
-  runner.on('fail', function(test) {
-    // For type = 'test' its possible that the test failed due to multiple
-    // done() calls. So report the issue here.
-    if (test.type === 'hook'
-      || test.type === 'test') {
-      runner.emit('test end', test);
-    }
+  runner.on('pass', function(test) {
+    var url = self.testURL(test);
+    var markup = '<li class="test pass %e"><h2>%e<span class="duration">%ems</span> '
+      + '<a href="%s" class="replay">‣</a></h2></li>';
+    var el = fragment(markup, test.speed, test.title, test.duration, url);
+    self.addCodeToggle(el, test.body);
+    appendToStack(el);
+    updateStats();
   });
 
-  runner.on('test end', function(test) {
-    // TODO: add to stats
-    var percent = stats.tests / this.total * 100 | 0;
-    if (progress) {
-      progress.update(percent).draw(ctx);
+  runner.on('fail', function(test) {
+    var el = fragment('<li class="test fail"><h2>%e <a href="%e" class="replay">‣</a></h2></li>',
+      test.title, self.testURL(test));
+    var stackString; // Note: Includes leading newline
+    var message = test.err.toString();
+
+    // <=IE7 stringifies to [Object Error]. Since it can be overloaded, we
+    // check for the result of the stringifying.
+    if (message === '[object Error]') {
+      message = test.err.message;
     }
 
-    // update stats
-    var ms = new Date() - stats.start;
-    text(passes, stats.passes);
-    text(failures, stats.failures);
-    text(duration, (ms / 1000).toFixed(2));
-
-    // test
-    var el;
-    if (test.state === 'passed') {
-      var url = self.testURL(test);
-      el = fragment('<li class="test pass %e"><h2>%e<span class="duration">%ems</span> <a href="%s" class="replay">‣</a></h2></li>', test.speed, test.title, test.duration, url);
-    } else if (test.isPending()) {
-      el = fragment('<li class="test pass pending"><h2>%e</h2></li>', test.title);
-    } else {
-      el = fragment('<li class="test fail"><h2>%e <a href="%e" class="replay">‣</a></h2></li>', test.title, self.testURL(test));
-      var stackString; // Note: Includes leading newline
-      var message = test.err.toString();
-
-      // <=IE7 stringifies to [Object Error]. Since it can be overloaded, we
-      // check for the result of the stringifying.
-      if (message === '[object Error]') {
-        message = test.err.message;
-      }
-
-      if (test.err.stack) {
-        var indexOfMessage = test.err.stack.indexOf(test.err.message);
-        if (indexOfMessage === -1) {
-          stackString = test.err.stack;
-        } else {
-          stackString = test.err.stack.substr(test.err.message.length + indexOfMessage);
-        }
-      } else if (test.err.sourceURL && test.err.line !== undefined) {
-        // Safari doesn't give you a stack. Let's at least provide a source line.
-        stackString = '\n(' + test.err.sourceURL + ':' + test.err.line + ')';
-      }
-
-      stackString = stackString || '';
-
-      if (test.err.htmlMessage && stackString) {
-        el.appendChild(fragment('<div class="html-error">%s\n<pre class="error">%e</pre></div>', test.err.htmlMessage, stackString));
-      } else if (test.err.htmlMessage) {
-        el.appendChild(fragment('<div class="html-error">%s</div>', test.err.htmlMessage));
+    if (test.err.stack) {
+      var indexOfMessage = test.err.stack.indexOf(test.err.message);
+      if (indexOfMessage === -1) {
+        stackString = test.err.stack;
       } else {
-        el.appendChild(fragment('<pre class="error">%e%e</pre>', message, stackString));
+        stackString = test.err.stack.substr(test.err.message.length + indexOfMessage);
       }
+    } else if (test.err.sourceURL && test.err.line !== undefined) {
+      // Safari doesn't give you a stack. Let's at least provide a source line.
+      stackString = '\n(' + test.err.sourceURL + ':' + test.err.line + ')';
     }
 
-    // toggle code
-    // TODO: defer
-    if (!test.isPending()) {
-      var h2 = el.getElementsByTagName('h2')[0];
+    stackString = stackString || '';
 
-      on(h2, 'click', function() {
-        pre.style.display = pre.style.display === 'none' ? 'block' : 'none';
-      });
-
-      var pre = fragment('<pre><code>%e</code></pre>', utils.clean(test.body));
-      el.appendChild(pre);
-      pre.style.display = 'none';
+    if (test.err.htmlMessage && stackString) {
+      el.appendChild(fragment('<div class="html-error">%s\n<pre class="error">%e</pre></div>',
+        test.err.htmlMessage, stackString));
+    } else if (test.err.htmlMessage) {
+      el.appendChild(fragment('<div class="html-error">%s</div>', test.err.htmlMessage));
+    } else {
+      el.appendChild(fragment('<pre class="error">%e%e</pre>', message, stackString));
     }
 
+    self.addCodeToggle(el, test.body);
+    appendToStack(el);
+    updateStats();
+  });
+
+  runner.on('pending', function(test) {
+    var el = fragment('<li class="test pass pending"><h2>%e</h2></li>', test.title);
+    appendToStack(el);
+    updateStats();
+  });
+
+  function appendToStack(el) {
     // Don't call .appendChild if #mocha-report was already .shift()'ed off the stack.
     if (stack[0]) {
       stack[0].appendChild(el);
     }
-  });
+  }
+
+  function updateStats() {
+    // TODO: add to stats
+    var percent = stats.tests / this.total * 100 | 0;
+    if (progress) {
+      progress.update(percent).draw(ctx);
+    }
+
+    // update stats
+    var ms = new Date() - stats.start;
+    text(passes, stats.passes);
+    text(failures, stats.failures);
+    text(duration, (ms / 1000).toFixed(2));
+  }
 }
 
 /**
@@ -247,6 +241,24 @@ HTML.prototype.testURL = function(test) {
   return makeUrl(test.fullTitle());
 };
 
+/**
+ * Adds code toggle functionality for the provided test's list element.
+ *
+ * @param {HTMLLIElement} el
+ * @param {string} contents
+ */
+HTML.prototype.addCodeToggle = function(el, contents) {
+  var h2 = el.getElementsByTagName('h2')[0];
+
+  on(h2, 'click', function() {
+    pre.style.display = pre.style.display === 'none' ? 'block' : 'none';
+  });
+
+  var pre = fragment('<pre><code>%e</code></pre>', utils.clean(contents));
+  el.appendChild(pre);
+  pre.style.display = 'none';
+};
+
 /**
  * Display error `msg`.
  *