Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #2083: HTML reporter regression causing duplicate error output #2112

Merged
merged 1 commit into from
May 25, 2016
Merged

Fix #2083: HTML reporter regression causing duplicate error output #2112

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
146 changes: 79 additions & 67 deletions lib/reporters/html.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the comment you mention [Object Error], but in the code it uses a different casing. (O->o)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dasilvacontin the code is correct, the comment is wrong.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clearing it up @pluma!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look further down, I didn't write the comment, just moved around the code :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielstjules true :)

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielstjules does this TODO mean this PR isn't ready?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment already existed above https://github.com/mochajs/mocha/pull/2112/files#diff-998e3fb668a2094eedccab3e56199e4fL141

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielstjules true again

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));
}
}

/**
Expand Down Expand Up @@ -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`.
*
Expand Down