Testsuite time should include setup and teardown #115
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gabegorelick
commented
May 9, 2020
| @@ -9,6 +9,10 @@ var mkdirp = require('mkdirp'); | |||
| var md5 = require('md5'); | |||
| var stripAnsi = require('strip-ansi'); | |||
|
|
|||
| // Save timer references so that times are correct even if Date is stubbed. | |||
| // See https://github.com/mochajs/mocha/issues/237 | |||
| var Date = global.Date; | |||
There was a problem hiding this comment.
I noticed that mocha uses this in its own reporters, so figured we should do it too.
gabegorelick
commented
May 9, 2020
| @@ -258,7 +273,7 @@ MochaJUnitReporter.prototype.getTestsuiteData = function(suite) { | |||
|
|
|||
| var _attr = { | |||
| name: this._generateSuiteTitle(suite), | |||
| timestamp: new Date().toISOString().slice(0,-5), | |||
| timestamp: this._Date.now(), | |||
There was a problem hiding this comment.
Kind of ugly but we need to keep the raw start timestamp and not round it until later. Since we only pass around the XML object, I have to stick it on timestamp. Later on, we convert this into a string.
A larger refactoring would pass around domain objects and only structure them as XML in getXml, but that's a larger change.
gabegorelick
commented
May 9, 2020
| @@ -202,6 +206,7 @@ function MochaJUnitReporter(runner, options) { | |||
| this._runner = runner; | |||
| this._generateSuiteTitle = this._options.useFullSuiteTitle ? fullSuiteTitle : defaultSuiteTitle; | |||
| this._antId = 0; | |||
| this._Date = (options || {}).Date || Date; | |||
There was a problem hiding this comment.
Tests can mock Date to simulate time elapsing.
gabegorelick
force-pushed
the
setup-teardown-time
branch
from
13bd5dc to
f49aff2
Compare
The `time` attribute on `testsuite` should equal the amount of time between the runner issuing the `suite` and `end suite` events. It was previously calculated as the sum of each individual testcase's `time`, but that doesn't account for setup and teardown. Similarly, total time for all testsuites should be the amount of time between the `start` and `end` events, which the `Base` reporter stores in `stats.duration` for us. Fixes michaelleeallen#114
gabegorelick
force-pushed
the
setup-teardown-time
branch
from
f49aff2 to
d2770c2
Compare
gabegorelick
commented
Jun 20, 2020
|
|
||
| this._runner.on('suite', function(suite) { | ||
| // allow tests to mock _onSuiteBegin | ||
| return this._onSuiteBegin(suite); |
There was a problem hiding this comment.
No tests do this yet, but added for symmetry with _onSuiteEnd.
|
@clayreimann This is ready whenever you have a chance. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
timeattribute ontestsuiteshould equal the amount of timebetween the runner issuing the
suiteandend suiteevents.It was previously calculated as the sum of each individual testcase's
time, but that doesn't account for setup and teardown.Similarly, total time for all testsuites should be the amount of
time between the
startandendevents, which theBasereporterstores in
stats.durationfor us.Fixes #114