views:

2368

answers:

12

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.

+4  A: 

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()
{ }
Jimmeh
Yes, it's [Fact(Skip = "This is not ready for prime time")]
Brad Wilson
A: 

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?

Mez
What if you are trying to test for negative behavior?
toast
You can just as easily use a boolean expression in either an Assert.IsTrue or Assert.IsFalse for negative or positive behaviors.
Jim Burger
Questioner didn't say he was writing code before finishing tests. He said he was writing other tests before finishing these tests. That may or may not be undisciplined, but I don't think it's a breach of TDD.
Steve Jessop
+8  A: 

It was deliberately left out. Brad Wilson discusses this and other questions here. His reply 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.

Mitch Wheat
+19  A: 

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.

Matt Howells
I second the 'throw new NotImplementedException()'.
Aaron Maenpaa
The throws method allows for the use of lamdbas, I really like it: MyException ex = Assert.Throws<MyException>(() => action());Assert.Equal("foobar", ex.Message);
Jim Burger
Personally, in the scenario where I'm "documenting future tests", I will just write the test and let it pass, but its empty body is a reminder that I wanted a test here. Or I'll even just leave the test name/description as a comment in the test file. Failing would be bad here, though.
Brad Wilson
Surely it is correct that the unimplemented test fails, since by definition if you haven't written the test yet you cannot have written the code. If the test failure breaks your CI you can just put an [Ignore] attribute on it as well.
Matt Howells
Failing unimplemented tests ruins the red/green/refactor rhythm. Either keep a separate todo list, or follow Jimmeh's advice and mark an unimplemented test as ignored or inconclusive until it's ready.
Don Kirkby
Don has it exactly right. Failing "future test placeholders" kills the TDD rhythm. One thing at a time... that future test placeholder is a reminder for later, not something to be done now.
Brad Wilson
A: 

MS Test has Assert.Fail() but it also has Assert.Inconclusive(). I think that the most appropriate use for Assert.Fail() is if you have some in-line logic that would be awkward to put in an assertion, although I can't even think of any good examples. For the most part, if the test framework supports something other than Assert.Fail() then use that.

Mark Cidade
+3  A: 

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.

Jim Burger
I agree doing this too much leads to YAGNI situations :-) thanks.
Mendelt
+1  A: 

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.

Craig Trader
For this example I would probably use the ExpectedExceptionAttribute common to most frameworks to save on nesting in a try catch, although this method is ood if you want to test the correct exception message is used.
Jim Burger
btw I think Brad Wilson will personally knock back any patch attempts to add a Fail method to the Assert class. ;)
Jim Burger
I do this an awful lot in jUnit. It seems very neat and elegant to me.ExpectedExceptionAttribute in NUnit seems to remove this particular need for Assert.Fail(), but the fact that we've found one valid use suggests there may be others.
slim
Based upon his explanation, that's fine, it's his code. It's even better if he gives you a polite "Thanks, but no, here's why." At the very least he gets feedback that someone cares enough about his work to want to improve it, even if the improvements aren't what he wants.
Craig Trader
I'd normally use ExpectedExceptionAttribute except where I need to check properties on the exception, like my example.
Matt Howells
Yes, we would be reluctant to include Fail(). For what it's worth, in the 18 months the framework has been used (and 13 months of its public release), nobody has ever asked for Fail() and nobody has ever submitted a patch for it. I think our reasoning has been sound.
Brad Wilson
+1  A: 

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.

Steve Jessop
+2  A: 

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")
}
Banengusk
A: 

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

I think you misread the question. I'm not expecting an exception. I'm using the test as a placeholder for a test I want to write later. I'm not testing anything yet so I want the test to fail as a reminder.
Mendelt
A: 

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.

lexx
A: 

I found Assert.Fail useful when programming with events or callbacks. In other words, given some input a certain event should not fire. Using Fail when such event occurs is much cleaner, and easier than writing addintional test code to set boolean flags.

Finglas