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.

Sign up for GitHub

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

Jump to bottom

Report skipped assertions #486

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Report skipped assertions #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions lib/results.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ function Results() {
this.count = 0;
this.fail = 0;
this.pass = 0;
this.skip = 0;
this.todo = 0;
this._stream = through();
this.tests = [];
Expand Down Expand Up @@ -104,6 +105,7 @@ Results.prototype._watch = function (t) {
var write = function (s) { self._stream.queue(s); };
t.once('prerun', function () {
var premsg = '';
if (t._skip) self.skip ++;
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this line count a skipped test as a skipped assertion. Unlike in line 122 which counts skipped asserts, this line seems to include numbers that aren't consistent.

There is no equivalent to Line 123 in this prerun callback.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, this is just an update to an old PR meant to illustrate the problem you are mentioning: tests and assertions are mixed together.

The reason the two are mixed together is that tape reports "assertions" as "tests", and doesn't count suites.

# tests 2
# pass  1
# fail  1

If we don't count suites, the only way to indicate that some assertions are skipped is to add one to tests because we don't know how many are actually skipped. That create the problem that you are mentioning.

The solution is probably to change the count and the output to be something like node-tap:

Suites:   1 failed, 1 passed, 1 skipped, 3 of 3 completed
Asserts:  2 failed, 3 passed, 2 skipped, of 7

My concern is not to break people with this change, so I will probably do this behind a flag. Once we agree on the output and the necessity of a flag, then the changes will be trivial.

What's your take? If would be great if you could help me by testing the upcoming changes in your environment. :)

if (t._skip) premsg = 'SKIP ';
else if (t._todo) premsg = 'TODO ';
write('# ' + premsg + coalesceWhiteSpaces(t.name) + '\n');
Expand All @@ -117,6 +119,7 @@ Results.prototype._watch = function (t) {
write(encodeResult(res, self.count + 1));
self.count ++;

if (res.skip) self.skip ++;
if (res.ok || res.todo) self.pass ++;
else {
self.fail ++;
Expand All @@ -136,6 +139,7 @@ Results.prototype.close = function () {
write('\n1..' + self.count + '\n');
write('# tests ' + self.count + '\n');
write('# pass ' + (self.pass + self.todo) + '\n');
if (self.skip) write('# skip ' + self.skip + '\n');
if (self.todo) write('# todo ' + self.todo + '\n');
if (self.fail) write('# fail ' + self.fail + '\n');
else write('\n# ok\n');
Expand Down
6 changes: 3 additions & 3 deletions lib/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,10 +544,10 @@ Test.prototype.doesNotThrow = function (fn, expected, msg, extra) {
});
};

Test.skip = function (name_, _opts, _cb) {
var args = getTestArgs.apply(null, arguments);
Test.skip = function (name_, opts_, cb_) {
var args = getTestArgs(name_, opts_, cb_);
args.opts.skip = true;
return Test(args.name, args.opts, args.cb);
return new Test(args.name, args.opts, args.cb);
};

// vim: set softtabstop=4 shiftwidth=4:
1 change: 1 addition & 0 deletions test/skip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ tap.test('test SKIP comment', function (assert) {
'1..0',
'# tests 0',
'# pass 0',
'# skip 1',
'',
'# ok',
''
Expand Down
1 change: 1 addition & 0 deletions test/skip_explanation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ tap.test('test skip explanations', function (assert) {
'1..9',
'# tests 9',
'# pass 9',
'# skip 6',
'',
'# ok',
''
Expand Down
59 changes: 59 additions & 0 deletions test/skip_output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
var test = require('../');

var concat = require('concat-stream');
var tap = require('tap');

tap.test('skip output test', function (assert) {
assert.plan(1);

var verify = function (output) {
assert.equal(output.toString('utf8'), [
'TAP version 13',
'# SKIP we require more vespene gas',
'# skip assertions',
'ok 1 not enough pylons # SKIP',
'# skip subtests',
'# SKIP we require more ziggurats',
'',
'1..1',
'# tests 1',
'# pass 1',
'# skip 3',
'',
'# ok',
''
].join('\n'));
};

var tapeTest = test.createHarness({ exit: false });
tapeTest.createStream().pipe(concat(verify));

// doesn't look like test.skip is available with createHarness()
// tapeTest.skip('we require more minerals', function (t) {
// t.plan(1);
// t.fail('should not fail test.skip()');
// });

tapeTest('we require more vespene gas', { skip: true }, function (t) {
t.plan(1);
t.fail('should not fail test with { skip: true}');
});

tapeTest('skip assertions', function (t) {
t.plan(1);
t.skip('not enough pylons');
});

tapeTest('skip subtests', function (t) {
// doesn't look like test.skip is available with createHarness()
// test.skip('build more farms', function (t) {
// t.plan(1)
// t.fail('should not run subtest with test.skip()');
// });
t.test('we require more ziggurats', { skip: true }, function (tt) {
tt.plan(1);
tt.fail('should not run subtest with { skip: true }');
});
t.end();
});
});