Skip to content

Commit

Permalink
Testsuite time should include setup and teardown
Browse files Browse the repository at this point in the history 
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 #114
  • Loading branch information
@gabegorelick
gabegorelick committed Jun 20, 2020
1 parent 3d1909d commit d2770c2
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 30 deletions.
43 changes: 34 additions & 9 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

var createStatsCollector;
var mocha6plus;

Expand Down Expand Up @@ -201,6 +205,7 @@ function MochaJUnitReporter(runner, options) {
this._runner = runner;
this._generateSuiteTitle = this._options.useFullSuiteTitle ? fullSuiteTitle : defaultSuiteTitle;
this._antId = 0;
this._Date = (options || {}).Date || Date;

var testsuites = [];
this._testsuites = testsuites;
Expand All @@ -220,10 +225,30 @@ function MochaJUnitReporter(runner, options) {
}
}.bind(this));

this._runner.on('suite', function(suite) {
this._onSuiteBegin = function(suite) {
if (!isInvalidSuite(suite)) {
testsuites.push(this.getTestsuiteData(suite));
}
};

this._runner.on('suite', function(suite) {
// allow tests to mock _onSuiteBegin
return this._onSuiteBegin(suite);
}.bind(this));

this._onSuiteEnd = function(suite) {
if (!isInvalidSuite(suite)) {
var testsuite = lastSuite();
if (testsuite) {
var start = testsuite[0]._attr.timestamp;
testsuite[0]._attr.time = this._Date.now() - start;
}
}
};

this._runner.on('suite end', function(suite) {
// allow tests to mock _onSuiteEnd
return this._onSuiteEnd(suite);
}.bind(this));

this._runner.on('pass', function(test) {
Expand Down Expand Up @@ -258,7 +283,7 @@ MochaJUnitReporter.prototype.getTestsuiteData = function(suite) {

var _attr = {
name: this._generateSuiteTitle(suite),
timestamp: new Date().toISOString().slice(0,-5),
timestamp: this._Date.now(),
tests: suite.tests.length
};
var testSuite = { testsuite: [ { _attr: _attr } ] };
Expand Down Expand Up @@ -383,11 +408,11 @@ MochaJUnitReporter.prototype.flush = function(testsuites){
* @returns {string}
*/
MochaJUnitReporter.prototype.getXml = function(testsuites) {
var totalSuitesTime = 0;
var totalTests = 0;
var stats = this._runner.stats;
var antMode = this._options.antMode;
var hasProperties = (!!this._options.properties) || antMode;
var Date = this._Date;

testsuites.forEach(function(suite) {
var _suiteAttr = suite.testsuite[0]._attr;
Expand All @@ -397,20 +422,21 @@ MochaJUnitReporter.prototype.getXml = function(testsuites) {
var _cases = suite.testsuite.slice(_casesIndex);
var missingProps;

_suiteAttr.time = 0;
// suiteTime has unrounded time as a Number of milliseconds
var suiteTime = _suiteAttr.time;

_suiteAttr.time = (suiteTime / 1000 || 0).toFixed(4);
_suiteAttr.timestamp = new Date(_suiteAttr.timestamp).toISOString().slice(0, -5);
_suiteAttr.failures = 0;
_suiteAttr.skipped = 0;

var suiteTime = 0;
_cases.forEach(function(testcase) {
var lastNode = testcase.testcase[testcase.testcase.length - 1];

_suiteAttr.skipped += Number('skipped' in lastNode);
_suiteAttr.failures += Number('failure' in lastNode);
suiteTime += testcase.testcase[0]._attr.time;
testcase.testcase[0]._attr.time = testcase.testcase[0]._attr.time.toFixed(4);
});
_suiteAttr.time = suiteTime.toFixed(4);

if (antMode) {
missingProps = ['system-out', 'system-err'];
Expand All @@ -430,7 +456,6 @@ MochaJUnitReporter.prototype.getXml = function(testsuites) {
delete _suiteAttr.skipped;
}

totalSuitesTime += suiteTime;
totalTests += _suiteAttr.tests;
});

Expand All @@ -439,7 +464,7 @@ MochaJUnitReporter.prototype.getXml = function(testsuites) {
var rootSuite = {
_attr: {
name: this._options.testsuitesTitle,
time: totalSuitesTime.toFixed(4),
time: (stats.duration / 1000 || 0).toFixed(4),
tests: totalTests,
failures: stats.failures
}
Expand Down
26 changes: 26 additions & 0 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
},
"license": "MIT",
"devDependencies": {
"@sinonjs/fake-timers": "^6.0.1",
"chai": "^3.0.0",
"chai-xml": "^0.3.0",
"eslint": "^7.0.0",
Expand Down
49 changes: 35 additions & 14 deletions test/mocha-junit-reporter-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var rimraf = require('rimraf');

var chai = require('chai');
var expect = chai.expect;
var FakeTimer = require('@sinonjs/fake-timers');
var xmllint = require('xmllint');
var chaiXML = require('chai-xml');
var mockXml = require('./mock-results');
Expand Down Expand Up @@ -63,7 +64,7 @@ describe('mocha-junit-reporter', function() {
return test;
}

function executeTestRunner(runner, options, callback) {
function runTests(reporter, options, callback) {
if (!callback) {
callback = options;
options = null;
Expand All @@ -72,6 +73,7 @@ describe('mocha-junit-reporter', function() {
options.invalidChar = options.invalidChar || '';
options.title = options.title || 'Foo Bar';

var runner = reporter.runner;
var rootSuite = runner.suite;

var suite1 = Suite.create(rootSuite, options.title);
Expand Down Expand Up @@ -100,6 +102,22 @@ describe('mocha-junit-reporter', function() {
pendingSuite.addTest(createTest('pending', null, null));
}

var _onSuiteEnd = reporter._onSuiteEnd.bind(reporter);

reporter._onSuiteEnd = function(suite) {
if (suite === rootSuite) {
// root suite took no time to execute
reporter._Date.clock.tick(0);
} else if (suite === suite1) {
// suite1 took an arbitrary amount of time that includes time to run each test + setup and teardown
reporter._Date.clock.tick(100001);
} else if (suite === suite2) {
reporter._Date.clock.tick(400005);
}

return _onSuiteEnd(suite);
};

runRunner(runner, callback);
}

Expand Down Expand Up @@ -147,7 +165,10 @@ describe('mocha-junit-reporter', function() {
allowUncaught: true
});

return new mocha._reporter(createRunner(), { reporterOptions: options });
return new mocha._reporter(createRunner(), {
reporterOptions: options,
Date: FakeTimer.createClock(0).Date
});
}

function runRunner(runner, callback) {
Expand Down Expand Up @@ -201,7 +222,7 @@ describe('mocha-junit-reporter', function() {

it('can produce a JUnit XML report', function(done) {
var reporter = createReporter({mochaFile: 'test/output/mocha.xml'});
executeTestRunner(reporter.runner, function() {
runTests(reporter, function() {
verifyMochaFile(reporter.runner, filePath);
done();
});
Expand All @@ -210,7 +231,7 @@ describe('mocha-junit-reporter', function() {
it('respects `process.env.MOCHA_FILE`', function(done) {
process.env.MOCHA_FILE = 'test/output/results.xml';
var reporter = createReporter();
executeTestRunner(reporter.runner, function() {
runTests(reporter, function() {
verifyMochaFile(reporter.runner, process.env.MOCHA_FILE);
done();
});
Expand All @@ -219,7 +240,7 @@ describe('mocha-junit-reporter', function() {
it('respects `process.env.PROPERTIES`', function(done) {
process.env.PROPERTIES = 'CUSTOM_PROPERTY:ABC~123';
var reporter = createReporter({mochaFile: 'test/output/properties.xml'});
executeTestRunner(reporter.runner, function() {
runTests(reporter, function() {
verifyMochaFile(reporter.runner, filePath, {
properties: [
{
Expand All @@ -234,7 +255,7 @@ describe('mocha-junit-reporter', function() {

it('respects `--reporter-options mochaFile=`', function(done) {
var reporter = createReporter({mochaFile: 'test/output/results.xml'});
executeTestRunner(reporter.runner, function() {
runTests(reporter, function() {
verifyMochaFile(reporter.runner, filePath);
done();
});
Expand All @@ -244,39 +265,39 @@ describe('mocha-junit-reporter', function() {
var dir = 'test/output/';
var path = dir + 'results.[hash].xml';
var reporter = createReporter({mochaFile: path});
executeTestRunner(reporter.runner, function() {
runTests(reporter, function() {
verifyMochaFile(reporter.runner, dir + getFileNameWithHash(dir));
done();
});
});

it('will create intermediate directories', function(done) {
var reporter = createReporter({mochaFile: 'test/output/foo/mocha.xml'});
executeTestRunner(reporter.runner, function() {
runTests(reporter, function() {
verifyMochaFile(reporter.runner, filePath);
done();
});
});

it('creates valid XML report for invalid message', function(done) {
var reporter = createReporter({mochaFile: 'test/output/mocha.xml'});
executeTestRunner(reporter.runner, {invalidChar: '\u001b'}, function() {
runTests(reporter, {invalidChar: '\u001b'}, function() {
assertXmlEquals(reporter._xml, mockXml(reporter.runner.stats));
done();
});
});

it('creates valid XML report even if title contains ANSI character sequences', function(done) {
var reporter = createReporter({mochaFile: 'test/output/mocha.xml'});
executeTestRunner(reporter.runner, {title: '[38;5;104m[1mFoo Bar'}, function() {
runTests(reporter, {title: '[38;5;104m[1mFoo Bar'}, function() {
verifyMochaFile(reporter.runner, filePath);
done();
});
});

it('outputs pending tests if "includePending" is specified', function(done) {
var reporter = createReporter({mochaFile: 'test/output/mocha.xml', includePending: true});
executeTestRunner(reporter.runner, {includePending: true}, function() {
runTests(reporter, {includePending: true}, function() {
verifyMochaFile(reporter.runner, filePath);
done();
});
Expand All @@ -286,7 +307,7 @@ describe('mocha-junit-reporter', function() {
var reporter = createReporter({mochaFile: 'test/output/console.xml', toConsole: true});

var stdout = mockStdout();
executeTestRunner(reporter.runner, function() {
runTests(reporter, function() {
verifyMochaFile(reporter.runner, filePath);

var xml = stdout.output[0];
Expand Down Expand Up @@ -325,7 +346,7 @@ describe('mocha-junit-reporter', function() {
describe('when "useFullSuiteTitle" option is specified', function() {
it('generates full suite title', function(done) {
var reporter = createReporter({useFullSuiteTitle: true });
executeTestRunner(reporter.runner, function() {
runTests(reporter, function() {
expect(suiteName(reporter._testsuites[0])).to.equal('');
expect(suiteName(reporter._testsuites[1])).to.equal('Root Suite Foo Bar');
expect(suiteName(reporter._testsuites[2])).to.equal('Root Suite Another suite!');
Expand All @@ -335,7 +356,7 @@ describe('mocha-junit-reporter', function() {

it('generates full suite title separated by "suiteTitleSeparatedBy" option', function(done) {
var reporter = createReporter({useFullSuiteTitle: true, suiteTitleSeparatedBy: '.'});
executeTestRunner(reporter.runner, function() {
runTests(reporter, function() {
expect(suiteName(reporter._testsuites[0])).to.equal('');
expect(suiteName(reporter._testsuites[1])).to.equal('Root Suite.Foo Bar');
expect(suiteName(reporter._testsuites[2])).to.equal('Root Suite.Another suite!');
Expand Down
14 changes: 7 additions & 7 deletions test/mock-results.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ module.exports = function(stats, options) {
name: "Mocha Tests",
tests: 4,
failures: "2",
time: "432.1100"
time: ((stats.duration || 0) / 1000).toFixed(4)
}
},
{
testsuite: [
{
_attr: {
name: "Root Suite",
timestamp: stats.start.toISOString().substr(0,stats.start.toISOString().indexOf('.')),
timestamp: "1970-01-01T00:00:00", // ISO timestamp truncated to the second
tests: "0",
failures: "0",
time: "0.0000"
Expand All @@ -31,10 +31,10 @@ module.exports = function(stats, options) {
{
_attr: {
name: "Foo Bar",
timestamp: stats.start.toISOString().substr(0,stats.start.toISOString().indexOf('.')),
timestamp: "1970-01-01T00:00:00",
tests: "3",
failures: "2",
time: "32.1060"
time: "100.0010"
}
},
{
Expand Down Expand Up @@ -93,10 +93,10 @@ module.exports = function(stats, options) {
{
_attr: {
name: "Another suite!",
timestamp: stats.start.toISOString().substr(0,stats.start.toISOString().indexOf('.')),
timestamp: "1970-01-01T00:01:40", // new Date(100001).toISOString().slice(0, -5)
tests: "1",
failures: "0",
time: "400.0040"
time: "400.0050"
}
},
{
Expand Down Expand Up @@ -144,7 +144,7 @@ module.exports = function(stats, options) {
{
_attr: {
name: "Pending suite!",
timestamp: stats.start.toISOString().substr(0,stats.start.toISOString().indexOf('.')),
timestamp: "1970-01-01T00:08:20", // new Date(100001 + 400005).toISOString().slice(0, -5)
tests: "1",
failures: "0",
skipped: "1",
Expand Down

0 comments on commit d2770c2

Please sign in to comment.