Is Assert.Fail() considered bad practice?

Question

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.

Answer 1

I use MbUnit for my Unit Testing. They have an option to Ignore tests, which show up as Orange (rather than Green or Red) in the test suite. Perhaps xUnit has something similar, and would mean you don't even have to put any assert into the method, because it would show up in an annoyingly different colour making it hard to miss?

Edit:

In MbUnit it is in the following way:

[Test]
[Ignore]
public void YourTest()
{ } 

Answer 2

If you're writing a test that just fails, and then writing the code for it, then writing the test. This isn't Test Driven Development.

Technically, Assert.fail() shouldn't be needed if you're using test driven development correctly.

Have you thought of using a Todo List, or applying a GTD methodology to your work?

Answer 3

Personally I have no problem with using a test suite as a todo list like this as long as you eventually get around to writing the test before you implement the code to pass.

Having said that, I used to use this approach myself, although now I'm finding that doing so leads me down a path of writing too many tests upfront, which in a weird way is like the reverse problem of not writing tests at all: you end up making decisions about design a little too early IMHO.

Incidentally in MSTest, the standard Test template uses Assert.Inconclusive at the end of its samples.

AFAIK the xUnit.NET framework is intended to be extremely lightweight and yes they did cut Fail deliberately, to encourage the developer to use an explicit failure condition.

Answer 4

Beware Assert.Fail and its corrupting influence to make developers write silly or broken tests. For example:

[TestMethod]
public void TestWork()
{
    try {
        Work();
    }
    catch {
        Assert.Fail();
    }
}

This is silly, because the try-catch is redundant. A test fails if it throws an exception.

Also

[TestMethod]
public void TestDivide()
{
    try {
        Divide(5,0);
        Assert.Fail();
    } catch { }
}

This is broken, the test will always pass whatever the outcome of the Divide function. Again, a test fails if and only if it throws an exception.

Answer 5

With the good code I usually do:

void goodCode() {
     // TODO void goodCode()
     throw new NotSupportedOperationException("void goodCode()");
}

With the test code I usually do:

@Test
void testSomething() {
     // TODO void test Something
     Assert.assert("Some descriptive text about what to test")
}

If using JUnit, and don't want to get the failure, but the error, then I usually do:

@Test
void testSomething() {
     // TODO void test Something
     throw new NotSupportedOperationException("Some descriptive text about what to test")
}

Answer 6

Why would you use Assert.Fail for saying that an exception should be thrown? That is unnecessary. Why not just use the ExpectedException attribute?