Add T.TODO and T.Skip directives

[wking]

The spec wants these without hyphens, so we can't generate them without a new API. You could have T.Directive(test bool, directive, description string), but it seems more convenient to have a separate method for each case.

[wking]

We could also drop the leading hyphen from the T.Ok output, as long as the description didn't start with a digit or hash. I still like the separate-methods approach I have in f5c62d7 best, but wanted to put the alternative on the table.

[mndrix]

it seems more convenient to have a separate method for each case

I agree. I like the separate methods.

When I skip tests, I usually skip more than one. So I'd like a signature like

func (t *T) Skip(count int, description string)

An example of my most common use case:

if ConditionIsMet() {
  t.Ok(Foo(), "foo")
  t.Ok(Bar(), "bar")
} else {
  t.Skip(2, "condition not met")
}

For TODO tests, it'd be nice if users could apply the TODO status to any test function. Right now TODO only makes sense for T.Ok but I'd eventually like to add methods for comparing numbers, strings, etc. I'd rather not make TODO variants of each of those.

The first thing that comes to mind is something like this:

t.TODO = true
t.Ok(Foo(), "foo")
t.EqOk(Bar(), Baz(), "bar == baz")
t.TODO = false  // could be done via defer too

or maybe do the same thing internally but hide it behind a callback:

t.TODO( func () {
   t.Ok(Foo(), "foo")
   t.EqOk(Bar(), Baz(), "bar == baz")
})

Having a plain field T.TODO bool would let users execute arbitrary logic to determine whether a group of tests should be marked TODO or not. Maybe the tests don't pass on Linux yet, but pass on other platforms, etc.

Preferences?

[wking]

On Wed, Jan 11, 2017 at 04:37:55PM -0800, Michael Hendricks wrote: func (t *T) Skip(count int, description string)

Done.

For TODO tests, it'd be nice if users could apply the TODO status to any test function.

This makes sense. I've gone with a child-state approach where: func (t *T) TODO() *T gives a child T that has a private .todo flag set. That allows for: todo := t.TODO() todo.Ok(Foo(), "foo") todo.EqOk(Bar(), Baz(), "bar == baz") t.Ok(…) // continue on with your non-TODO t and also one-off method chaining: t.TODO().Ok(Foo(), "foo") The only thing that makes me uncomfortable with the child-state approach is that each state entry has its own Writer. If you want to change writers for both the non-TODO and TODO states, you'll have to do both: t.Writer = newWriter todo.Writer = t.Writer I'd rather restrict Writer-holding to the root state instance (like 1e8f72e currently does for nextTestNumber), but there's not a clear way to do that for a public property. Potential approaches: a. Remove the public Writer from T, use T for the child “states” and most consuming function signatures, and add a RootT (or some such) that expands T to expose Writer. Only folks holding that RootT would be able to adjust the Writer. b. Replace Writer with a SetWriter setter. Regardless of whether you call SetWriter on the root state or some ancestor state, you'd always set the internal .writer on the root state. c. Leave 1e8f72e as it stands and expect folks to not mess with Writer unless they've thought it over ;). And regardless of how we move on this, it may be wise to define a TAP-harness interface: type T interface { func Ok(test bool, description string) func Fail(description string) … } type t struct { nextTestNumber int … } to decouple the internal struct from the consumer-facing interface. Then we could use different stucts for the root state and child states without consumers having to bother about the distinction.

[mndrix]

I've gone with a child-state approach where:

func (t *T) TODO() *T

gives a child T that has a private .todo flag set

I like this API. It works well for both one-off and multiple TODO tests, as you've outlined.

I often "comment out" a block of tests. In that case, I like the diff to show just the new "comment" lines. Like this:

+t = t.TODO()
 t.Ok(Foo(), "foo")
 t.EqOk(Bar(), Baz(), "bar == baz")
+t = t.Done()
 t.Ok(…)  // continue on with your non-TODO t

That leaves "git blame" output pointing at the commits which originally created Foo and Bar tests. It also makes it clear on code review that I haven't changed the tests, just their expected pass/fail status. So I think we should have Done() or EndTODO() or something.

As far as implementation goes, the parent-child semantics complicate it more than I'd like. I think we can retain the nice API you've created but use a copy semantics instead. Namely: "TODO() returns a derivative T in todo mode, sharing all other state". If T.nextTestNumber were *int instead of int, then the new methods become very short.

func (t *T) TODO() *T {
    newT := *t
    newT.todo = true
    return &newT
}

func (t *T) Done() *T {
    newT := *t
    newT.todo = false
    return &newT
}

expect folks to not mess with Writer unless they've thought it over ;)

I'm OK with this :-). If we get to the point where a T interface becomes obviously necessary, I think we can revisit it at that point.

[wking]

On Thu, Jan 12, 2017 at 03:36:01PM -0800, Michael Hendricks wrote: +t = t.TODO() t.Ok(Foo(), "foo") t.EqOk(Bar(), Baz(), "bar == baz") +t = t.Done() t.Ok(…) // continue on with your non-TODO t

This is basically your initial propopsal [1], so I've pushed 1e8f72e → 967e479 which adds a public TODO attribute. I've kept the chainable method (now returning a sibling state instead of a child state), but changed its name to ‘Todo()’. [1]: #6 (comment)

[mndrix]

Nicely done. Thanks.