Wednesday, June 4, 2008

On Being Out-Babooned

I'm being out-babooned. A few months ago I decided it might be a good idea to snag the codebaboon.com domain, only to realise that someone else (Mark Theunissen apparently) had registered it literally days earlier. A little frustrated by my lack of initiative, I figured it was no big deal, that this new baboon would probably just do nothing with it or put up some craptastic site that I could mock now and again.

Boy was I wrong. Not to pimp some stranger's site, but check it out. Pretty nice looking, eh? He's got a cool little baboon logo that kind of looks like him, and a very nice, clean style. He shows up first in the Google search for code baboon, and has far more comments on his posts (which means he has a few readers). And to top it all off the bastard even has a photo of himself and a baboon!

Looks like I need to step it up. My frequency of posts has been increasing lately, so that's a start. I'm not sure what more I can do to make Blogger look sexy, so maybe I need to look into registering www.therealcodebaboon.com or something. Most importantly, I'll be heading to the Zoo this weekend to steal a baboon for a night on the town. I'm going to get photos of us having a drink on a patio, playing flag football in a park, riding a tandem bike, and high-fiving after scoring the winning touchdown in a game of flag football. Eat that, Mark.

Monday, June 2, 2008

Oops, I Did It Again...

By 'it', I mean Assert. But lets step back.

Following a dry run for Ryan Weppler's upcoming webcast Testing Out the MVC Routing, a few guys got into a discussion about multiple Assert statements in a unit test. Steve 'asserted' (oh yeah, I just went there) that you should only ever have 1 Assert per unit test (edit: from Steve per the comments: "the one Assert per test thing is just best practice, not a written-in-stone sort of thing. As long as you have a good reason for doing it, then fill your boots. Just ask yourself why you're doing it."). I was attending remotely and didn't have a mic, so I could not respond, which turned out to be a good thing. You see, I wanted to say something like "I have a bunch of tests from my demo code that used multiple Asserts, and they all were necessary/made sense". Because I couldn't speak, I decided to send an email to that effect.

But the good thing about email is that it generally encourages you to back up your random statements with crazy things like facts and examples, so I popped open the Dice Hockey code and looked at my tests. Sure enough a bunch have more than one Assert. Yet as I gazed upon those 'perfect' tests with the intent to throw them in Steve's metrosexually attractive visage, I realised that I couldn't do it. In each case, Steve was right. Damn him and the Mac he rode in on!

Anyways, et's take a look for ourselves at a function from one of my tests:

private void EnsureAllDesiredPlayersWereRetrieved(
                IEnumerable<Guid> playerIds)
{
    Assert.AreEqual(playerIdsToRetrieve.Length, 
        playerIds.Count(), "Not all players retrieved!");
 
    IEnumerable<Guid> playersNotRetrieved = 
        (from ptr in playerIdsToRetrieve
         select ptr).Except(playerIds);
 
    Assert.IsTrue(playersNotRetrieved.Count() == 0, 
       "Not all players retrieved!");
}

In this first example, the first Assert is meaningless. It only has value because I know that during my data initialization at the start of the test I put more players into the database than I did in my list of ID's to retrieve. But really, the second Assert is the one that makes sure everyone got retrieved that should have been. If I want to test that no other players were retrieved (which is basically what that first Assert is doing), I should have another test.

And hey, speaking of that, I do have another test for that! Well, sort of. I have another function, as seen here:

private void EnsureNoPlayersWereRetrievedThatShouldNotHaveBeen(
            IEnumerable<Guid> playerIds)
{
    IEnumerable<Guid> playersThatShouldntHaveBeenRetrieved = 
       (from p in playerIds
        join ptnr in playerIdsToNotRetrieve on p equals ptnr
        select p);
 
        Assert.IsTrue(
playersThatShouldntHaveBeenRetrieved.Count() == 0,
          "Players were retrieved that should not have been!");
}

That looks nice, right? But check out where these functions get used:

[TestMethod]
public void Should_retrieve_only_specified_players_from_Players_table()
{
    var retrievedPlayers = 
        testGateway.GetPlayers(playerIdsToRetrieve);
 
    // Check that all desired players were retrieved
    EnsureAllDesiredPlayersWereRetrieved(
        (from rp in retrievedPlayers
         select rp.PlayerId));
 
    // Check that no players were retrieved that 
    // should not have been
    EnsureNoPlayersWereRetrievedThatShouldNotHaveBeen(
        (from rp in retrievedPlayers
         select rp.PlayerId));
}

That's a pretty clear-cut case of bad test writing; those should be two seperate tests, one for each function. Combine that with the clean-up of the two Asserts in that first function and we'd have a much nicer solution.

UPDATE: Code is all visable now, sorry for the formatting issues.