Add ability to mark a Test as skipped or incomplete

[es]

Past discussion: #434

---

Would a PR adding the ability to skip tests be a welcome change? On a custom fork of TinyMCE we're modifying the functionality and as such certain tests are failing. We want to keep the tests in the codebase and are currently forced to comment them out. Having the ability to skip tests would be the best solution in this case.

The syntax would be test.skip(func...).

I'll gladly make a PR for this, just looking for approval before I start working.

[jzaefferer]

It would be great if you could implement this as a plugin, that way we don't have to commit to an API just yet, and anyone interested in this feature, like you, could start using it.

For some ideas on how to implement this, some other approaches for the API and more discussion, see #434

If you need help implementing a QUnit plugin, check out the existing plugins, or ask me or @leobalter, e.g. via #jquery-dev on Freenode.

[jzaefferer]

To clarify, a plugin would be great to pave the way for landing this in QUnit itself.

[JamesMGreene]

The trouble with creating this as a plugin is that the plugin would have to alter the state of test result data (e.g. to a skipped: true flag, or something) as well as hacking ALL of the Reporters to appropriately reflect the difference in their respective UIs.

It definitely needs to be part of QUnit core or else it will become a big PITA for the plugin maintainers to continue updating it to duck-punch/hack various new/updated Reporters.

[JamesMGreene]

Same goes for the "pending" feature discussed in #434, i.e. for tests that have only been stubbed in by name but not implemented.

Syntax would be to not include the function callback in the call to test:

test("TODO: Implement this test that does X");

[JamesMGreene]

Perhaps we could add a generic status property to the test result data? e.g.

• "passed"
• "failed"
• "skipped"/"inconclusive"
• "pending"/"not implemented"/"future"/"incomplete"

[gibson042]

skipped/pending/future/etc. seem to mean the same thing functionally, and do appear in other Javascript frameworks. Jasmine x-prefixes functions (describe → xdescribe, it → xit) in addition to the empty-body test("TODO") equivalent, and Mocha embeds .skip (describe → describe.skip, it → it.skip). I think others are similar to one or the other.

[jzaefferer]

One of the objections I have, or used to have, was that this feature could be used to write placeholders or disable tests that then stick around forever. The alternative, commenting tests, is usually worse though. With that in mind, I'm currently toying with the idea of adding an expiration date to skips:

skip("this no worky", "2014-08-26", function() { ... });

Which would be okay for, say, a month, then starts to fail. Probably not feasible, but maybe this inspires someone to come up with a better concept that addresses the problem of disabled tests rotting away.

[es]

@jzaefferer In the plugin I was planning on having comments logged on skipped tests. This forces people to be aware of the skipped tests and bring attention to them if need be.

[gibson042]

@jzaefferer It's difficult to get too specific while QUnit is still feeling out syntax options, but I could see something like testIf or test( label, fn ).skipIf( reason, fnDecide ). It's certainly more explicit that what already happens to serve the same purpose in query core's suite.

[es]

I think the reason skipping works in other test frameworks is the simplicity behind it. I feel if we begin adding timeout dates/conditionals it'll over complicate the feature, which should be avoided.

[JamesMGreene]

Can we get some manner of consensus on if we're discussing 1 or 2 goals here, and if both are desired? I view the discussion as:

• Some people want the ability to skip tests (e.g. QUnit.test.skip("x", fn))
• Some people want the ability to stub pending/future tests (e.g. QUnit.test("y");)

I think both goals are worthy but that their implementation and evaluation in the UI would be different, IMHO. However, as they are similar, it is probably a good idea to discuss them together at the onset.

[gibson042]

Taking inspiration from TAP this time, I'd like to recast the terminology while preserving mutual compatibility:

• TODO: Expects failure (i.e., a reversal of normal operation—passes are interesting instead of failures, and failures do not effect pass→fail state changes in the containing run/module/suite/test). Reporters must take care to separate TODO failures from non-TODO failures, and should highlight TODO passes.
• SKIP: Does not run at all. Reporters must take care to report SKIPs, and should treat them as a category distinct from pass/fail.

Discussion in this ticket has centered on SKIP, but past examples would actually benefit more from TODO.

At any rate, I prefer QUnit.test.skip("x", fn) over QUnit.test("y") for SKIP—it formalizes commenting-out, and is actually sufficiently general to subsume the latter (e.g., QUnit.test.skip("z")).

[JamesMGreene]

Taking inspiration from TAP this time, I'd like to recast the terminology while preserving mutual compatibility...

Awesome! I didn't realize any other frameworks/protocol had both of these.

At any rate, I prefer QUnit.test.skip("x", fn) over QUnit.test("y") for SKIP—it formalizes commenting-out

Agreed.

and is actually sufficiently general to subsume the latter (e.g., QUnit.test.skip("z")).

True. Which status would that result in, though? SKIP or TODO? I'm leaning toward SKIP.

[gibson042]

and is actually sufficiently general to subsume the latter (e.g., QUnit.test.skip("z")).

True. Which status would that result in, though? SKIP or TODO? I'm leaning toward SKIP.

Agreed. TODO, if introduced, should be entirely distinct and explicitly opt-in (come to think of it, that clarity would another benefit of not supporting QUnit.test("y")).

[JamesMGreene]

I don't know... QUnit.test.todo("y"); is a little annoying to me personally.

I would prefer QUnit.test("y") to represent pending/TODO tests, or supporting both QUnit.test("y") and QUnit.test.todo("y") to trigger the same functionality/status.

[gibson042]

@JamesMGreene QUnit.test.todo("y") doesn't even make sense, nor does any todo without a corresponding function. See above: TODO represents cases where the testing code exists and is executed, but is expected to fail.

[JamesMGreene]

TODO represents cases where the testing code exists and is executed, but is expected to fail.

Oh, didn't read that closely enough before. That's strange behavior to me.

I would envision TODO as a test without a callback (or at least no assertions).

I would envision what you described not as a test status at all but rather an assertion that cannot fail, e.g. assert.inconclusive(...), and ergo would not make its containing test fail.

[gibson042]

Ok, I think that's reason enough to take TODO off the table for this issue and focus on SKIP (which is entirely appropriate given its description). I just wanted to make sure that we don't end up unnecessarily inconsistent with the field of existing technologies, in which skip means "don't run, but report the fact" and todo means "run, but don't let failures fail the containing group" and both are conceptually applicable at assertion, test, and suite level.

[jzaefferer]

Is there any reason for skip being a property of QUnit.test? Wouldn't QUnit.skip work just as well?

If so, since the interest is still there and there seems to be some agreement on the API design, a PR that implements QUnit.skip( name, callback ) would be welcome. It could take inspiration from #434 for the html reporter, but otherwise this needs to be done from scratch.

Would also be nice to include the semantics of this new method in the description of the PR. We should be able to use that text for the API documentation later.

[es]

@jzaefferer Awesome, I think adding QUnit.skip is the best way forward. I'll add examples and a description of the functionality that can be used in documentation in the PR.

[JamesMGreene]

For anyone else who wants the Mocha-style "pending" syntax, I created this little plugin to build on top of @leobalter's QUnit.skip implementation: JamesMGreene/qunit-pending

Since QUnit core v1.16.0 isn't released yet, it is currently depending (from a dev/Node perspective) on a personal branch I created that is a copy of the latest specific commit in the master branch. I'll be sure to update the dependencies in the "package.json" when QUnit core v1.16.0 is published.

P.S. I fully admit this syntax is harder to search for.