assert.Fail signature

[tbruyelle]

I don't get why the signature of assert.Fail requires a mandatory string, and I think it's misleading.

I had a panic because of that, I used the method like that :

assert := assert.New(t)
assert.Fail("stuff should be %d", 42) // PANICS 

The panic occurs because 42 is type asserted to string (see https://github.com/stretchr/testify/blob/master/assert/assertions.go#L178)

So currently the correct way to use this method is :

assert.Fail("failed", "stuff should be %d", 42)

But I don't see the benefit of such signature, I think the method shouldn't have a failureMessage argument, only msgAndArgs.

WDYT?

[icholy]

Changing it would be a non-compatible API change, so I think that's out of the question. I just want to know why you're calling it directly?

[tbruyelle]

It's an exported method, so why not ? Sometimes you just want to make your test fail, according to some conditions that can't be asserted with this library.

[icholy]

@tbruyelle I mean as opposed to using testing/T.Fail

[tbruyelle]

To keep the nice output of testify.

[brackendawson]

I'm certain that Fail was accidentally exported. It's not a more useful version of testing.T.Errorf() because it actually contains two failure messages, failureMessage is the message from the assertion and ...msgAndArgs is the user provided failure message format string and args.

I'd accept a PR which marks this function and method as deprecated and tells people to use testing.T.Errorf instead.

Same for FailNow, Failf, and FailNowf.