Unit-test: the Last Reporter #2900
Conversation
…eralls typically uses the Job ID, not the Build ID, so presumably it is designed to handle multiple jobs in the same build?
200 ms timeout was already causing flakes even before the coverage additions anyway; testing suggests that specific tests won't pass with instrumentation without increased time.
… by covering it in the `BUILDTMP/mocha.js` bundle
…her than real usage)
ScottFreeCode
added
area: browser
| runner.emit('test', rootSuite.suites[1].tests[1]); | ||
| rootSuite.suites[1].tests[1].pending = true; | ||
| rootSuite.suites[1].tests[1].duration = 0; | ||
| runner.emit('pending', rootSuite.suites[1].tests[1]); |
There was a problem hiding this comment.
Remind me to revisit this suppression of the non-query-parameters portion of the link in #2728, which attempts to make that opening portion work in all browsers.
| runner.emit('test', rootSuite.suites[1].tests[1]); | ||
| rootSuite.suites[1].tests[1].pending = true; | ||
| rootSuite.suites[1].tests[1].duration = 0; | ||
| runner.emit('pending', rootSuite.suites[1].tests[1]); |
| runner.emit('test', rootSuite.suites[1].tests[1]); | ||
| rootSuite.suites[1].tests[1].pending = true; | ||
| rootSuite.suites[1].tests[1].duration = 0; | ||
| runner.emit('pending', rootSuite.suites[1].tests[1]); |
There was a problem hiding this comment.
Could have pulled in the real Runner, but then this would have been more like an integration test than a unit test... (I made a tradeoff to reuse the real Suite and Test objects mainly because I felt that the code would be too bloated if I tried to recreate them completely but too brittle if I tried to recreate just the bits and pieces that the HTML reporter currently happens to be relying upon.)
|
|
||
| it('should show an error if the tag is missing', function () { | ||
| try { | ||
| // TODO: Figure out why it's ok to add an element to display this error but not ok to just add the missing element in the first place. |
There was a problem hiding this comment.
We can feel free to remove this comment and replace it with a new GitHub issue. I just needed to jot it down while it was on my mind but didn't want to get sidetracked from the work at hand.
|
|
||
| before(saveRealOutput); // so that the following afterEach is guaranteed to work | ||
|
|
||
| afterEach(function () { restoreOutput(); }); // WARNING: this is solely a fallback so *further* tests can be guaranteed to report correctly; these tests will not be reported correctly unless `restoreOutput` is called at the end of the test, before throwing any errors |
There was a problem hiding this comment.
The comment here might not be true when running in Karma and not opening the debug page -- I don't know enough about how it works. I am certain that it is crucial for using the HTML reporter to report the HTML reporter tests, however (e.g. in a manual page such as the one added in this commit as well, or in Karma's debug usage of Mocha's HTML reporter). I'm considering updating the comment to clarify this. It's the explanation for the try-catching in all the tests, so I hope it's clear enough.
karma.conf.js
Outdated
| @@ -34,7 +34,9 @@ module.exports = function (config) { | |||
| // we use the BDD interface for all of the tests that | |||
| // aren't interface-specific. | |||
| 'test/browser-fixtures/bdd.fixture.js', | |||
| 'test/unit/*.spec.js' | |||
| 'test/unit/*.spec.js', | |||
| 'test/browser-unit/*.spec.js', | |||
There was a problem hiding this comment.
Planning on filling test/browser-unit with tests for lib/browser/*.js, especially any that don't just end up covered by extension of being used in other tested stuff.
karma.conf.js
Outdated
| 'test/unit/*.spec.js' | ||
| 'test/unit/*.spec.js', | ||
| 'test/browser-unit/*.spec.js', | ||
| 'test/browser-reporters/*.spec.js' | ||
| ]), |
There was a problem hiding this comment.
We could put the HTML reporter test in the browser unit folder (it's not like we're planning on adding multiple browser reporters in core), but this is more obvious by parallel of test/reporters.
|
I think the Snyk error may be because |
Everything changed when the browser got coverage.