-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
fix(logging): Summarize SKIPPED tests in debug.html. #1115
Conversation
@@ -22,10 +22,17 @@ | |||
info: function(info) { | |||
if (info.dump && window.console) window.console.log(info.dump); | |||
}, | |||
complete: function() {}, | |||
complete: function() { | |||
console.log('Skipped ' + this.skipped + ' tests'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use window.console.log
as in the other places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Otherwise LGTM |
var msg = result.skipped ? 'SKIPPED ' : (result.success ? 'SUCCESS ' : 'FAILED '); | ||
if (result.skipped) { | ||
this.skipped++; | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you remove print skipped test? ('SKIPPED testSuite description' line 36)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such tests talk about problems in code(test code, source code).
Usually you have to either fix the skipped test or delete.
And such annoying mechanism makes you pay attention to such tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, these annoying messages force you to pay attention to tests that you have deliberately marked as not important. The deliberate act of marking tests to be SKIPPED is a signal that Karma should obey. The common case of marking tests to be skipped is the use of ddescribe()
or iit()
to isolate a single suite or test for debugging or analysis. But such debugging or analysis is more difficult than necessary when the console is filled with hundreds of lines of SKIPPED tests.
Occasionally a developer uses xit()
to skip a single test while investigating some other problem. The summary line at the end of the Karma run reminds the developer to return to this case.
Before: hundreds of SKIPPING lines in console.log. After: A single summary line with a count. Closes karma-runner#1111
I like this change. LGTM |
I think this is good as well. When you do iit() you get tons of "SKIPPED" logs in the console which makes it hard to see anything else... |
Before: hundreds of SKIPPING lines in console.log.
After: A single summary line with a count.
Closes #1111