scottcorgan / tap-out

A different tap parser
MIT License
23 stars 28 forks source link

Support for `TODO` and `SKIP` tests #16

Open keithamus opened 8 years ago

keithamus commented 8 years ago

Summary

Currently, tap-out does not support test directives (the current test directives, as per TAP 13 are SKIP and TODO).

Details

A test directive comes in the form of a # <DIRECTIVE> <REASON> at the end of a test status (the ok/not ok lines). See TAP 13 Specification heading "DIRECTIVES" for more. So a full status might look like one of these combinations:

ok - a test
ok - a test # SKIP foo
not ok - a test # SKIP foo
ok - a test # TODO foo
not ok - a test # TODO foo

As I said, directives come in the form of two styles - SKIP, and TODO. Both have subtly different behaviours... which I'll detail:

SKIP

A SKIP block (to the best of my understanding) basically signifies a test that has not been run. These are likely to be marked as SKIP inside of the test runner - and I would go so far as to say that most test runner implementations probably wont run a SKIP test. The motivation behind skip tests seems to be that they can be used as a mechanism for disabling some tests because, for example, they won't run properly in a particular environment. I suppose test runners could also use SKIP as a signifier to run only the non-skip tests, via some kind of grep mechanism.

SKIP tests will probably always be ok, but the spec does not define them as such.

Proposal

I suggest tap-out's behaviour add a skip key to skipped assertions, with the reason as the value. pass and fail arrays should not have any skipped tests, they should all be in a new skip array. So these two lines:

ok - a test # SKIP foo
not ok - a test # SKIP foo

Currently produce:

    {
      "type": "assert",
      "raw": "ok - a test # SKIP foo",
      "ok": true,
      "name": "a test # SKIP foo",
      "test": 0
    },
    {
      "type": "assert",
      "raw": "not ok - a test # SKIP foo",
      "ok": false,
      "name": "a test # SKIP foo",
      "error": {},
      "test": 0
    },

But should produce

    {
      "type": "assert",
      "raw": "ok - a test # SKIP foo",
      "ok": true,
      "name": "a test",
      "test": 0,
      "skip": "foo"
    },
    {
      "type": "assert",
      "raw": "not ok - a test # SKIP foo",
      "ok": false,
      "name": "a test",
      "error": {},
      "test": 0,
      "skip": "foo"
    },

TODOs

I'm going to quote the prose from the spec because this one is better defined:

Note that if the TODO has an explanation it must be separated from TODO by a space. These tests represent a feature to be implemented or a bug to be fixed and act as something of an executable "things to do" list. They are not expected to succeed. Should a todo test point begin succeeding, the harness should report it as a bonus. This indicates that whatever you were supposed to do has been done and you should promote this to a normal test point.

So in other words:

I suggest tap-out's behaviour add a todo key to todo assertions, with the reason as the value. The fail array should not have any todo tests, they should all be in a new todo array. pass can include any todo tests that were ok. So these two lines:

ok - a test # TODO foo
not ok - a test # TODO foo

currently produce:

    {
      "type": "assert",
      "raw": "ok - a test # TODO foo",
      "ok": true,
      "name": "a test # TODO foo",
      "test": 0
    },
    {
      "type": "assert",
      "raw": "not ok - a test # TODO foo",
      "ok": false,
      "name": "a test # TODO foo",
      "error": {},
      "test": 0
    }

but should be

    {
      "type": "assert",
      "raw": "ok - a test # TODO foo",
      "ok": true,
      "name": "a test",
      "todo": "foo",
      "test": 0
    },
    {
      "type": "assert",
      "raw": "not ok - a test # TODO foo",
      "ok": false,
      "name": "a test",
      "todo": "foo",
      "error": {},
      "test": 0
    }