Is Assert.Fail() considered bad practice?

2019-03-09 19:06发布

I use Assert.Fail a lot when doing TDD. I'm usually working on one test at a time but when I get ideas for things I want to implement later I quickly write an empty test where the name of the test method indicates what I want to implement as sort of a todo-list. To make sure I don't forget I put an Assert.Fail() in the body.

When trying out xUnit.Net I found they hadn't implemented Assert.Fail. Of course you can always Assert.IsTrue(false) but this doesn't communicate my intention as well. I got the impression Assert.Fail wasn't implemented on purpose. Is this considered bad practice? If so why?


@Martin Meredith That's not exactly what I do. I do write a test first and then implement code to make it work. Usually I think of several tests at once. Or I think about a test to write when I'm working on something else. That's when I write an empty failing test to remember. By the time I get to writing the test I neatly work test-first.

@Jimmeh That looks like a good idea. Ignored tests don't fail but they still show up in a separate list. Have to try that out.

@Matt Howells Great Idea. NotImplementedException communicates intention better than assert.Fail() in this case

@Mitch Wheat That's what I was looking for. It seems it was left out to prevent it being abused in another way I abuse it.

13条回答
三岁会撩人
2楼-- · 2019-03-09 19:31

I've always used Assert.Fail() for handling cases where you've detected that a test should fail through logic beyond simple value comparison. As an example:

try 
{
  // Some code that should throw ExceptionX
  Assert.Fail("ExceptionX should be thrown")
} 
catch ( ExceptionX ex ) 
{
  // test passed
}

Thus the lack of Assert.Fail() in the framework looks like a mistake to me. I'd suggest patching the Assert class to include a Fail() method, and then submitting the patch to the framework developers, along with your reasoning for adding it.

As for your practice of creating tests that intentionally fail in your workspace, to remind yourself to implement them before committing, that seems like a fine practice to me.

查看更多
家丑人穷心不美
3楼-- · 2019-03-09 19:35

Wild guess: withholding Assert.Fail is intended to stop you thinking that a good way to write test code is as a huge heap of spaghetti leading to an Assert.Fail in the bad cases. [Edit to add: other people's answers broadly confirm this, but with quotations]

Since that's not what you're doing, it's possible that xUnit.Net is being over-protective.

Or maybe they just think it's so rare and so unorthogonal as to be unnecessary.

I prefer to implement a function called ThisCodeHasNotBeenWrittenYet (actually something shorter, for ease of typing). Can't communicate intention more clearly than that, and you have a precise search term.

Whether that fails, or is not implemented (to provoke a linker error), or is a macro that doesn't compile, can be changed to suit your current preference. For instance when you want to run something that is finished, you want a fail. When you're sitting down to get rid of them all, you may want a compile error.

查看更多
走好不送
4楼-- · 2019-03-09 19:36

I think you should ask yourselves what (upfront) testing should do.

First, you write a (set of) test without implmentation. Maybe, also the rainy day scenarios.

All those tests must fail, to be correct tests: So you want to achieve two things: 1) Verify that your implementation is correct; 2) Verify that your unit tests are correct.

Now, if you do upfront TDD, you want to execute all your tests, also, the NYI parts. The result of your total test run passes if: 1) All implemented stuff succeeds 2) All NYI stuff fails

After all, it would be a unit test ommision if your unit tests succeeds whilst there is no implementation, isnt it?

You want to end up with something of a mail of your continous integration test that checks all implemented and not implemented code, and is sent if any implemented code fails, or any not implemented code succeeds. Both are undesired results.

Just write an [ignore] tests wont do the job. Neither, an asserts that stops an the first assert failure, not running other tests lines in the test.

Now, how to acheive this then? I think it requires some more advanced organisation of your testing. And it requires some other mechanism then asserts to achieve these goals.

I think you have to split up your tests and create some tests that completly run but must fail, and vice versa.

Ideas are to split your tests over multiple assemblies, use grouping of tests (ordered tests in mstest may do the job).

Still, a CI build that mails if not all tests in the NYI department fail is not easy and straight-forward.

查看更多
Evening l夕情丶
5楼-- · 2019-03-09 19:41

For this scenario, rather than calling Assert.Fail, I do the following (in C# / NUnit)

[Test]
public void MyClassDoesSomething()
{
    throw new NotImplementedException();
}

It is more explicit than an Assert.Fail.

There seems to be general agreement that it is preferable to use more explicit assertions than Assert.Fail(). Most frameworks have to include it though because they don't offer a better alternative. For example, NUnit (and others) provide an ExpectedExceptionAttribute to test that some code throws a particular class of exception. However in order to test that a property on the exception is set to a particular value, one cannot use it. Instead you have to resort to Assert.Fail:

[Test]
public void ThrowsExceptionCorrectly()
{
    const string BAD_INPUT = "bad input";
    try
    {
        new MyClass().DoSomething(BAD_INPUT);
        Assert.Fail("No exception was thrown");
    }
    catch (MyCustomException ex)
    {
         Assert.AreEqual(BAD_INPUT, ex.InputString); 
    }
}

The xUnit.Net method Assert.Throws makes this a lot neater without requiring an Assert.Fail method. By not including an Assert.Fail() method xUnit.Net encourages developers to find and use more explicit alternatives, and to support the creation of new assertions where necessary.

查看更多
叛逆
6楼-- · 2019-03-09 19:41

It was deliberately left out. This is Brad Wilson's reply as to why is there no Assert.Fail():

We didn't overlook this, actually. I find Assert.Fail is a crutch which implies that there is probably an assertion missing. Sometimes it's just the way the test is structured, and sometimes it's because Assert could use another assertion.

查看更多
Emotional °昔
7楼-- · 2019-03-09 19:41

This is the pattern that I use when writting a test for code that I want to throw an exception by design:

[TestMethod]
public void TestForException()
{
    Exception _Exception = null;

    try
    {
        //Code that I expect to throw the exception.
        MyClass _MyClass = null;
        _MyClass.SomeMethod();
        //Code that I expect to throw the exception.
    }
    catch(Exception _ThrownException)
    {   
        _Exception = _ThrownException
    }
    finally
    {
        Assert.IsNotNull(_Exception);
        //Replace NullReferenceException with expected exception.
        Assert.IsInstanceOfType(_Exception, typeof(NullReferenceException));
    }
}

IMHO this is a better way of testing for exceptions over using Assert.Fail(). The reason for this is that not only do I test for an exception being thrown at all but I also test for the exception type. I realise that this is similar to the answer from Matt Howells but IMHO using the finally block is more robust.

Obviously it would still be possible to include other Assert methods to test the exceptions input string etc. I would be grateful for your comments and views on my pattern.

查看更多
登录 后发表回答