DaedTech

Stories about Software

Sep
23

By

Unit Testing DateTime.Now Without Isolation

Category: .NET Tags: , | 4 Comments

My friend Paul pointed something out to me the other day regarding my post about TDD even when you have to discard tests. I believe that this trick was taken from the book Working Effectively With Legacy Code by Michael Feathers (though I haven’t yet read this one, so I can’t be positive.

I was writing some TDD test surrounding the following production method:

public virtual void SeedWithYearsSince(DropDownList list, int year)
{
    for (int index = year; index <= DateTime.Now.Year; index++)
        list.Items.Add(new ListItem(index.ToString()));
}

and the problem I was having is that any tests that I write and check in will be good through the end of 2012 and essentially have an expiration date of Jan 1st, 2013.

What Paul pointed out is that I could refactor this to the following:

protected virtual int CurrentYear
{
    get
    {
        return DateTime.Now.Year;
    }
}

public virtual void SeedWithYearsSince(DropDownList list, int year)
{
    for (int index = year; index <= CurrentYear; index++)
          list.Items.Add(new ListItem(index.ToString()));
    
}

And, once I've done that, I can introduce the following class into my test class:

public class CalenderDropDownFillerExtension : CalendarDropdownFiller
{
    private int _currentYear;
    protected override int CurrentYear
    {
        get
        {
            return _currentYear;
        }
    }

    public CalenderDropDownFillerExtension(DateTimeFormatInfo formatInfo, int yearToUse) : base(formatInfo)
    {
        _currentYear = yearToUse;
    }
            
}

With all that in place, I can write a test that no longer expires:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Adds_Two_Items_When_Passed_2011()
{
    var filler = new CalenderDropDownFillerExtension(new DateTimeFormatInfo(), 2012);
    var list = new DropDownList();
    filler.SeedWithYearsSince(list, 2011);

    Assert.AreEqual(2, list.Items.Count);
}

In this test, I use the new class that requires me to specify the current year. It overrides the base class, which uses DateTime.Now in favor of the "current" year I've passed it, which has nothing to do with the non-deterministic quantity "Now". As a result, I can TDD 'til the cows come home and check everything in so that nobody accuses me of having a Canadian girlfriend. In other words, I get to have my cake and eat it too.

Sep
14

By

TDD Even with DateTime.Now

Category: .NET Tags: , , |

Recently, I posted my incredulity at the idea that someone would go to the effort of writing unit tests and not source control them as a matter of course. I find that claim as absurd today as I did then, but I did happen on an interesting edge case where I purposely discarded a unit test I wrote during TDD that I would otherwise have kept.

I was writing a method that would take a year in integer form and populate a drop down list with all of the years starting with that one up through the current year. In this project, I don’t have access to an isolation framework like Moles or Typemock, so I have no way of making DateTime.Now return some canned value (check out this SO post for an involved discussion of the sensitivity of unit tests involving DateTime.Now).

So, as I thought about what I wanted this method to do, and how to get there, I did something interesting. I wrote the following test:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Adds_Item_When_Passed_2012()
{
    var myFiller = new CalendarDropdownFiller(new DateTimeFormatInfo());
    var myList = new DropDownList();
    myFiller.SeedWithYearsSince(myList, 2012);

    Assert.AreEqual(1, myList.Items.Count);
}

To get this to pass, I changed the method SeedWithYearsSince() to add a random item to the list. Next test I wrote was:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Adds_Item_With_Text_2012_When_Passed_2012()
{
    var myFiller = new CalendarDropdownFiller(new DateTimeFormatInfo());
    var myList = new DropDownList();
    myFiller.SeedWithYearsSince(myList, 2012);

    Assert.AreEqual("2012", myList.Items[0].Value);
}

Now, I had to actually add “2012” in the method, but it was still pretty obtuse. To get serious, I wrote the following test:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Adds_Two_Items_When_Passed_2011()
{
    var myFiller = new CalendarDropdownFiller(new DateTimeFormatInfo());
    var myList = new DropDownList();
    myFiller.SeedWithYearsSince(myList, 2011);

    Assert.AreEqual(2, myList.Items.Count);
}

Now the method had to do something smart, so I wrote:

public virtual void SeedWithYearsSince(DropDownList list, int year)
{
    for (int index = year; index <= DateTime.Now.Year; index++)
        list.Items.Add(new ListItem(index.ToString()));
}

And, via TDD, I got to the gist of my method correctly. (I would later write tests that passed in a null list and a negative year and test that descriptive exceptions were thrown, but this is more or less the finished product). But now, let's think about the unit tests vis a vis source control.

Of the three tests I've written, the first two should always pass unless I get around to finishing the time machine that I started building a few years back. We might consolidate those into a single test that's a little more meaningful, perhaps by dropping the first one. We might also tease out a few more cases here to guard against regressions, say proving that calling it with 2010 adds 2010, 2011 and 2012 or something. While I don't generally feel good about checking in tests that exercise code dependent on external state (like "Now"), we can feel pretty good about these given the nature of "Now".

But that last test about 2 items when passed 2011 is only good for the remainder of 2012. When you wake up bright and early on New Year's morning and run to the office and kick off a test run, this test will fail. Clearly we don't want to check that test in, so all things being equal, we'll discard it. That's a bummer, but it's okay. The point of the unit tests written here was a design strategy -- test driven development. If we can't keep the artifacts of that because, say, we don't have access to an isolation framework or permission ot use one, it's unfortunate, but c'est la vie. We'll check in the tests that we can and call it a day.

This same reasoning applies within the context of whatever restrictions are placed on you. Say you are assigned to a legacy codebase (using the Michael Feathers definition of "legacy" as code without unit tests) and do not have rights to add a test project, for whatever reason. Well, then write them to help you work, keep them around as best you can to help for as long as you can, and discard them when you have to. If you have a test project but not Moles or Typemock, you do what we did here. If you have code that you have to use that lacks seams, contains things like singeltons/static methods or otherwise presents testability problems, take the same approach. Better to test during TDD and discard then not to test at all since you can at least guard against regressions and get fast feedback during initial development.

I've often heard people emphasize that TDD is a development methodology first and the unit tests for source control are a nice ancillary benefit. But I think the example of DateTime.Now really drives home that point. The fact that DateTime.Now (or legacy code, GUI code, threaded code, etc) is fickle and hard to test need not be a blocker from doing TDD. Clearly I think we should strive to write only meaningful tests and to keep them all around, but this isn't an all or nothing proposition. Make sure you're verifying your code first and foremost, preserve what you can, and seek to improve through increasingly decoupled code, better tooling, and more practice writing good tests.

Sep
7

By

Why the Statement “I Don’t Source Control My Unit Tests” Makes Me Happy

Category: Anti Patterns Tags: , , | 12 Comments

An Extraordinary Claim

I was talking to a friend the other day, and he relayed a conversation that he’d had with a co-worker. They were discussing writing unit tests and the co-worker claimed that he writes unit tests regularly, but discards them and never checks them into source control. I was incredulous at this, so I asked what the rationale was, to which my friend cited his co-worker’s claims (paraphrased) that “[unit tests] are fragile, they take too long to maintain, they prohibit adapting to new requirements, errors are caused by user change requests not incorrect code, and they’re hard to understand.” (emphasis mine, and I’ll return to this later).

This struck me as so bizarre — so preposterous — that I immediately assumed I was out of my league and had missed an industry sea-change. I remember a blog post by Scott Hanselman entitled “I’m A Phony. Are You?” and I felt exactly what he meant. Clearly, I’d been exposed. I was not only wrong about unit tests, but I was 180 degrees, polar opposite, comically wrong. I had wasted all of this time and effort with TDD, unit test maintenance, unit tests in the build, etc when what I really should have been doing was writing these things once and then tossing them since their benefit is outweighed by their cost.

Given that I don’t have direct access to this co-worker, I took to the internet to see if I could locate the landmark post or paper that had turned the tide on this subject and no doubt a whole host of other posts and papers following suit. So, I googled “Don’t Check In Unit Tests”. Your mileage may vary because I was logged into gmail, but I saw old posts by Steve Sanderson and Jeff Atwood encouraging people to write tests, a few snarky posts about not testing, and other sorts of things you might expect. I had missed it. I tried “Don’t version control unit tests” and saw similar results. A few variants on the same and similar results too. Apparently, most of the internet was still thinking inside of the box that the co-worker had escaped with his bold new vision.

Finally, I googled “Should I source control unit tests?” and found one link from programmer’s stack exchange and another from stackoverflow. In a quick and admittedly very rough count of people who had answered and/or voted one way or the other, the vote appeared to be about 200 to 0 for source controlling versus leaving out, respectively. The answer that most succinctly seemed to summarize the unanimous consensus was Greg Whitfield’s, which started with:

Indeed yes. How could anyone ever think otherwise?

Hmmm…

There’s Really No Argument

With my confidence somewhat restored, I started thinking of all the reasons that source controlling unit tests makes sense:

  1. Provides living ‘documentation’ of intentions of class author.
  2. Prevents inadvertent regressions during changes.
  3. Allows fearless refactoring of your code and anyone else’s.
  4. Incorporation into the build for metric and verification purposes.
  5. Testing state at the time of tagged/branched versions can be re-created.
  6. (As with source controlling anything) Hard drive or other hardware failure does not result in lost work.

I could probably go on, but I’m not going to bother. Why? Because I don’t think that the rationale are actually rationale for not checking in unit tests. I think it’s more likely that this person who “writes tests but doesn’t actually check them in” might have had a Canadian girlfriend as a teenager. In other words, I sort of suspect that these “disposable” unit tests, whose existence can neither be confirmed nor denied, may not necessarily actually exist and so the rationale for not checking them in becomes, well, irrelevant.

And, look at the rationale. The first three clauses (fragile, too long to maintain, prohibit new work) seem to be true only for someone who writes really bad unit tests (i.e. someone without much practice who may, I dunno, be discouraged during early attempts). Because while it’s true that unit tests, like any other code, constitute a maintenance liability, they need not be even remotely prohibitive, especially in a nicely decoupled code base. The fourth clause (errors are everyone’s fault but the programmer) is prima facie absurd, and the fifth and most interesting clause seems to be an unwitting admission of the problem – a difficulty understanding how to unit test. This is probably the most important reason for “not checking in” or, more commonly and accurately, not writing unit tests at all — they’re hard when you don’t know how to write them and haven’t practiced it.

So Why the Happy?

You’re probably now wondering about the post title and why any of this would make me happy. It’s simply, really. I’m happy that people feel the need to make excuses (or phantom unit tests) for why they don’t automate verification of their work. This indicates real progress toward what Uncle Bob Martin describes in a talk that he does called “Demanding Software Professionalism” and alludes to in this post. In his talk, Bob suggests that software development is a nascent field compared to others like medicine and accounting and that we’re only just starting to define what it means to be a software professional. He proposes automated verification in the form of TDD as the equivalent of hand-washing or double-entry bookkeeping respectively in these fields and thinks that someday we’ll look back on not practicing TDD and clean coding the way we now look back on surgeons that refused to wash their hands prior to surgery.

But having a good idea and even demonstrating that it works isn’t enough. Just ask Ignaz Semmelweis who initially discovered, and empirically demonstrated that hand-washing reduced surgery mortality rates, only to be ridiculed and dismissed by his peers in the face of cold-hard evidence. It wasn’t until later, after Semmelweis had been committed to an insane asylum and died that his observations and hypothesis got more backers (Lister, Pasteur, et al) and a better marketing campaign (an actual theoretical framework of explanation called “Germ Theory”). In Semmelweis’s time, a surgeon could just call him a crank and refuse to wash his hands before surgery. Decades later, he would have to say, “dude, no, I totally already washed them when you weren’t looking” if he was feeling lazy. You can even ask his Canadian girlfriend.

At the end of the day, I’m happy because the marketing for TDD and clean coding practice must be gaining traction and acceptance if people feel as though they have to make excuses for doing it. I try to be a glass half full kind of guy, and I think that’s the glass half full outlook. I mean one person not wanting to automate tests doesn’t really matter in the scheme of things at the moment since there is no shortage of people who also don’t want to, but it’s good to see people making excuses/stories for not wanting to rather than just saying “pff, waste of time.”

(And, for what it’s worth, I do acknowledge the remote possibility that someone actually does write and discard unit tests on a regular and rigorous basis. I just don’t think it’s particularly likely.)

Jul
3

By

TDD Prevents Copy and Paste Programming

Category: Anti Patterns Tags: , |

Copy, Paste, Fail, Repeat

Today, I was reviewing some code, and I saw a “before” that looked like this:

private void RefreshClassifierValues(Array NewValues)
{
    List ClassifiersList = Classifiers;
    int i = 0;
    foreach (Classifier classifier in ClassifiersList)
    {
        classifier.Value = NewValues.GetValue(i++);
    }
}

The first thing that stuck out to me was that I really wanted to stick “var” in there a couple of times because I read the word “classifier” so many time it lost all meaning. The second thing that struck me was the after:

private void RefreshClassifierValues(Array NewValues)
{
    List ClassifiersList = Classifiers;
    int i = 0;
    foreach (Classifier classifier in ClassifiersList)
    {
        classifier.Value = NewValues.GetValue(i++);
    }
}

private void RefreshClassifierEnabledStatus(bool value)
{
    List ClassifiersList = Classifiers;
    int i = 0;
    foreach (Classifier classifier in ClassifiersList)
    {
        classifier.Enabled = value;
    }
}

If you’d like to play a game of “find the compiler warning”, by all means, go ahead, but I warn you — the next sentence is a spoiler. In the new code, “i” is never read anywhere, which will generate a warning about unused variables. Annoying, but not the end of the world, and a fairly quick fix.

But, how did the code get like this? I’ve isolated the change in a way that should make this fairly obvious if you think about it for a minute or two. Basically, someone took the first method, copy and pasted it, and modified the payload of the foreach to suit his or her needs. Of course, the new need didn’t involve reading the local index variable, so oops.

I talked with the developer whose code change it was, doing my due diligence in pointing out that this is a classic example of one of the many problems with copy/paste programming. I tried to think back to a time when I’d been burned by this sort of thing recently, when it dawned on me that this simply wouldn’t happen to me. I say that not because I’m some kind of purist that disables ctrl-V and never pastes any code anywhere (though I do keep this to a pretty strict minimum), but rather because this is simply a non-starter for someone who practices TDD.

TDD Is Full of Win

If I were going to write the method RefreshClassifierStatus, the first test that I would write is that Classifier[0].Enabled was false when I passed in false. That would go red, and then I’d write some obtuse code that set Classifier[0].Enabled to false. Next, I’d write a test that said it was true when I passed in true and, after failing, I’d set that property to the passed in value. Finally, I’d write a test that Classifier[Classifier.Count – 1].Enabled was true when true was passed in and suddenly I’d have a foreach loop executing properly.

As an aside, at this point, I’d set the list to null and write a test for how the method should behave when that happens. This seems not to have been considered in this code, which is another problem with copy/paste programming — it’s a multiplier for existing mistakes.

So where in this, exactly, do I introduce an unused variable? That’s never going to help me get any test to go green. Where in this do I ever copy and paste the method wholesale? That’s not the simplest thing that I can do. So, with these two guiding heuristics, followed rather strictly, it’s simply impossible for me to introduce needless noise into the code. TDD forces an impressive level of efficiency most of the time.

TDD is Full of Fast

Had I written this method, I would have done it in probably 5 minutes or so (and that’s a fairly pessimistic estimate, I think), generating 4 unit tests for my trouble. The person that wrote this, I’m guessing, spent 30 seconds or less doing it. So, I’m behind by 4.5 minutes. But, let’s add to that the time that this developer will spend after seeing the code review firing back up the development environment, navigating to the code, removing the line of code, re-compiling, and re-running to make sure that it’s still okay. I’m now only maybe 2 minutes behind.

Now, let’s also factor in that I just realized that problem about the null check, which I’m going to email over and suggest be fixed in the new and old methods. Now there are two test cases instead of 1, and I’m suddenly ahead in terms of development time.

Okay, so the time here is a bit contrived and it’s not as though mistakes can’t be made with TDD, but I can say that a lot fewer of them are. The point here is that copy/paste programming is supposed to be super fast. It’s what you do when there’s no time for good software development practice because you need to hurry. And yet, it’s really not that fast in the grand scheme of things. It just makes you busier and wrong more efficiently than being wrong by hand.

So, save yourself time, effort, and the headache of getting a reputation for sloppy work. Slow down and do it right. TDD isn’t required for this, but it sure helps.

Apr
27

By

Mock.Of() and Mock.Get() in Moq

Category: .NET Tags: , , , , | 6 Comments

Today, I’d like to highlight a couple of features of Moq that I didn’t know about until relatively recently (thanks to a recent google+ hangout with Moq author, Daniel Cazzulino). Since learning about these features, I’ve been getting a lot of mileage out of them. But, in order to explain these two features and the different paradigm they represent, let me reference my normal use of Moq.

Let’s say we have some class PencilSharpener that takes an IPencil and sharpens it, and we want to verify that this is accomplished by setting the Pencil’s length and sharpness properties:

public void Sharpen_Sets_IsSharp_To_True()
{
    var myPencilDouble = new Mock();
    myPencilDouble.SetupProperty(pencil => pencil.IsSharp);
    myPencilDouble.Object.IsSharp = false;
    myPencilDouble.SetupProperty(pencil => pencil.Length);
    myPencilDouble.Object.Length = 12;

    var mySharpener = new PencilSharpener();
    mySharpener.Sharpen(myPencilDouble.Object);

    Assert.IsTrue(myPencilDouble.Object.IsSharp);
}

So, I create a test double for the pencil, and I do some setup on it, and then I pass it into my sharpener, after which I verify that the sharpener mutates it in an expected way. Fairly straight forward. I create the double and then I manipulate its setup, before passing its object in to my class under test. (Incidentally, I realize that I could call “SetupAllProperties()”, but I’m not doing that for illustrative purposes).

But, sometimes I’d rather not think of the test double as a double, but just some object that I’m passing in. That is, perhaps I don’t need to invoke any setup on it, and I just want to reason about the actual proxy implementation, rather than stub.object. Well, that’s where Mock.Of<>() comes in:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Sharpen_Sets_IsSharp_To_True()
{
    var myPencil = Mock.Of();
    myPencil.IsSharp = false;

    var mySharpener = new PencilSharpener();
    mySharpener.Sharpen(myPencil);

    Assert.IsTrue(myPencil.IsSharp);
}

Much cleaner, eh? I never knew I could do this, and I love it. In many tests now, I can reason about the object not as a Mock, but as a T, which is an enormous boost to readability when extensive setup is not required.

Ah, but Erik, what if you get buyer’s remorse? What if you have some test that starts off simple and then over time and some production cycles, you find that you need to verify it, or do some setup. What if we have the test above, but the Sharpen() method of PencilSharpener suddenly makes a call to a new CanBeSharpened() method on IPencil that must suddenly return true… do we need to scrap this approach and go back to the old way? Well, no, as it turns out:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Sharpen_Sets_IsSharp_To_True()
{
    var myPencil = Mock.Of();
    myPencil.IsSharp = false;
    Mock.Get(myPencil).Setup(pencil => pencil.CanBeSharpened()).Returns(true);

    var mySharpener = new PencilSharpener();
    mySharpener.Sharpen(myPencil);

    Assert.IsTrue(myPencil.IsSharp);
}

Notice the third line in this test. Mock.Get() takes some T and grabs the Mock containing it for you, if applicable (you’ll get runtime exceptions if you try this on something that isn’t a Mock’s object). So, if you want to stay in the context of creating a T, but you need to “cheat”, this gives you that ability.

The reason I find this so helpful is that I tend to pick one of these modes of thinking and stick with it for the duration of the test. If I’m creating a true mock with the framework — an elaborate test double with lots of customized returns and callbacks and events — I prefer to instantiate a new Mock(). If, on the other hand, the test double is relatively lightweight, I prefer to think of it simply as a T, even if I do need to “cheat” and make the odd setup or verify call on it. I find that this distinction aids a lot in readability, and I’m taking full advantage. I realize that one could simply retain a reference to the Mock and another to the T, but I’m not really much of a fan (though I’m sure I do it now and again). The problem with that, as I see it, is that you’re maintaining two levels of abstraction simultaneously, which is awkward and tends to be confusing for maintainers (or you, later).

Anyway, I hope that some of you will find this as useful as I did.

By the way, if you liked this post and you're new here, check out this page as a good place to start for more content that you might enjoy.
« Older Posts
Newer Posts »