tape-testing / tape

tap-producing test harness for node and browsers
MIT License
5.77k stars 307 forks source link

t.skip() differs from tape.skip() and t.test(, { skip: true }) #545

Closed Krinkle closed 8 months ago

Krinkle commented 3 years ago
tape.test('suite with skipped test', function (t) {
  t.skip('should skip', function (t) {
    t.end();
  });
  t.end();
});

Based on the documentation, I assumed that t.test() and t.skip() would both behave similar to tape.test() and tape.skip(), and that in both cases t.skip() would "Generate a new test that will be skipped over.".

I noticed as part of preparing an integration test for js-reporters (by creating a test somewhat similar in results to this mocha example), that the above example does not result in a "test" object being emitted on the stream. Instead, it produces an "assert" object.

I checked all four ways to create skipped tests, and found that 3/4 create a test, but the above one creates an assert instead.

const tape = require('tape');

// const stream = tape.createStream({ objectMode: true });
// stream.on('data', (row) => console.log(row));

tape.skip('top-level skip', function (t) {
  t.end();
});
tape.test('top-level test with skip', { skip: true }, function (t) {
  t.end();
});
tape.test('top-level test', function (t) {
  t.skip('child skip', function (t) {
    t.end();
  });
  t.test('child test with skip', { skip: true }, function (t) {
    t.end();
  });
  t.end();
});
{ type: 'test', id: 0, name: 'top-level test with skip', skip: true, todo: false }
{ type: 'end', test: 0 }

{ type: 'test', id: 1, name: 'top-level test', skip: false, todo: false }
  { type: 'assert', test: 1, id: 0, ok: true, skip: true, todo: false, name: 'child skip', operator: 'skip', objectPrintDepth: 5 }
  { type: 'test', id: 2, name: 'child test with skip', skip: true, todo: false, parent: 1 }
  { type: 'end', test: 2 }
{ type: 'end', test: 1 }
TAP version 13
# SKIP top-level test with skip
# top-level test
ok 1 child skip # SKIP
# SKIP child test with skip

This relates to issue https://github.com/substack/tape/issues/251, which asked for skipped tests to be reported in TAP as a test with a # SKIP directive as specified in TAP 13. This was resolved for the t.skip() method, but not yet for the three other ways of creating skipped tests.

Somewhat counter-intuitively, while t.skip() is the only one of four methods that is reported to TAP as a test, it is only the only one internally reported as an assert. I don't yet know if that's significant, but it might be relevant for compat.

ljharb commented 3 years ago

Interesting. I'd certainly expect all of the 4 skip mechanisms to behave the same.

It'd be worth exploring a PR that makes them do so.

ljharb commented 8 months ago

It's worth noting that t.skip() doesn't take a callback function - it takes a string message, and an "extra" object that will be available on the dispatched event (for things that listen for that). The readme also says "Generate an assertion that will be skipped over.", so I think this is by design.