Don’t jump the red light!
Posted by Graeme | Posted in Development | Posted on 15-02-2010
1
There’s a good reason why the test-driven development cycle says you should always watch a test fail before you write the production code that makes it pass. I was taught a lesson in this today, “school of hard knocks” style…
How did it happen?
I have a simple class which implements INotifyPropertyChanged and has property:
public class MyClass : INotifyPropertyChanged
{
private string _name;
public string Name
{
get { return _name; }
set
{
_name = value;
PropertyChanged(this, new PropertyChangedEventArgs("Name"));
}
}
public event PropertyChangedEventHandler PropertyChanged = delegate { };
}
To test that the PropertyChanged event is raised at the appropriate time I use Caliburn’s handy (and very readable) PropertyHasChangedAssertion:
[TestFixture]
public class MyClassTestFixture
{
[Test]
public void Name_WhenSet_RaisesPropertyChanged()
{
var test = new MyClass();
test.AssertThatChangeNotificationIsRaisedBy(x => x.Name);
}
}
At the time I obviously thought this was too simple to worry about, saw the test passed as expected and moved on. All good… Or so it seemed!
Fast forward a week or two. I started to get some strange errors – not test failures – in my MSBuild output:
error : Internal error: An unhandled exception occurred.
error : System.Exception: No context was provided to test the notification, use When(Action affectProperty) to provide a context.
error : at Caliburn.Testability.Assertions.PropertyHasChangedAssertion`2.Finalize()
Not only is the message a bit cryptic without any context (e.g. a test) but the error was intermittent. Oh joy! After a bit of detective work (more than you might think!) I realised that this is because the Caliburn’s PropertyHasChangedAssertion checks that you called its When(Action affectProperty) method in its finalizer:
~PropertyHasChangedAssertion()
{
if(!_isValidAssertion)
throw new Exception(
"No context was provided to test the notification, use When(Action affectProperty) to provide a context.");
}
While this makes the test extremely readable (which is, of course, extremely important), if you forget to call When(), if and when you get an error is up to the non-deterministic finalization gods. An easy one to fix:
[Test]
public void Name_WhenSet_RaisesPropertyChanged()
{
var test = new MyClass();
test.AssertThatChangeNotificationIsRaisedBy(x => x.Name)
.When(() => test.Name = "New name");
}
But I didn’t get the feedback that I should have done from doing TDD properly, and wasted time as a result.
What went wrong?
Here is how TDD is supposed to be performed:
Because the code was trivial, I skipped the second step and didn’t check that the test failed before I carried on and implemented the property. If I hadn’t skipped this step, there’s a good chance that the following sequence of events would have occurred:
- I write the test, and add an empty property definition to make it compile
- I run the test and see that it unexpectedly passes
- I scratch my head for a bit, but I’m already looking at the offending line of code so it’s much easier to spot the problem
- The penny drops, I spot the mistake and fix it
- I run the test again, and this time it fails. Happy days.
- I implement the property changed notification and carry on
Maybe I would have got the error when I ran the test, and that would have given me another clue about the cause of the problem.
It could have been worse!
In the week or two since I made the mistake I could have carried on to use AssertThatChangeNotificationIsRaisedBy in tens or hundreds of other tests, which would have made it much harder to find the one with the missing When() call. I was lucky that there were only a few uses in my tests.
A lesson learned?
I hope so! When time is short it can be hard to make yourself go through these steps over and over again, but they are all there for a reason – to stop us writing code that doesn’t do what we think it does. I will be trying especially hard to stick to the steps, but we’ll have to wait and see how it goes.
