views:

441

answers:

5

So I'm starting to catch the TDD bug but I'm wondering if I'm really doing it right... I seem to be writing A LOT of tests.

The more tests the better, sure, but I've got a feeling that I'm over doing it. And to be honest, I don't know how long I can keep up writing these simple repetitive tests.

For instance, these are the LogOn actions from my AccountController:

public ActionResult LogOn(string returnUrl)
{
    if (string.IsNullOrEmpty(returnUrl))
        returnUrl = "/";

    var viewModel = new LogOnForm()
    {
        ReturnUrl = returnUrl
    };

    return View("LogOn", viewModel);
}

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult LogOn(LogOnForm logOnForm)
{
    try
    {
        if (ModelState.IsValid)
        {
            AccountService.LogOnValidate(logOnForm);

            FormsAuth.SignIn(logOnForm.Email, logOnForm.RememberMe);

            return Redirect(logOnForm.ReturnUrl);
        }
    }
    catch (DomainServiceException ex)
    {
        ex.BindToModelState(ModelState);
    }
    catch
    {
        ModelState.AddModelError("*", "There was server error trying to log on, try again. If your problem persists, please contact us.");
    }

    return View("LogOn", logOnForm);
}

Pretty self explanatory.

I then have the following suite of tests

public void LogOn_Default_ReturnsLogOnView()
public void LogOn_Default_SetsViewDataModel()
public void LogOn_ReturnUrlPassedIn_ViewDataReturnUrlSet()
public void LogOn_ReturnUrlNotPassedIn_ViewDataReturnUrDefaults()
public void LogOnPost_InvalidBinding_ReturnsLogOnViewWithInvalidModelState()
public void LogOnPost_InvalidBinding_DoesntCallAccountServiceLogOnValidate()
public void LogOnPost_ValidBinding_CallsAccountServiceLogOnValidate()
public void LogOnPost_ValidBindingButAccountServiceThrows_ReturnsLogOnViewWithInvalidModelState()
public void LogOnPost_ValidBindingButAccountServiceThrows_DoesntCallFormsAuthServiceSignIn()
public void LogOnPost_ValidBindingAndValidModelButFormsAuthThrows_ReturnsLogOnViewWithInvalidModelState()
public void LogOnPost_ValidBindingAndValidModel_CallsFormsAuthServiceSignIn()
public void LogOnPost_ValidBindingAndValidModel_RedirectsToReturnUrl()

Is that over kill? I haven't even shown the services tests!

Which ones (if any) can I cull?

TIA,
Charles

+1  A: 

Hi Charles,

That looks about right to me. Yes, you will write a lot of Unit Tests and, initially, it will seem like overkill and TBH a waste of time; but stick with it, it'll be worth it. What you should be aiming for (rather than just 100% code coverage) is 100% function coverage. However... if you find that you're writing a lot of UTs for the same method it's possible that that method is doing too much. Try separating your concerns more. In my experience the body of an Action should be doing little more than newing-up a class to do the real work. It's that class that you should really be targeting with UTs.

Chris

Chris Arnold
Ok, so you're saying that all those tests have a place? On the matter of writing too many tests and separating your concerns... I was thinking I had too many tests for my post method and (if I do say so myself) the separation of concerns was just right. I guess that really is a sign that I am writing too many tests...? I will definitely stick with it, and I think that with more experience I'll be able to discern what and what not to test easier (as you and many others have hinted).
Charlino
+1  A: 

I'm unit testing only code what i'm unsure about. Sure - you can never know what will back stab you but writing tests for trivial things seems like an overkill for me.

I'm not a unit-testing/tdd guru - but i think that it's fine if you do NOT write tests just to have them. They must be useful. If you are experienced enough with unit testing - you start to feel when they are going to be valuable and when not.

You might like this book.

Edit:
Actually - i just found a quote about this underneath isolation frameworks chapter. It's about overspecifying tests (one particular test), but i guess the idea remains in more global scope:

Overspecifying the tests
If your test has too many expectations, you may create a test that breaks down with even the lightest of code changes, even though the overall functionality still works. Consider this a more technical way of not verifying the right things. Testing interactions is a double-edged sword: test it too much, and you start to lose sight of the big picture—the overall functionality; test it too little, and you’ll miss the important interactions between objects.

Arnis L.
I'm sure I'll get the hang of it and after writing a fair share of tests I'll be able to make decisions on what to test and what not to. Oh, and yep, I've got that book and just finished reading it... great read and would recommend it to anyone else. But I can't really remember any advice in regards to exactly what and how much to test.
Charlino
Just checked it once again (book) - seems that my memory is messed up a bit. Have seen this elsewhere. Sorry. :)
Arnis L.
That's ok, you had some good advice none-the-less :-)
Charlino
A: 

100% coverage is very ideal, its really helpful if you have to massively refactor your code however, as the tests will govern your code specs to make sure it is correct.

I am personally not a 100% TDD (sometimes too lazy too) but if you intend to do 100%, maybe you should write some test helpers to take away some burden on these repetitive tests. For example, write a helper to test all your CRUD in a standard post structure with a callback to allow you pass in some evaluation might save you a lot of time.

goodwill
+2  A: 

I'd say you are doing a little more than you probably have to. While it is nice to test every possible path your code can take, some paths just aren't very important or don't result in real differences in behavior.

In your example take LogOn(string returnUrl)

The first thing you do in there is check the returnUrl parameter and re-assign it to a default value if it is null/empty. Do you really need a whole unit test just to make sure that one line of code happens as expected? It isn't a line likely to break easily.

Most changes that might break that line be things that would throw a compile error. A change in the default value being assigned is possible in that line (maybe you decide later that "/" isn't a good default value... but in your unit test, I bet you hard-coded it to check for "/" didn't you? So the change in the value will necessitates a change in your test... which means you aren't testing your behavior, instead you are testing your data.

You can achieve a test for the behavior of the method by simply having one test that does NOT supply a parameter. That will hit your "set default" part of the routine while still testing that the rest of the code behaves well too.

Stephen M. Redd
Sounds like very good advice - I'll take that home and chew on it. Re: Hard coded "/" test - yes, picked it like a snotty nose! ;-)
Charlino
Keep in mind that I'm not advocating that you don't hard code some values in tests, though some purists disagree. In that case I'd probably hard code testing against "/" too... I'm just advocating that your test shouldn't be testing just values.
Stephen M. Redd
+6  A: 

It all depends on how much coverage you need / want and how much dependability is an issue.

Here are the questions you should ask yourself:

  • Does this unit test help implement a feature / code change that I don't already have?
  • Will this unit test help regression test/debug this unit if I make changes later?
  • Is the code to satisfy this unit test non-trivial or does it deserve a unit test?

Regarding the 3rd one, I remember when I started writing unit tests (I know, not the same thing as TDD) I would have tests that would go like:

string expected, actual;
TypeUnderTest target = new TypeUnderTest();
target.PropertyToTest = expected;
actual = target.PropertyToTest;
Assert.AreEqual<string>(expected, actual);

I could have done something more productive with my time like choose a better wallpaper for my desktop.

I recommend this article by ASP.net MVC book author Sanderson:

http://blog.codeville.net/2009/08/24/writing-great-unit-tests-best-and-worst-practises/

TJB
The link is an excellent article
Kane
Yeah, sounds like answers to those questions should come easier with experience. Funny you should mention Steve because his MVC book is currently on my night stand... finished chapter 1 last night! ;-) Sounds like I should get stuck into reading it.
Charlino
@Charlino, I just ordered it! after reading the reviews and now some of his posts i'm excited! Hope this helped.
TJB