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.

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

resolves #2149, #1257: Make TAP reporter conform to TAP13 #3452

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
d3c2df9
added test to make sure first line of report is version line
mollstam Aug 8, 2018
c067a6e
reporter will now print version line as first line
mollstam Aug 8, 2018
9c78b61
Changed test to look for plan (1..N) on second line.
mollstam Aug 8, 2018
78ab6c5
Added integration tests to make sure we conform to spec.
mollstam Aug 8, 2018
b59af19
Extended integration test to validate plan, test is growing...
mollstam Aug 8, 2018
ddc5b17
removed some section comments and minor restructure
mollstam Aug 8, 2018
f030a4b
broke out testLineMatcher to testLinePredicate with minor refactor.
mollstam Aug 8, 2018
43c94c8
added a bunch of predicates to identify lines
mollstam Aug 8, 2018
8d369b7
stacks and errors are now wrapped inside YAML blocks
mollstam Aug 8, 2018
9677744
changed arrow function to ye olde function to appease bots
mollstam Aug 8, 2018
3047441
found linter and made code conform, `npm run test` now clean.
mollstam Aug 8, 2018
253d76a
added reporters.fixture.js for testing reporter capabilities
mollstam Aug 9, 2018
e00b99c
Fixed error message potentially breaking YAML block, some cleanup.
mollstam Aug 9, 2018
1e559b3
TAP now inherits from Base
mollstam Aug 9, 2018
419fc33
TAP no longer inherits from Base, decided not to use any Base functio…
mollstam Aug 9, 2018
3551db3
TAP unit tests now expect message starting with literal
mollstam Aug 9, 2018
68d8b9c
TAP integration tests use reporters fixture instead of uncaught
mollstam Aug 9, 2018
3b5f88f
refactored yaml indentation to clean things up
mollstam Sep 19, 2018
3cac7f1
fix to refactored indent which added an extra space on some lines
mollstam Sep 19, 2018
b5b88a1
Retrigger appveyor test
mollstam Sep 19, 2018
ce24d96
TAP reporter inherits Base
mollstam Sep 20, 2018
d5b519d
TAP reporter uses runner.stats rather than internal tracking
mollstam Sep 20, 2018
3242408
added support for `spec` reporterOption and started implementing `TAP…
mollstam Sep 20, 2018
61e8c79
TAP13 tests now run with proper spec selected
mollstam Sep 20, 2018
8dc5b65
extracted fail reporting to producers
mollstam Sep 20, 2018
4b11a31
added missing tests to TAP12 spec which are identical to TAP13 expect…
mollstam Sep 20, 2018
38b8ce3
extracted pending reporting to producers
mollstam Sep 20, 2018
7951bf0
extracted pass reporting to producers
mollstam Sep 20, 2018
ff80cf4
extracted end reporting to producers and refactored to common base
mollstam Sep 20, 2018
0496ef3
fixed bad redeclare
mollstam Sep 20, 2018
42035e7
fix to integrations tests to use TAP13 spec
mollstam Sep 20, 2018
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
132 changes: 113 additions & 19 deletions lib/reporters/tap.js
Expand Up @@ -7,6 +7,7 @@
*/

var Base = require('./base');
Copy link
Contributor

@plroebuck plroebuck Sep 19, 2018

Choose a reason for hiding this comment

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

Disagree with this change. IMO, all reporters should extend Base. See below for at least one reason.

var inherits = require('../utils').inherits;

/**
* Expose `TAP`.
Expand All @@ -24,46 +25,47 @@ exports = module.exports = TAP;
* @api public
* @param {Runner} runner
*/
function TAP(runner) {
function TAP(runner, options) {
Base.call(this, runner);

var self = this;
Copy link
Contributor

@plroebuck plroebuck Sep 21, 2018

Choose a reason for hiding this comment

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

Add this line:
var stats = runner.stats;

var n = 1;
Copy link
Contributor

@plroebuck plroebuck Sep 20, 2018

Choose a reason for hiding this comment

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

n is same as runner.stats.tests. Remove.

var passes = 0;
var failures = 0;

var tapSpec = '12';
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be numeric instead?

if (options && options.reporterOptions) {
if (options.reporterOptions.spec) {
tapSpec = options.reporterOptions.spec;
}
}

if (tapSpec === '13') {
this.producer = new TAP13Producer();
} else {
this.producer = new TAP12Producer();
}

runner.on('start', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

  runner.on('start', function() {
    var ntests = runner.grepTotal(runner.suite);
    self.producer.writeStart(ntests);
  });

var total = runner.grepTotal(runner.suite);
console.log('%d..%d', 1, total);
self.producer.writeStart(runner);
});

runner.on('test end', function() {
++n;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove test end event handler.


runner.on('pending', function(test) {
console.log('ok %d %s # SKIP -', n, title(test));
self.producer.writePending(n, test);
Copy link
Contributor

@plroebuck plroebuck Sep 21, 2018

Choose a reason for hiding this comment

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

    var n = stats.tests;
    self.producer.writePending(n, test);

});

runner.on('pass', function(test) {
passes++;
console.log('ok %d %s', n, title(test));
self.producer.writePass(n, test);
});

runner.on('fail', function(test, err) {
failures++;
console.log('not ok %d %s', n, title(test));
if (err.message) {
console.log(err.message.replace(/^/gm, ' '));
}
if (err.stack) {
console.log(err.stack.replace(/^/gm, ' '));
}
self.producer.writeFail(n, test, err);
});

runner.once('end', function() {
console.log('# tests ' + (passes + failures));
console.log('# pass ' + passes);
console.log('# fail ' + failures);
self.producer.writeEnd(runner);
Copy link
Contributor

@plroebuck plroebuck Sep 21, 2018

Choose a reason for hiding this comment

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

self.producer.writeEnd(stats);

});
}

Expand All @@ -77,3 +79,95 @@ function TAP(runner) {
function title(test) {
return test.fullTitle().replace(/#/g, '');
}

/**
* Inherit from `Base.prototype`.
*/
inherits(TAP, Base);

/**
* Initialize a new `TAPProducer`. Should only be used as a base class.
*
* @private
* @class
* @api private
*/
function TAPProducer() {}

plroebuck marked this conversation as resolved.
Show resolved Hide resolved
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
TAPProducer.prototype.writePending = function(n, test) {
console.log('ok %d %s # SKIP -', n, title(test));
};

TAPProducer.prototype.writePass = function(n, test) {
console.log('ok %d %s', n, title(test));
};

TAPProducer.prototype.writeEnd = function(runner) {
Copy link
Contributor

Choose a reason for hiding this comment

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

TAPProducer.prototype.writeEnd = function(stats) {

console.log('# tests ' + (runner.stats.passes + runner.stats.failures));
Copy link
Contributor

@plroebuck plroebuck Sep 20, 2018

Choose a reason for hiding this comment

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

Just noticed this, but think this is possibly an unreported existing bug.
runner.stats.tests will not agree with this math if pending tests occurred.

console.log('# pass ' + runner.stats.passes);
console.log('# fail ' + runner.stats.failures);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this include a summary line for pending tests?


/**
* Initialize a new `TAP12Producer` which will produce output conforming to the TAP12 spec.
*
* @private
* @class
* @api private
*/
function TAP12Producer() {}

TAP12Producer.prototype.writeStart = function(runner) {
var total = runner.grepTotal(runner.suite);
console.log('%d..%d', 1, total);
};

TAP12Producer.prototype.writeFail = function(n, test, err) {
console.log('not ok %d %s', n, title(test));
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
if (err.message) {
console.log(err.message.replace(/^/gm, ' '));
}
if (err.stack) {
console.log(err.stack.replace(/^/gm, ' '));
}
};

inherits(TAP12Producer, TAPProducer);

/**
* Initialize a new `TAP13Producer` which will produce output conforming to the TAP13 spec.
*
* @private
* @class
* @api private
*/
function TAP13Producer() {}

TAP13Producer.prototype.yamlIndent = function(level) {
return Array(level + 1).join(' ');
};

TAP13Producer.prototype.writeStart = function(runner) {
console.log('TAP version 13');
var total = runner.grepTotal(runner.suite);
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
console.log('%d..%d', 1, total);
};

TAP13Producer.prototype.writeFail = function(n, test, err) {
console.log('not ok %d %s', n, title(test));
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
var emitYamlBlock = err.message != null || err.stack != null;
if (emitYamlBlock) {
console.log(this.yamlIndent(1) + '---');
if (err.message) {
console.log(this.yamlIndent(2) + 'message: |-');
console.log(err.message.replace(/^/gm, this.yamlIndent(3)));
}
if (err.stack) {
console.log(this.yamlIndent(2) + 'stack: |-');
console.log(err.stack.replace(/^/gm, this.yamlIndent(3)));
}
console.log(this.yamlIndent(1) + '...');
}
};

inherits(TAP13Producer, TAPProducer);
63 changes: 63 additions & 0 deletions test/integration/fixtures/reporters.fixture.js
@@ -0,0 +1,63 @@
'use strict';

/**
* This file generates a wide range of output to test reporter functionality.
*/

describe('Animals', function() {

it('should consume organic material', function(done) { done(); });
it('should breathe oxygen', function(done) {
// we're a jellyfish
var actualBreathe = 'nothing';
var expectedBreathe = 'oxygen';
expect(actualBreathe, 'to equal', expectedBreathe);
done();
});
it('should be able to move', function(done) { done(); });
it('should reproduce sexually', function(done) { done(); });
it('should grow from a hollow sphere of cells', function(done) { done(); });

describe('Vertebrates', function() {
describe('Mammals', function() {
it('should give birth to live young', function(done) {
var expectedMammal = {
consumesMaterial: 'organic',
breathe: 'oxygen',
reproduction: {
type: 'sexually',
spawnType: 'live',
}
};
var platypus = JSON.parse(JSON.stringify(expectedMammal));
platypus['reproduction']['spawnType'] = 'hard-shelled egg';

expect(platypus, 'to equal', expectedMammal);
done();
});

describe('Blue Whale', function() {
it('should be the largest of all mammals', function(done) { done(); });
it('should have a body in some shade of blue', function(done) {
var bodyColor = 'blueish_grey';
var shadesOfBlue = ['cyan', 'light_blue', 'blue', 'indigo'];
expect(bodyColor, 'to be one of', shadesOfBlue);

done();
});
});
});
describe('Birds', function() {
it('should have feathers', function(done) { done(); });
it('should lay hard-shelled eggs', function(done) { done(); });
});
});

describe('Tardigrades', function() {
mollstam marked this conversation as resolved.
Show resolved Hide resolved
it('should answer to "water bear"', function(done) { done(); });
it('should be able to survive global mass extinction events', function(done) {
throw new Error("How do we even test for this without causing one?")
done();
});
Copy link
Contributor

@plroebuck plroebuck Sep 20, 2018

Choose a reason for hiding this comment

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

Can we add a pending test somewhere in this fixture?

it('just might one day');

});
});
131 changes: 131 additions & 0 deletions test/integration/reporters.spec.js
Expand Up @@ -99,4 +99,135 @@ describe('reporters', function() {
});
});
});

describe('tap', function() {
var not = function(predicate) {
mollstam marked this conversation as resolved.
Show resolved Hide resolved
return function() {
return !predicate.apply(this, arguments);
};
};
var versionPredicate = function(line) {
return line.match(/^TAP version \d+$/) != null;
};
var planPredicate = function(line) {
return line.match(/^1\.\.\d+$/) != null;
};
var testLinePredicate = function(line) {
return line.match(/^not ok/) != null || line.match(/^ok/) != null;
};
var diagnosticPredicate = function(line) {
return line.match(/^#/) != null;
};
var bailOutPredicate = function(line) {
return line.match(/^Bail out!/) != null;
};
var anythingElsePredicate = function(line) {
return (
versionPredicate(line) === false &&
planPredicate(line) === false &&
testLinePredicate(line) === false &&
diagnosticPredicate(line) === false &&
bailOutPredicate(line) === false
);
};

describe('produces valid TAP v13 output', function() {
var runFixtureAndValidateOutput = function(fixture, expected) {
it('for ' + fixture, function(done) {
var args = ['--reporter=tap', '--reporter-options', 'spec=13'];
run(fixture, args, function(err, res) {
if (err) {
done(err);
return;
}

var expectedVersion = 13;
var expectedPlan = '1..' + expected.numTests;

var outputLines = res.output.split('\n');

// first line must be version line
expect(
outputLines[0],
'to equal',
'TAP version ' + expectedVersion
);

// plan must appear once
expect(outputLines, 'to contain', expectedPlan);
expect(
outputLines.filter(function(l) {
return l === expectedPlan;
}),
'to have length',
1
);
// plan cannot appear in middle of the output
var firstTestLine = outputLines.findIndex(testLinePredicate);
// there must be at least one test line
expect(firstTestLine, 'to be greater than', -1);
var lastTestLine =
outputLines.length -
1 -
outputLines
.slice()
.reverse()
.findIndex(testLinePredicate);
var planLine = outputLines.findIndex(function(line) {
return line === expectedPlan;
});
expect(
planLine < firstTestLine || planLine > lastTestLine,
'to equal',
true
);

done();
});
});
};

runFixtureAndValidateOutput('passing.fixture.js', {
numTests: 2
});
runFixtureAndValidateOutput('reporters.fixture.js', {
numTests: 12
});
});

it('places exceptions correctly in YAML blocks', function(done) {
var args = ['--reporter=tap', '--reporter-options', 'spec=13'];
run('reporters.fixture.js', args, function(err, res) {
if (err) {
done(err);
return;
}

var outputLines = res.output.split('\n');

for (var i = 0; i + 1 < outputLines.length; i++) {
if (
testLinePredicate(outputLines[i]) &&
testLinePredicate(outputLines[i + 1]) === false
) {
var blockLinesStart = i + 1;
var blockLinesEnd =
i +
1 +
outputLines.slice(i + 1).findIndex(not(anythingElsePredicate));
var blockLines =
blockLinesEnd > blockLinesStart
? outputLines.slice(blockLinesStart, blockLinesEnd)
: outputLines.slice(blockLinesStart);
i += blockLines.length;

expect(blockLines[0], 'to match', /^\s+---/);
expect(blockLines[blockLines.length - 1], 'to match', /^\s+\.\.\./);
}
}

done();
});
});
});
});