TDD

Review of Roy Osherove’s review.

Posted on by Vadim in TDD

I just finished watching video by Roy Osherove where he reviews Unit Tests for NetDinner MVC source code.

I liked the video and many points Roy makes are very valid.  However, let me concentrate on the things I didn’t like.

  1. During the video Roy complains how tired he is because it’s 2 AM and he has to record the video.  Who’s fault is that?  I’m as a viewer prefer for my presenter to be fresh and energetic.  Roy, I want to you to teach me, I don’t want to know that you’re tired and that your wife and the kid sleep in another room.
  2. Roy also criticize people who created a test with multiple asserts.  In that particular case the test method had two asserts.  I personally don’t see big problem to use more than one Assert as long as the test name describe what is tested.  He also gives viewers a doctor analogy which I fail to find appropriate.
  3. I was very surprised when after complaining about multiple asserts in a single unit test, Roy recommends using RowTest feature of MbUnit / NUnit.  Roy, how do you describe what is the difference between the first row and second row tests? About three years ago when I was a rookie in TDD world,  I switched to MbUnit from NUnit because of RowTest attribute.  NUnit didn’t have this feature at the time.  However, I stopped using RowTest in my unit tests because it’s not clear what is tested.  However, I still use this RowTests for my integration tests.
    Here are couple of reasons why I stopped using RotTest attribute in my unit tests.

    a. Let assume that you have a test that fails if a value is null or empty string.  Here’s an example: 

    [Row("")]
[Row(null)]
public void UserName_property_should_faile_when_value_is_empty_or_null(string userName)

    

    The problem is that when one of row test fails, it’s hard to see which one failed. 
    

    b. I use TestDriven.NET.  When you run Row tests, TestDriven.NET executes all the row tests. To execute only one row I had to comment out all the other rows. 
    

    Instead of example above, I prefer to create two tests like this: 
    

    [Test]
public void UserName_property_should_faile_when_value_is_empty()

[Test]
public void UserName_property_should_faile_when_value_is_null()

Roy, no hard feelings.  I personally believe that you are doing a great job of educating people of better development practices.  I myself learned a lot from you.  Just don’t talk like Roy’s way is the only way.

5 thoughts on “Review of Roy Osherove’s review.”

  1. Hello

    I agree that B is really annoying.

    As for A, though, you can handle using the parameters as the fail reason.

    Assert.AreEuqual(“bob”, userName, (userName ?? “none”) + ” failed)

    If that’s not explicit enough, pass in the fail reason as a parameter
    [RowTest]
    [Row(“”, “Empty string failed”)]

    Reply

  2. [Row(“”, Description=”Should fail when string is empty.”)]
    [Row(null, Description=”Should fail when string is null.”)]

    MbUnit will also show the formatted values as part of the test step name.

    Reply

  3. Jay & Jeff,

    First of all thanks for stopping by.

    I’ve used Description in Row attribute on many occasions but only for my integration tests.

    For myself, I found that it’s easier to maintain my unit tests when they are in separate methods instead of using Row attribute. It doesn’t mean that it’s the right medicine to everyone.

    As a developer you have to make a decision what makes scenes to you. Don’t follow religiously some technique just because you read it on internet.

    Reply

  4. Yup. There are many considerations involved in test readability. I have certainly written my share of unintelligible tests using a variety of different obfuscation techniques. :-)

    Reply

  5. Vadim just sent me a link to remind me of this conversation due to something else we were talking about.

    I just read Roy’s book, and I have a lot to say about it. But, as it pertains to this…

    The test doesn’t complete with multiple asserts when one assert fails.
    With row tests, all tests will run even if some of them fail.

    His concern about the multiple asserts is that an assert you never get to could tell you more information to troubleshoot the problem. That makes logical sense, but I’ve never run into it.

    I’m not shy about multiple asserts, although I typically (2 known exceptions) don’t use them to replace multiple tests. I’ll assert a pre condition, make a change, and assert the post condition. I look at it kind of like a proof.

    The two exceptions are property testers: Testing properties is largely a formality if they don’t have any logic. But, you need tests to achieve the 100% branching coverage. And, you never know if someone is going to drop in some logic when you’re not expecting it. So, for properties, I end up with two tests. One that makes sure all of the properties initialize to proper expected values when the object is created, and another to make sure all of the gets and sets work. If there are 10 properties, I have 2 methods with 10 asserts each. Those could be individual tests, and maybe they should be, I just can’t bring myself to do it for automatic properties.

    I do like row tests, though, quite a bit. I agree it can affect readability, but that can be managed. In many cases, the tests would all be identical except for the input and output values. You could move the common logic to a helper method, and have multiple tests call the same helper. But, I like the row tests, so go that route. (I used the helper method before I knew there was such a thing as a row test).

    Reply

Leave a comment