Showing posts with label TDD. Show all posts
Showing posts with label TDD. Show all posts

Thursday, September 08, 2011

Guided by Tests–Day Two

This post is third in a series about a group TDD experiment to build an application in 5 days using only tests.  Read the beginning here.

Today we break new ground on our application, starting with writing our first test. Today is still a teaching session, where I’ll write the first set of tests to demonstrate naming conventions and how to demonstrate how to use TDD with the rules we defined the day before. But first, we need to figure out where we should start.

Logical Flow

In order to determine where we should start it helps to draw out the logical flow of our primary use case: create a new dependency viewer from an NDepend AssembliesDependencies.xml file.  The logical flow looks something like this:

LogicalFlowDiagram

  1. User clicks “New”
  2. The user is prompted to select a file
  3. Some logical processing occurs where the file is read,
  4. …a graph is produced,
  5. …and the UI is updated.

The question on where to start is an interesting one. Given limited knowledge of what we need to build or how these components will interact, what area of the logical flow do we know the most about? What part can we reliably predict the outcome?

Starting from scratch, it seemed the most reasonable choice was to start with the part that reads our NDepend file. We know the structure of the file and we know that the contents of the file will represent our model.

Testing Constraints

When developing with a focus on testability, there are certain common problems that arise when trying to get a class under the test microscope. You learn to recognize them instantly, and I’ve jokingly referred to this as spidey-sense – you just know these are going to be problematic before you start.

While this is not a definitive list, the obvious ones are:

  • User Interface: Areas that involve the user-interface can be problematic for several reasons:
    • Some test-runners have a technical limitation and cannot launch a user-interface based on the threading model.
    • The UI may require complex configuration or additional prerequisites (style libraries, etc) and is subject to change frequently
    • The UI may unintentionally require human interaction during the tests, thereby limiting our ability to reliably automate.
  • File System: Any time we need files or folder structure, we are dependent on the environment to be setup a certain way with dummy data.
  • Database / Network: Being dependent on external services is additional overhead that we want to avoid. Not only will tests run considerably slower, but the outcome of the test is dependent on many factors that may not be under our control (service availability, database schema, user permissions, existing data).

Some of the less obvious ones are design considerations which may make it difficult to test, such as tight coupling to implementation details of other classes (static methods, use of “new”, etc).

In our case, our first test would be dependent on the file system. We will likely need to test several different scenarios, which will require many different files.  While we could go this route, working with the file system directly would only slow us down. We needed to find a way to isolate ourselves.

The team tossed around several different suggestions, including passing just xml as string. Ultimately, as this class must read the contents of the file we decided that the best way to work with Xml was an XmlReader. We could simulate many different scenarios by setting up a stream containing our test data.

Our First Test

So after deciding that the name of our class would be named NDependStreamParser, our first test looked something like this:

using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace DependencyViewer
{
    [TestClass]
    public class NDependStreamParserTests
    {
        [TestMethod]
        public void TestMethod1()
        {
            Assert.Fail();
        }
    }
}

We know very little about what we need. But at the very least, the golden rule is to ensure that all tests must fail from the very beginning. Writing “Assert.Fail();” is a good habit to establish.

In order to help identify what we need, it helps to work backward. So we start by writing our assertions first and then, working from the bottom up, fill in the missing pieces.  Our discovery followed this progression:

Realization Code Written
At the end of the tests, we’ll have some sort of results. The results should not be null.

At this point, test compiles, but it’s red.
Test Code:
object results = null;
Assert.IsNotNull( results,
           “Results were not produced.”);
Where will the results come from? We’ll need a parser. The results will come after we call Parse.

The code won’t compile because this doesn’t exist. If we use the auto-generate features of Visual Studio / Resharper the test compiles, but because of the default NotImplementedException, the test fails.
Test Code:
var parser = new NDependStreamParser(); 
object results = parser.Parse();
Assert.IsNotNull( results,
           “Results were not produced.”);
We need to make the test pass.

Do whatever we need to make it green.
Implementation:

public object Parse()
{
    // yes, it's a dirty hack. 
    // but now the test passes.
    return new object();
}
Our tests passes, but we’re clearly not done. How will we parse? The data needs to come from somewhere. We need to read from a stream.

Introducing the stream argument into the Parse method won’t compile (so the tests are red), but this is a quick fix in the implementation.
Test Code:

var parser = new NDependStreamParser();
var sr = new StringReader("");
var reader = XmlReader.Create(sr);
object results = parser.Parse(reader);
// ...

Implementation:

public object Parse(XmlReader reader) { //...
Our return type shouldn’t be “Object”. What should it be?

After a short review of the NDepend AssembliesDependencies.xml file, we decide that we should read the list of assemblies from the file into a model object which we arbitrarily decide should be called ProjectAssembly. At a minimum, Parse should return an IEnumerable<ProjectAssembly>.

There are a few minor compilation problems to address here, including the auto-generation of the ProjectAssembly class. These are all simple changes that can be made in under 60 seconds.
Test Code:

var parser = new NDependStreamParser();
var sr = new StringReader("");
var reader = XmlReader.Create(sr);
IEnumerable<ProjectAssembly> results 
    = parser.Parse(reader);
// ...

Implementation:

public IEnumerable<ProjectAssembly> Parse(
	XmlReader reader)
{
    return new List<ProjectAssembly>();
}

At this point, we’re much more informed about how we’re going to read the contents from the file. We’re also ready to make some design decisions and rename our test accordingly to reflect what we’ve learned. We decide that (for simplicity sake) the parser should always return a list of items even if the file is empty. While the implementation may be crude, the test is complete for this scenario, so we rename our test to match this decision and add an additional assertion to improve the intent of the test.

Sidenote: The naming convention for these tests is based on Roy Osherove’s naming convention, which has three parts:

  • Feature being tested
  • Scenario
  • Expected Behaviour
[TestMethod]
public void WhenParsingAStream_WithNoData_ShouldProduceEmptyContent()
{
    var parser = new NDependStreamParser();
    var sr = new StringReader("");
    var reader = XmlReader.Create(sr);

    IEnumerable<ProjectAssembly> results =
        parser.Parse(reader);

    Assert.IsNotNull( results,
        "The results were not produced." );

    Assert.AreEqual( 0, results.Count(),
        "The results should be empty." );
}

Adding Tests

We’re now ready to start adding additional tests. Based on what we know now, we can start each test with a proper name and then fill in the details.

With each test, we learn a little bit more about our model and the expected behaviour of the parser. The NDepend file contains a list of assemblies, where each assembly contains a list of assemblies that it references and a list of assemblies that it depends on. The subsequent tests we wrote:

  • WhenParsingAStream_ThatContainsAssemblies_ShouldProduceContent
  • WhenParsingAStream_ThatContainsAssembliesWithReferences_EnsureReferenceInformationIsAvailable
  • WhenParsingAStream_ThatContainsAssembliesWithDependencies_EnsureDependencyInformationIsAvailable

It’s important to note that these tests aren’t just building the implementation details of the parser, we’re building our model object as well. Properties are added to the model as needed.

Refactoring

Under the TDD mantra “Red, Green, Refactor”, “Refactor” implies that you should refactor the implementation after you’ve written the tests. However, the scope of the refactor should apply to both tests and implementation.

Within the implementation, you should be able to optimize the code freely assuming that you aren’t adding additional functionality. (My original implementation details of using the XmlParser was embarrassing, and I ended up experimenting with the reader syntax later that night until I found a clean elegant solution. The tests were invaluable for discovering what was possible.)

Within the tests, refactoring means removing as much duplication as possible without obscuring the intent of the test. By the time we started the third test, the string-concatenation to assemble our xml and plumbing code to create our XmlReader was copied and pasted several times. This plumbing logic slowly evolved into a utility class that used an XmlWriter to construct our test data.

Next: Day Three

submit to reddit

Wednesday, September 07, 2011

Guided by Tests–Day One

This post is second in a series about a group TDD experiment to build an application in 5 days using only tests.  Read the beginning here.

Today is the pitch. We'll talk about what we're going to build and how we're going to do it, but first we need to understand the motivation behind our methodology. We need to understand why we test.

Methodology

Kent Beck's TDD by Example was one of the first books I read about TDD and to this day it's still one of my favourites. I know many prefer Roy Osherove's Art of Unit Testing (which I also highly recommend) because it's newer and has .net examples, but in comparison they are very different books.  Roy's book represents the evolved practice of Unit Testing and it provides a solid path to understand testing and the modern testing frameworks that have taken shape since Kent's book was written in 2002. Kent's book is raw, the primordial ooze that walked upon land and declared itself sentient, it defines the methodology as an offshoot of extreme programming and is the basis for the frameworks Roy explains how to use. If you're looking for a book on how to start and the mechanics of TDD, this is it.

As an offshoot of extreme programming, the core philosophy of TDD is:

  • Demonstrate working software at all times
  • Improve confidence while making changes
  • Build only the software you need

My experiment is based on the strategies defined in Kent's book – the premise is to demonstrate working software and get early feedback – but in a nutshell, you make incredibly small incremental changes and compile and run tests a lot (30-50 times per hour). I sometimes refer to this rapid code/test cycle as the Kent Beck Method or Proper TDD. I don't necessarily follow this technique when I code, but it's a great way to start until you learn how to find your balance. Eventually, running tests becomes instinct or muscle memory – you just know when you should.

The Rules

For our experiment, we followed these rules:

1. Tests must be written first.

This means that before we do anything, we start by writing a test. If we need to create classes in order to get the test to compile, that is a secondary concern: create the test first, even if it doesn’t compile, then introduce what you need.

2. Code only gets written to satisfy a test.

This means that we write only the code that is needed to make a test pass. If while writing the code you're tempted to add additional functionality that you might need, don't succumb to writing that code now. Instead, focus on the current test and make a note for additional tests. This ensures that we only write the code that is needed.

3. Code must always compile (at least within 30 seconds of making a change)

This may seem like a difficult practice to adopt, but it is extremely valuable. To follow this rule, make small changes that can be corrected easily and compile frequently to provide early feedback. If you find yourself breaking a contract that takes twenty minutes to fix, you're doing it wrong.

4. Time between red/green must be very short (<1 minute, meaning rollback if you're not sure.)

This is also a very difficult rule to keep, but it forces you to recognize the consequences of your changes. You should know before you run the test whether it's going to pass or fail. Your brain will explode when you're wrong (and that’s a good thing).

The Problem

As mentioned in the first post, my current project uses NDepend to perform static analysis as part of our build process. The build generates a static image. As our solution analyzes over 80 assemblies, the text becomes illegible, and its somewhat difficult to read (image text has been blurred to hide assembly names).

Geez!

The two primary use cases I wanted the application to have:

  • Allow the user to select an NDepend AssembliesDependencies.xml file and display it as a graph
  • Once the graph has been rendered, provide an option to choose which assemblies should be displayed

The graph would be implemented using Graph#, and we could borrow much of the presentation details from Sacha Barber’s post. Using Graph# would provide a rich user-experience, all we’d need to do is implement the surrounding project framework.  Piece of cake!

I asked the team if there were any additional considerations that they would like to see. We would take some of these considerations into account when designing the solution. Some suggestions were made:

  • Multi-tab support so that more than one graph can be open at a time
  • Ability to color code the nodes
  • Ability to save a project (and load it presumably)

Getting Started

With the few remaining minutes in the lunch hour, we set up the project:

  1. Create a new WPF Project.  Solution Name: DependencyViewer, Project Name: DependencyViewer.
  2. Create a new Test Project, DependencyViewer.Tests
  3. Added a reference from the Test Project to the WPF Project.
  4. Deleted all automatically generated files:
    1. App.xaml
    2. MainWindow.xaml
    3. TestClass1.
  5. Renamed the default namespace of the Test Project to match the namespace of the WPF Application. For more info, read this detailed post which explains why you should.

While I have some preferences on third-party tools, etc – we’ll get to those as they’re needed.

Next: Day Two

submit to reddit

Tuesday, September 06, 2011

Guided By Tests

Last week I started a TDD experiment at work. I gathered some interest from management and then sought out participants with a very simple concept: build an application in 5 days, over lunch hour, using nothing but unit tests to drive development.

My next few posts will be part of a series that shares the details of that experiment.

Motivation

Over the years, I've had many conversations with colleagues about TDD. One of the most common comments made is that they don't know what to test or where to start. Another common theme is the desire to get into a project at the very beginning where all the project members have bought into the concept. I've always found this comment to be strange. It's a great comment and it makes perfect sense but -- why the beginning? Why not the middle or end of the project after the tests are put into place? I sense they're expressing two things. First they're expressing the obvious, that life for the development team would be much different if they had tests from the beginning. And secondly, they're interested in understanding how the team managed to establish and sustain the process.

The goal of my experiment would show how to start a project with TDD and to walk through the process of deconstructing requirements into testable components. The process would use tests as the delivery mechanism but would also showcase how to design software for testability using separation of concerns and SOLID OO design principles. Requirements and documentation would be deliberately vague, and the application would need to showcase common real world problems. The tricky part would be finding an application to build.

Fortunately, I was recently inspired by a post by Sacha Barber that provides a great example of how to use the open source graphing framework Graph# to build a graph application in WPF. I immediately saw how I could use this. My current project uses NDepend for static analysis and as part of the build process it produces an image of our dependency graph. With over 80 nodes and illegible text, the static image is mostly useless. However, the build process spits out the details for the graph as an XML file. With some simple xml parsing, I could use Sacha's example to build my own graph.

So rather than spending my weekend working on this application, why not turn this into a teaching exercise? (Ironically, I'm spending my weekends blogging about it)

Follow up posts:

submit to reddit

Friday, August 12, 2011

On Dependency Injection and Violating Encapsulation Concerns

For me, life is greatly simplified when dependencies are inverted and constructor injection is used to provide a clean mechanism to introduce dependencies to a class. It's explicit and easy to test.

Others however argue that some dependencies are private implementation details to the class and external callers shouldn't know about them. The argument is that exposing these dependencies through the constructor violates encapsulation and introduces coupling. This argument is usually coupled with resistance to using interfaces or classes with virtual methods.

From my experience, there are times when composition in the constructor makes sense but there's a fine line when constructor injection should be used. I want to use this post to elaborate on these arguments and provide my perspective on this debate.

Does Constructor Injection violate Encapsulation?

Does exposing the internal dependencies of a class in the constructor expose the implementation details of a class? If you are allowing callers to construct the class directly, then you are most certainly breaking encapsulation as the callers must posses the knowledge of how to construct your class.  However, regardless of the constructor arguments if callers know how to construct your class you are also coupling to a direct implementation and will undoubtedly create a test impediment elsewhere. There are some simple fixes for this (including more constructor injection or another inversion of control technique) which I'll outline later.

So if constructor injection violates encapsulation, when is it safe to use composition in the constructor?

It really depends on the complexity and relationship of the subject to its dependencies. If the internal dependencies are simple and contain no external dependencies there is likely no harm in using composition to instantiate them in the constructor (the same argument can be made for small static utility methods). For instance, a utility class that performs some translation or other processing activity with limited outcomes may work as a private implementation detail. Problems arise as soon as these dependencies take on further dependencies or produce output that influences the conditional logic of the subject under test. When this happens the argument to keep these as internally controlled dependencies becomes flawed and it may be necessary to upgrade the "private implementation detail" to an inverted dependency.

Before:

This example shows a class that has a private implementation detail that influences logic of the subject under test. The consequence of this design is that the internal details of the validator leak into the test specifications. This leads to very brittle tests as tests must concern themselves with object construction and validation rules -- any change to the object or validation logic will break the tests.

public class MyModelTranslator
{
    public MyModelTranslator()
    {
       _validator = new MyModelValidator();
    }

    public MyViewModel CreateResult(MyModelObject model)
    {
        var result = new MyViewModel();
        
        if (_validator.Validate( model ));
        {
             result.Id = model.Id;
        }
        else {
            result.State = MyResultState.Invalid;
        }
        return result;
    }
}

[Test]
public void WhenCreatingAResult_FromAVaidModelObejct_ShouldHaveSameId()
{
    var subject = new MyModelTranslator();

    // create model object with deep understanding how Id's
    // and expiry dates are related 
    var model = new MyModelObject()
    {
        Id = "S1402011"
        Expiry = Datetime.Parse("12/31/2011");
    };
    var result = subject.CreateResult( model );
    Assert.AreEqual("S1402011", result.Id);
}

After:

This example shows the above example corrected to use a constructor injected dependency that can be mocked. In doing so, the test is not encumbered with object creation or validation details and can easily simulate validate states without introducing brittleness.

public class MyModelTranslator
{
    public MyModelTranslator(MyModelValidator validator);
    {
       _validator = validator;
    }

    // ...
}

[Test]
public void WhenCreatingAResult_FromAVaidModelObejct_ShouldHaveSameId()
{
    var validatorMock = new Mock<MyModelValidator>();

    var subject = new MyModelTranslator(validatorMock.Object);
    var model = new MyModelObject()
    {
      Id = "Dummy"
    };

    // we no longer care how validation is done, 
    // but we expect validation to occur and can easily control
    // and test outcomes
    ValidatorMock.Setup( x => x.Validate( model )).Returns( true );

    var result = subject.CreateResult( model );
    Assert.AreEqual("Dummy", result.Id);
}

Combatting Coupling with Inversion of Control

As the above points out, if we expose the dependencies of a class in the constructor we move the coupling problems out of the class and into the callers. This is not good but it can easily resolved.

An example that shows that our controller class now knows about the MyModelValidator:

public class MyController
{
    public MyController()
    {
        _translator = new MyModelTranslator( new MyModelValidator() );
    }
}

More Constructor Injection

Ironically, the solution for solving coupling and construction problems is more constructor injection. This creates a Russian Doll where construction logic is deferred and pushed higher and higher up the stack resulting in a top level component which is responsible for constructing the entire object graph. While this provides an intuitive API that clearly outlines dependencies, it's tedious to instantiate by hand. This is where dependency injection  frameworks like Unity, StructureMap and others come in: they can do the heavy lifting for you. If done right, your application should only have well defined points where the dependency container is used.

public class MyController : IController
{
    public MyController(MyModelTranslator translator)
    {
        _translator = translator;
    }
}

Note that this assumes that the constructor arguments are abstractions -- either interfaces, abstract classes or classes with virtual methods. In effect, by exposing the constructor arguments we trade off the highly sealed encapsulated black box for an open for extensibility model.

Factories / Builders

In some cases, using Constructor Injection everywhere might not be a good fit. For example, if you had a very large and complex object graph, creating everything upfront using constructor injection might represent a performance problem. Likewise, not all parts of the object graph will be used immediately and you may need to defer construction until needed.  In these cases, the good ol' Gang of Four Factory pattern is a handy mechanism to lazy load components.

So rather that construct the entire object graph and pass resolved dependencies as constructor arguments, pass the factory instead and create the lower-level sub-dependencies when needed.

public class MyControllerFactory : IControllerFactory
{
    private IUnityContainer _container;
    
    public MyControllerFactory(IUnityContainer container)
    {
        _container = container;
    }

    public IController Create()
    {
        _container.Resolve<MyController>();
    }
}

Service Location

While Service Location is an effective mechanism for introducing inversion of control, I'm listing it last for a reason. Unlike Constructor Injection, Service Location couples all your code to an intermediate provider. Depending on your application and relative complexity this might be acceptable, but from personal experience it's a very slippery slope -- actually, it would be more appropriate to call it a cliff because removing or replacing the container from an existing codebase can be a massive undertaking.

I see service location as a useful tool when refactoring legacy code towards Constructor Injection. The lower "leaf-nodes" of the object graph can take advantage of constructor injection and the higher nodes can temporarily use service location to create the "leaves". As you refactor, the service locator is removed and replaced with constructor injection, which results in an easier to test and discoverable API. This also provides a pragmatic bottom up refactor approach.

Summary

  • Use "new" when dependencies have no external dependencies or don't influence conditional flow of the subject.  Conversely, use constructor injection when dependencies have additional dependencies.
  • Use an IoC container to construct objects that use constructor injection, but only use the IoC container in high level components that are responsible for construction of the object graph. If constructing the entire object graph is a performance concern, consider encapsulating the container in a Factory that can be used to resolve remaining parts of the object graph when needed.
  • To prevent violating encapsulation concerns, use inversion of control to promote using abstractions instead of concrete dependencies.

submit to reddit

Tuesday, May 31, 2011

The Tests are Broken, Now What?

Despite our best intentions to write durable tests, it seems inevitable that tests will break at some point. In some cases, breaking a test can be seen as a very positive thing: we've introduced a side-effect that has broken existing functionality, and our tests have helped prevent a bug. However, in some cases, tests break because of external factors -- maybe the tests weren't isolated enough, maybe a developer went rogue and forgot to run the tests, or some other poorly justified or lousy excuse. While identifying what went wrong in your development process is important, the immediate concern is that these broken tests represent a failed build and developer productivity is at risk -- what should you do with the broken tests?

If the developer looking at the tests knows a bit about the code, they should have enough information in the tests to help sort things out.  The developer should drop what they're doing, slow down and research the code and the tests and fix the broken tests. If caught early, it's very manageable. Unfortunately, this isn't the common case -- the developer introducing the breaking changes doesn't understand the tests and is ultimately left with one of two approaches:

  1. Comment out or remove the test
  2. Mark the test as Ignored

Neither of these approaches "fix" anything.  The first approach, to comment the test out, is equivalent to pissing on the requirements. The test supposedly represents a piece of functional behaviour of the application, the fact that it's broken suggests a feature is also broken. The only reason to comment out a test is if that functionality has ceased to exist or the test was invalid in the first place. (Take a look at my checklist for unit test code reviews to help qualify invalid tests)

The second alternative to mark the tests as Ignored can be dangerous, depending on whether you're able to track why the test has been disabled and able to monitor when it enters this state.  This is one part developer process, one part testing framework, one part reporting.

Within NUnit, the Ignore attribute fits the bill nicely.  The attribute accepts a string message which is used to report why the test has been disabled. It should be standard developer process to only allow use of Ignore if a message is provided. (Come to think of it, I should put in a feature request so that it is required.)  When NUnit runs, the Ignored tests are not executed but the number of ignored tests are included in the xml output, meaning that your build server can track ignored tests over time. You can also bypass the Ignored attribute within the NUnit user-interface by simply selecting the Ignored test and running it manually.

[Ignore("The xml for this test isn't serializing correctly. Please fix!")]
[Test]
public void SuperImportantTest()
{
    // snip
}

Within MSTest however, Ignore is essentially the same as commenting out the tests.  There's no ability to record why the test has been excluded (developers must leave comments), and when MSTest runs the test suite the Ignored tests are simply excluded from the test run.  Looking at the MSTest output, the TRX file does not define a status for ignored, meaning that it can't be tracked in your build reports.

<ResultSummary outcome="Completed">
  <Counters 
    total="1" 
    executed="1" 
    passed="1" 
    error="0" 
    failed="0" 
    timeout="0" 
    aborted="0" 
    inconclusive="0" 
    passedButRunAborted="0" 
    notRunnable="0" 
    notExecuted="0" 
    disconnected="0" 
    warning="0" 
    completed="0" 
    inProgress="0" 
    pending="0" />
</ResultSummary>

Though I'm not a big fan of this, I'll spare you the complaining and move on to working solutions.

The best approach within MSTest isn't to disable the test but to indicate that it isn't reliable and needs attention. In addition, we need these tests to stand out without breaking the build. This is exactly what Inconclusive is for.

[TestMethod] 
public void SuperImportantTest() 
{ 
    Assert.Inconclusive("The xml for this test isn't serializing correctly. Please fix!"); 
    
    // snip 
}

The key to using Assert.Inconclusive is to put the statement as the first code-expression in your test. When MSTest encounters the Assert.Inconclusive statement, execution of the test is immediately halted, and the test is recorded as "Inconclusive". The TRX reports this status separately, and the message appears in the xml output.

Next Steps

With the tests marked as problematic, it’s possible to check-in the tests and unblock developers from a failed build while you and the original test author figure out how to fix the problem. It’s a temporary patch and is not intended for all breaking tests, just small fires that happen during critical developer crunches.  Once the fire has been put out and the tests corrected, you really should go back and investigate why the test broke in the first place:

  • Should the developer’s changes have broken this test?  If not, could the code/test have been written differently to be more durable?
  • What could the developer have done differently? Is this a one time occurrence or a disconnect in process?

That’s all for now.

submit to reddit

Monday, April 18, 2011

Testing Asynchronous Logic

I've been doing a fair bit of asynchronous logic recently where I'm designing objects to raise events after long-running operations have completed, and naturally this creates a few challenges during testing. When a method uses the asynchronous pattern, the work is executed on a separate thread and execution returns immediately to the caller. This is a problem for the a unit test as they are by design a series of standalone sequential steps -- the test may finish or execute assertions before the event is called.

The immediate workaround is to add delays into the tests using a Thread.Sleep statement.

[Test]
public void AsynchronousTestsDontAlwaysWork()
{
    bool eventCalled = false;
    
    var subject = new MyClass();
    subject.OnComplete += (o,e) => eventCalled = true;
    
    subject.DoWorkAsync();
    
    Thread.Sleep(100);
    
    Assert.AreTrue( eventCalled ); // can fail because the event might raise after this assertion
}

This strategy will work, but it's dangerous in the long-term because the duration of the asynchronous operation is dependent on the complexity of the implementation and the environment the test is executing in.  In other words, you may need to increase sleep interval if the implementation needs to do more work or if the tests fail on slower machines. In the end, the sleep interval reflects the lowest common denominator in your environment (that slow laptop that intern uses) and the entire test run becomes painfully (and unnecessarily) slow.

The solution is to add a little extra plumbing to block execution until the event is fired.

[Test]
public void AsynchronousTestWithBlockingAlwaysWorks()
{
    var reset = new System.Threading.ManualResetEvent(false);
    
    var subject = new MyClass();
    subject.OnComplete += (o,e) => reset.Set();
    
    subject.DoWorkAsync();
    
    if (!reset.WaitOne(1000)) // block for at most 1 second
    {
        Assert.Fail("The test timed out waiting for the OnComplete event.");
    }
}

The above example uses the ManualResetEvent, which in short is a signalling mechanism. The WaitOne method blocks execution until another thread calls the Set method or the timeout value is exceeded. In the above example, I've set up an anonymous delegate for my event so that the test finishes as soon as the asynchronous logic completes.

Cleaning it up...

As I mentioned earlier, I've been doing a lot of this type of testing so the above pattern has appeared in more than one test. It would be nice to encapsulate this a bit and streamline it into a reusable utility. This post on StackOverflow had an interesting concept to wrap the blocking logic into an object that supports IDisposable. The original poster was attempting to use Expressions to access events, which sadly isn't possible from outside the class, but here's an updated example that uses reflection and a named parameter for the event:

public sealed class EventWatcher : IDisposable
{
    private readonly object _target;
    private readonly EventInfo _eventInfo;
    private readonly Delegate _listener;
    private readonly ManualResetEvent _reset;
    private readonly int _timeout;

    public static EventWatcher Create<T>(T target, string eventName, int timeout = 1000)
    {
        EventInfo eventInfo = typeof (T).GetEvent(eventName);
        if (eventInfo == null)
            throw new ArgumentException("Event not found.", "eventName");

        return new EventWatcher(target, eventInfo, timeout);
    }

    private EventWatcher(object target, EventInfo eventInfo, int timeout)
    {
        _target = target;
        _eventInfo = eventInfo;
        _timeout = timeout;

        _reset = new ManualResetEvent(false);

        // Create our event listener
        _listener = CreateEventListenerDelegate(eventInfo.EventHandlerType);

        _eventInfo.AddEventHandler(target, _listener);
    }

    public void SetEventWasRaised()
    {
        _reset.Set();
    }

    private Delegate CreateEventListenerDelegate(Type eventType)
    {
        // Create the event listener's body, setting the 'eventWasRaised_' field.
        var setMethod = typeof(EventWatcher).GetMethod("SetEventWasRaised");
        var body = Expression.Call(Expression.Constant(this), setMethod);

        // Get the event delegate's parameters from its 'Invoke' method.
        MethodInfo invokeMethod = eventType.GetMethod("Invoke");
        var parameters = invokeMethod.GetParameters()
            .Select((p) => Expression.Parameter(p.ParameterType, p.Name));

        // Create the listener.
        var listener = Expression.Lambda(eventType, body, parameters);
        return listener.Compile();
    }

    public void Dispose()
    {
        bool eventWasFired = _reset.WaitOne(_timeout);
        // Remove the event listener.
        _eventInfo.RemoveEventHandler(_target,_listener);

        if (!eventWasFired)
        {
            string message = String.Format("Test timed-out waiting for event \"{0}\" to be raised.", _eventInfo.Name);
            Assert.Fail(message);
        }
    }
}

[Test]
public void DemonstrateEventWatcherWithUsingBlockAndDisposePattern()
{
    var subject = new MyClass();
    
    using(EventWatcher.Create(subject, "OnComplete"))
    {
        subject.DoWorkAsync();
        
    } // throws Assert.Fail() if event wasn't called.
}

Some Refinements...

This solution works, but there are a few drawbacks:

  1. The using block is a very succinct mechanism for controlling scope, but unfortunately if an exception occurs in the body of the using statement it will get swallowed by the exception in our EventWatcher's Dispose method.
  2. This only validates that the event was called, and it doesn't give us access to the event argument, which might be of interest to the test.
  3. The event name is a literal string value which isn't very refactor friendly.

Preventing Hidden Exceptions from the Finally block

To address the first problem where exceptions aren't properly handled inside the using statement, we change the signature slightly to perform the test logic in an Action.

// snip...
public static void WaitForEvent<T>(T target, string eventName, Action action) 
{ 
    var watcher = Create(target, eventName);

    try 
    { 
        action(); 
    } 
    catch (Exception) 
    { 
        watcher.Clear(); 
        throw; 
    } 
    finally 
    { 
        watcher.Dispose(); 
    } 
}
// ...end snip

[Test] 
public void DemonstrateEventWatcherWithAnActionInsteadOfUsingBlock() 
{ 
    EventWatcher.WaitForEvent(subject, "OnComplete", () => subject.DoWorkAsync()); 
}

Gaining Access to the EventArgs

Addressing the second problem requires some changes to the CreateEventListenerDelegate so that the arguments of the event are recorded.

//snip
public void SetEventWasRaised(EventArgs e) 
{ 
    EventArg = e; 
    _reset.Set(); 
}

public EventArgs EventArg { get; private set; }

private Delegate CreateEventListenerDelegate(Type eventType) 
{ 
    // Get the event delegate's parameters from its 'Invoke' method. 
    MethodInfo invokeMethod = eventType.GetMethod("Invoke"); 
    var parameters = invokeMethod.GetParameters() 
        .Select((p) => Expression.Parameter(p.ParameterType, p.Name)).ToList();

    // Setup the callback to pass in the EventArgs 
    var setMethod = typeof(EventWatcher).GetMethod("SetEventWasRaised"); 
    var body = Expression.Call(Expression.Constant(this), setMethod, parameters[1]);

    // Create the listener. 
    var listener = Expression.Lambda(eventType, body, parameters); 
    return listener.Compile(); 
} // end snip

[Test] 
public void DemonstrateEventWatcherCanCaptureEventArgs() 
{ 
    EventArgs e; 
    EventWatcher.WaitForEvent(subject, "OnComplete", out e, t => t.DoWorkAsync()); 
    
    Assert.IsNotNull(e); 
} 

Making it Refactor-Friendly...

Tackling the last problem where we don't have type-safety for our event, we need a special workaround because we're unable to use an Expression to capture our event. From a language perspective, C# will only allow external callers to add or remove event handler assignments, so we can't write a lamda expression that points to an event reference. Hopefully this is something that will be added to the language in a future version, but until then accessing the event requires some special hackery.

Taking a play from the Moq source code, we can infer the event name by using a dynamic proxy and intercepting the method invocation when we perform an event assignment. It's a long and awkward way around, but it works, albeit it comes with all the caveats of using a dynamic proxy: non-sealed classes with virtual events, MarshalByRefObject, or interfaces.

A note about virtual events: I’m personally not a big fan of virtual events (Resharper warnings, etc), but if you’re comfortable putting the event declarations on an interface, you can explicitly specify the interface in the generic type argument.  EventWatcher.Create<IMyInterface>(…)

public class EventAssignmentInterceptor : Castle.DynamicProxy.IInterceptor
{
    private Castle.DynamicProxy.IInvocation _lastInvocation;

    #region IInterceptor implementation

    public void Intercept(IInvocation invocation)
    {
        _lastInvocation = invocation;
    }

    #endregion

    public string GetEventName()
    {
        string methodName = _lastInvocation.Method.Name;
        return methodName.Replace("add_", "").Replace("remove_", "");
    }
}

public class EventHelper
{
    public static string GetEventName<T>(T target, Action<T> action) where T : class
    {
        var generator = new Castle.DynamicProxy.ProxyGenerator();
        var interceptor = new EventAssignmentInterceptor();

        T proxy;

        if (typeof(T).IsInterface)
        {
            proxy = generator.CreateInterfaceProxyWithoutTarget<T>(interceptor);
        }
        else
        {
            proxy = generator.CreateClassProxy<T>(interceptor);
        }

        action(proxy);

        return interceptor.GetEventName();
    }
}

Examples

Here’s a few examples that illustrate the various syntax.  Note, for sake a brevity (this post is crazy large) the overloads that define events as literals are not shown.

Basic usage in a using block. Note that exceptions inside the using may be swallowed by the dispose, and there's no access to the event args.

using(EventWatcher.Create(subject, t => t.OnComplete += null)) 
{ 
    subject.PerformWork(); 
} 

using(EventWatcher.Create(subject, t => t.OnComplete += null, timeout:1000) 
{ 
    subject.PerformWork(); 
}

Condensed usage. Provides access to the EventArgs and avoids the dispose problem.

EventArgs e = EventWatcher.Create(subject, t=> t.OnComplete +=null, () => subject.PerformWork() );

EventArgs e; 
EventWatcher.Create(subject, t=> t.OnComplete += null, out e, ()=> subject.PerformWork() ); 

Fluent usage. A cleaner syntax that declares the event and the action separately.

EventArgs e = EventWatcher 
            .For(subject, t => t.OnComplete += null) 
            .WaitOne(() => subject.PerformWork()) 

EventArgs e; 
EventWatcher.For(subject, t=> t.OnComplete += null) 
            .WaitOne( () => subject.PerformWork(), out e);

Full Source

The full source is here.

public sealed class EventWatcher : IDisposable
{
    private readonly object _target;
    private readonly EventInfo _eventInfo;
    private readonly Delegate _listener;
    private readonly ManualResetEvent _reset;
    private readonly int _timeout;

    #region Create
    public static EventWatcher Create<T>(T target, string eventName, int timeout = 1000)
    {
        EventInfo eventInfo = typeof(T).GetEvent(eventName);
        if (eventInfo == null)
            throw new ArgumentException("Event not found.", "eventName");

        return new EventWatcher(target, eventInfo, timeout);
    }

    public static EventWatcher Create<T>(T subject, Action<T> eventAssignment, int timeout = 1000) where T : class
    {
        string eventName = EventHelper.GetEventName(subject, eventAssignment);

        return Create(subject, eventName, timeout);
    } 
    #endregion

    #region For
    public static EventWatcherSetup<T> For<T>(T subject, Action<T> eventAssignment) where T : class
    {
        string eventName = EventHelper.GetEventName(subject, eventAssignment);
        return For(subject, eventName);
    }

    public static EventWatcherSetup<T> For<T>(T subject, string eventName)
    {
        return new EventWatcherSetup<T>(subject, eventName);
    } 
    #endregion

    #region WaitForEvent
    public static EventArgs WaitForEvent<T>(T target, Action<T> eventAssignment, Action action) where T : class
    {
        EventArgs e;
        WaitForEvent(target, eventAssignment, out e, action);
        return e;
    }

    public static void WaitForEvent<T>(T target, Action<T> eventAssignment, out EventArgs e, Action action) where T : class
    {
        string eventName = EventHelper.GetEventName(target, eventAssignment);
        WaitForEvent(target, eventName, out e, action);
    }

    public static EventArgs WaitForEvent<T>(T target, string eventName, Action action)
    {
        EventArgs e;
        WaitForEvent(target, eventName, out e, action);
        return e;
    }

    public static void WaitForEvent<T>(T target, string eventName, out EventArgs e, Action action)
    {
        var watcher = Create(target, eventName);

        try
        {
            action();
        }
        catch (Exception)
        {
            watcher.Clear();
            throw;
        }
        finally
        {
            e = watcher.EventArg;
            watcher.Dispose();
        }
    } 
    #endregion

    private EventWatcher(object target, EventInfo eventInfo, int timeout)
    {
        _target = target;
        _eventInfo = eventInfo;
        _timeout = timeout;

        _reset = new ManualResetEvent(false);

        // Create our event listener
        _listener = CreateEventListenerDelegate(eventInfo.EventHandlerType);

        _eventInfo.AddEventHandler(target, _listener);
    }

    public void SetEventWasRaised(EventArgs e)
    {
        EventArg = e;
        _reset.Set();
    }

    public void Clear()
    {
        _reset.Set();
    }

    public EventArgs EventArg { get; private set; }

    private Delegate CreateEventListenerDelegate(Type eventType)
    {
        // Get the event delegate's parameters from its 'Invoke' method.
        MethodInfo invokeMethod = eventType.GetMethod("Invoke");
        var parameters = invokeMethod.GetParameters()
            .Select((p) => Expression.Parameter(p.ParameterType, p.Name)).ToList();

        var setMethod = typeof(EventWatcher).GetMethod("SetEventWasRaised");
        var body = Expression.Call(Expression.Constant(this), setMethod, parameters[1]);

        // Create the listener.
        var listener = Expression.Lambda(eventType, body, parameters);
        return listener.Compile();
    }

    public void Dispose()
    {
        bool eventWasFired = _reset.WaitOne(_timeout);
        // Remove the event listener.
        _eventInfo.RemoveEventHandler(_target,_listener);

        if (!eventWasFired)
        {
            string message = String.Format("Test timed-out waiting for event \"{0}\" to be raised.", _eventInfo.Name);
            Assert.Fail(message);
        }
    }
}

public class EventWatcherSetup<T>
{
    private readonly T _target;
    private readonly string _eventName;

    public EventWatcherSetup(T target, string eventName)
    {
        _target = target;
        _eventName = eventName;
    }

    public EventArgs WaitOne(Action action)
    {
        EventArgs e;

        WaitOne(action, out e);
        
        return e;
    }
    public void WaitOne(Action action, out EventArgs e)
    {
        EventWatcher.WaitForEvent(_target,_eventName, out e, action);
    }

}

public class EventAssignmentInterceptor : Castle.DynamicProxy.IInterceptor
{
    public Castle.DynamicProxy.IInvocation LastInvocation;

    #region Implementation of IInterceptor

    public void Intercept(IInvocation invocation)
    {
        LastInvocation = invocation;
    }

    #endregion

    public string GetEventName()
    {
        string methodName = LastInvocation.Method.Name;
        return methodName.Replace("add_", "").Replace("remove_", "");
    }
}

public class EventHelper
{
    public static string GetEventName<T>(T target, Action<T> action) where T : class
    {
        var generator = new Castle.DynamicProxy.ProxyGenerator();
        var interceptor = new EventAssignmentInterceptor();

        T proxy;

        if (typeof(T).IsInterface)
        {
            proxy = generator.CreateInterfaceProxyWithoutTarget<T>(interceptor);
        }
        else
        {
            proxy = generator.CreateClassProxy<T>(interceptor);
        }

        action(proxy);

        return interceptor.GetEventName();
    }
}

submit to reddit

Thursday, March 03, 2011

Building Custom Test Frameworks

Author’s note: this post focuses on writing test tools to simplify repetitive tasks such as complex assertions and is inspired by colleagues that wrote a tool that nearly evolved beyond their control. I am not encouraging developers to write frameworks that mask symptoms of poorly isolated components or other design problems.

For some, evolving code through writing tests is seen as a labour of love. For others, it's just labour. The latter is especially true when you realize that you have to write a lot of repetitious tests that will likely introduce test friction later on. When faced with this dilemma, developers rise to the challenge and write code.

Fortunately, there's a lot of great test framework tools popping up in the .net community that are designed to plug in to your test code. (I’m running out of battery power on my laptop as I write this, so my list of tools is lacking.  Send me a note and I’ll list your tool here) These tools can certainly make things easier by removing some obstacles or laborious activities, but if you're planning on writing your own tool or framework there a few pitfalls.

Common Pitfalls

Obscured Intent

One of the goals of well written unit tests is to provide live examples of proper usage so that developers can learn more about the code's intended behaviour. This benefit can be significantly hindered when the usage and clear intent of your code has been abstracted into your framework.

Eventual Complexity

Applying automation to a problem follows the 80/20 rule where the majority of problems fit nicely into your abstraction. The edge cases however have a tendency to add bloat and it doesn't take much to quickly trash the idea of a simple tool. This is rarely a consequence of poor planning or bad design; additional complexity tends to creep in over time as your code evolves or as more consumers of the tool come on board.

There's a consequence to this type of complexity: if few developers understand the tool's implementation, you risk limiting these developers to be tool maintainers. Even worse, if these developers leave your team there's a risk that the entire suite of tests will be abandoned if they start failing.

Dependence on Tool Quality / False Positives

In TDD, application defects hide in the tests you haven't written, so quality is a reflection of the accuracy and completeness of the tests. Likewise, tests that leverage a custom test framework are only as reliable as the completeness and accuracy of the tool. If the framework takes on the lions share of the work, then the outcome of the test is abdicated to the tool. This is dangerous because any change to the tool's logic could unintentionally allow subtle defects in the tested code to go unnoticed. False positives = tests that lie!

Recommendations

Tests for Tools

Oddly enough, if your goal is to write a tool so that you don't need to write tests, you are going to need to write tests for the tool. Having tests for your custom tool ensures that false positives aren’t introduced as the tool is enhanced over time. From a knowledge transfer perspective, the tests serve as a baseline to describe the tool’s responsibilities and enable others to add new functionality (and new tests) with minimal oversight from the original tool author.

Design for Readability

Great frameworks fit in and enable you to express intent; so be careful of over-reaching and trying to do too much. Make sure your tool doesn't abstract away clues as to what the test is intended for, and if possible use descriptive method names to improve readability. Documenting your tool with Xml documentation syntax and comments is also very helpful for intent.

Evolve Naturally

Unless you spend weeks planning out your unit tests, custom test tools are the product of necessity that are realized when you start to write the second or third duplicated test.  I tend to realize my test tools as “found treasures” of my normal TDD development cycle.

Rather than diving in and writing a framework or tool first, focus on satisfying the requirement of working software by writing the test using normal TDD best practices (red, green, refactor). During the clean up and refactor step, you will find duplication between tests or even different fixtures. When that happens, promote the duplicated code to a helper method, then a helper class. I like to think that great TDD is about refactoring mercilessly while keeping the original intent of the code clear – it’s about balance, know when you’ve gone too far.

If you a reach a point where you can extract commonalities between a few helper classes into a generic solution, you’ll find yourself standing in the doorway between where your fixtures were self-contained and where they’ve become dependent on shared test logic.  Learn to recognize this moment because this is when you should stop writing tests for production code for a moment and write some tests for the tool. It’s also a good idea to keep a few of the manual tests around so that you can go back into the production code and deliberately break it to prove that both sets of tests are equivalent.

…Until next time. Happy coding.  (Now where’s my dang power supply?)

Monday, December 13, 2010

Unit Test Review Checklist

Whenever someone mentions that they want to bring a code-review process into a project, my first inclination is to have teams review the tests.  Tests should be considered production assets that describe the purpose and responsibilities of the code, and developers should be making sure that their tests satisfy this goal.  If the tests look good and they cover the functionality that is required, I don’t see much point in dwelling on the implementation: if the implementation sucks, you can refactor freely assuming you have proper coverage from your tests.

A unit test review process is a great way to ensure that developers are writing useful tests that will help deliver high quality code that is flexible to change.  As code-review processes have the tendency to create stylistic debates within a development group, having a checklist of the 5-10 points to look for is a great way to keep things moving in the right direction.

Here’s my criteria that I’d like to see in a unit test.  Hopefully you find this useful, too.

Pass Guideline
 

Is the test well named?

  • Does the test name clearly represent the functionality being tested?
  • Will someone with little or no knowledge of the component be able to decipher why the test exists?
 

Is the test independent?

  • Does the test represent a single unit of work?
  • Can the test be executed on its own or is it dependent on the outcome of tests that preceded it?  For example, a fixture that shares state between tests may inadvertently include side-effects.
 

Is the test reliable?

  • Does the test produce the same outcome consistently?
  • Does the test run without manual intervention to determine a successful outcome?
 

Is the test durable?

  • Is the test designed so that it isn’t susceptible to changes in other parts of the application?  For example:
    • Does the test have complex or lengthy setup?
    • Will changes in the subject's dependencies lead to breakages?
 

Are the assertions valid?

  • Does the test contain assertions that describe the functionality being tested?
  • Do the assertions include helpful error messaging that describe what should have happened if the test fails?
  • Are any assertions redundant such as testing features of the CLR (constructors or get/set properties)?
  • Are the assertions specific to this test or are they duplicated in other tests?
 

Is the test well structured and documented?

  • Does the test contain sufficient comments?
  • Does the test highlight the subject under test and corresponding functionality being exercised?
  • Is there a clear indication of arrange/act/assert pattern such that the importance of the arrange makes sense, the action easily identifiable and the assertions clear?
 

Is the test isolating responsibilities of the subject?

  • Does the test make any assumptions about the internals of dependencies that aren't immediately related to the subject?
  • Is the test inaccurately or indirectly testing a responsibility or implementation of another object?  If so, move the test to the appropriate fixture.
 

Are all scenarios covered?

  • Does the fixture test all paths of execution? Not just for the code that has been written, but also for scenarios that may have been missed:
    • invalid input?;
    • failures in dependencies? 
 

Does the test handle resources correctly?

  • If the test utilizes external dependencies (databases, static resources), is the state of these dependencies returned to a neutral state?
  • Are resources allocated efficiently to ensure that the test runs as fast as possible:
    • setup/teardown effectively;
    • No usages of Thread.Sleep()?
 

Is the complexity of the test balanced?

Tests need to strike a balance between Don't Repeat Yourself and clear examples of how to use the subject.

  • Is the test too complicated or difficult to follow?
  • Is there a lot of duplicated code that would lead to time consuming effort if the subject changed? 

 

What do you think?  Are there criteria that you’d add?  Things you disagree with?

Happy coding.

Monday, October 18, 2010

Callbacks with Moq

My current project is using Moq as our test mocking framework. Although I've been a fan of RhinoMocks for several years I've found that in general, they both support the same features but I'm starting to think I like Moq's syntax better. I'm not going to go into an in-depth comparison of the mocking frameworks, as that's well documented elsewhere. Instead, I want to zero in on a feature that I've had my eye on for sometime now.

Both RhinoMocks and Moq have support for a peculiar feature that let's you invoke a callback method in your test code when a method on your mock is called. This feature seems odd to me because in the majority of cases a mock is either going to return a value or throw an exception - surely invoking some arbitrary method in your test must represent a very small niche requirement.

According to the RhinoMock documentation, the callback takes a delegate that returns true or false. From this it's clear that you would use this delegate to inspect the method's incoming parameters and return false if it didn't meet your expectations. However, as Ayende eludes, this is a powerful feature that can be easily be abused.

Moq provides this feature too, but the syntax is different. Rather than requiring a delegate Func<bool>, Moq's Callback mimics the signature of the mocked method. From this syntax it's not obvious that the callback should be used for validating inbound parameters, which suggests that it could be abused, but it also implies freedom for the test author to do other things. Granted this can get out of control and can be abusive, but perhaps a level of discretion about it's usage is also implied?

Here are a few examples where Callbacks have been helpful:

Validating inbound method parameters

The best example I can imagine for inspecting an inbound parameter is for a method that has no return value. For example, sending an object to a logger. If we were to assume that validation logic of the inbound parameter is the responsibility of the logger, we would only identify invalid arguments at runtime. Using the callback technique, we can write a test to enforce that the object being logged meets minimum validation criteria.

[TestMethod]
public void Ensure_Exception_Is_Logged_Properly()
{
  Exception exception;

  Mock.Get(Logger)
      .Setup( l => l.Error(It.IsAny<Exception>())
      .Callback<Exception>( ex => exception = ex )

  Subject.DoSomethingThatLogsException();

  Assert.AreEqual(ex.Message, "Fatal error doing something");
}

Changing state of inbound parameters

Imagine a WPF that uses the MVVM pattern and we need to launch a view model as a modal dialog. The user can make changes to the view model in the dialog and click ok, or they can click cancel. If the user clicks ok, the view model state needs to reflect their changes. However if they click cancel, any changes made need to be discarded.

Here's the code:

public class MyViewModel : ViewModel
{
  /* snip */

  public virtual bool Show()
  {
      var clone = this.Clone();
      var popup = new PopupWindow();
      popup.DataContext = clone;

      if (_popupService.ShowModal(popup))
      {
          CopyStateFrom(clone);
          return true;
      }
      return false;
  }

}

Assuming that the popup service is a mock object that returns true when the Ok button is clicked, how do I test that the contents of the popup dialog are copied back into the subject? How do I guarantee that changes aren't applied if the user clicks cancel?

The challenges with the above code is that the clone is a copy of my subject. I have no interception means to mock this object unless I introduce a mock ObjectCloner into the subject (that is ridiculous btw). In addition to this, the changes to the view model happen while the dialog is shown.

While the test looks unnatural, Callbacks fit this scenario really well.

[TestMethod]
public void When_User_Clicks_Ok_Ensure_Changes_Are_Applied()
{
  Mock.Get(PopupService)
      .Setup(p => p.ShowModal(It.IsAny<PopupWindow>())
      .Callback<PopupWindow>( ChangeViewModel )
      .Returns(true);

  var vm = new MyViewModel(PopupService)
              {
                  MyProperty = "Unchanged"
              };

  vm.Show();

  Assert.AreEqual("Changed", vm.MyProperty);
}

private void ChangeViewModel(PopupWindow window)
{
  var viewModel = window.DataContext as MyViewModel;
  viewModel.MyProperty = "Changed";
}

The key distinction here is that changes that occur to the popup are in no way related to the implementation of the popup service. The changes in state are a side-effect of the object passing through the mock. We could have rolled our own mock to simulate this behavior, but Callbacks make this unnecessary.

Conclusion

All in all, Callbacks are an interesting feature that allow us to write sophisticated functionality for our mock implementations. They provide a convenient interception point for parameters that would normally be difficult to get under the test microscope.

How are you using callbacks? What scenarios have you found where callbacks were necessary?

submit to reddit

Tuesday, October 12, 2010

Working with Existing Tests

You know, it’s easy to forget the basics after you’ve been doing something for a while.  Such is the case with TDD – I don’t have to remind myself of the fundamental “red, green, refactor” mantra everything I write a new test, it’s just baked in.  When it’s time to write something new, the good habits kick in and I write a test.  After all, this is what the Driven part of Test Driven Development is about: we drive our development through the creation of tests.

The funny thing is, the goal of TDD isn’t to produce tests.  Tests are merely a by-product of the development of the code, and having tests that demonstrate that the code works is one of the benefits.  Once they’re written, we forget about them and move on – we only return to them if something unexpected broke.

Wait.  Why are they breaking?  Maybe we forgot something, somewhere.

The Safety Net Myth

One of the reasons that tests break is because there’s a common perception that once the code is written, we no longer need the tests to drive development.  “We’ve got tests, so let’s just see what breaks after I make these changes…”

This strategy works when you want to try “what-if” scenarios or simple proper refactorings, but it falls flat for long-term coding sessions.  The value of the tests diminish quickly the longer the coding session lasts.  Simply put, tests are not safety nets – if you go off making changes for a few days you’re only going to find that the tests get in the way as they don’t represent your changes and your code won’t compile.

This may seem rudimentary, but let’s go back and review the absolute basics of TDD methodology:

  1. Start by writing a failing test. (RED)
  2. Implement the code necessary to make that test pass. (GREEN)
  3. Remove any duplication and clean it up.  (REFACTOR)

It’s easy to forget the basics.  The very first step is to make sure we have a test that doesn’t pass before we do any work, and this is easily overlooked when we already have tests for that functionality.

Writing tests for new functionality

If you want to introduce new functionality to your code base, challenge your team to introduce those changes to the tests first.  This may seem altruistic to some, especially if it’s been a long time since the tests were written or if no-one on the team is familiar with the tests or their value. 

Here’s a ridiculously simple tip:

  1. Locate the code you think may need to change for this feature.
  2. Introduce a fatal error into the code.  Maybe comment out the return value and return null, or throw an exception.
  3. Run the tests.

With luck, all the areas of your tests that are impacted by this code are broken.  Review these tests and ask yourself:

  • Does this test represent a valid requirement after I introduce my change?  If not, it’s safe to remove it.
  • How does this test relate to the change that I’m introducing?  Would my change alter the expected results of this test?  If yes, change the expected results. These tests should fail after you remove the fatal flaw you introduced moments ago.
  • Do any of these tests represent the new functionality I want to introduce?  If not, write that test now.

(If nothing breaks, you’ve got a different problem.  Do some research on what it would take to get this code under a test, and write tests for new functionality.)

Conclusion

The duct tape programmer will argue that you can’t make an omelette without breaking some eggs, which is true – we should have the courage to stand up and fix things that are wrong.  But I’d argue that you must do your homework first - if you don’t check for other ingredients, you’re just making scrambled eggs. 

In my experience, long term refactorings that don’t leverage the tests are a recipe for test-abandonment; your tests and code should always be moments from being able to compile.  The best way to keep the tests valid is to remember the basics – they should be broken before you start introducing changes.

submit to reddit

Tuesday, October 05, 2010

Writing Easier to Understand Tests

For certain, the long term success of any project that leverages Tests has got to be Tests that are easy to understand and provide value.

For me, readability is the gateway to value. I want to be able to open the test, and BOOM! Here’s the subject, here’s the dependencies, this is what I’m doing, and here’s what I expect it to do. If I can’t figure it out within a few seconds, I start to question the value of the tests. And that’s exactly what happened to me earlier this week.

The test I was looking at had some issues, but the developer who wrote it had their heart in the right place and made attempts to keep it relatively straight-forward. It was using a Context-Specification style of test and was using Moq to Mock out the physical dependencies, but I got tied up in the mechanics of the test. I found that the trouble I was having was determining which Mocks were part of the test versus Mocks that were in the test to support related dependencies.

Below is an example of a similar test and the steps I took to clean it up. Along the way I found something interesting, and I hope you do, too.

Original (Cruft Flavor)

Here’s a sample of the test. For clarity sake, I only want to illustrate the initial setup of the test, so I’ve omitted the actual test part. Note, I’m using a flavor of context specification that I’ve blogged about before, if it seems like a strange syntax, you may want to read up.

using Moq;
using Model.Services;
using MyContextSpecFramework;
using Microsoft.VisualStudio.TestTools.UnitTesting;

public class TemplateResolverSpecs : ContextSpecFor<TemplateResolver>
{
   protected Mock<IDataProvider> _mockDataProvider;
   protected Mock<IUserContext> _mockUserContext;
   protected IDataProvider _dataProvider;
   protected IUserContext _userContext;

   public override void Context()
   {
       _mockDataProvider = new Mock<IDataProvider>();
       _mockUserContext = new Mock<IUserContext>();

       _userContext = _mockUserContext.Object;
       _dataProvider = _mockDataProvider.Object;
   }

   public override TemplateResolver InitializeSubject()
   {
       return new TemplateResolver(_dataProvider,_userContext);
   }

   // public class SubContext : TemplateResolverSpecs
   // etc
}

This is a fairly simple example and certainly those familiar with Moq’s syntax and general dependency injection patterns won’t have too much difficultly understanding what’s going on here. But you have to admit that while this is a trivial example there’s a lot of code here for what’s needed – and you had to read all of it.

The Rewrite

When I started to re-write this test, my motivation was for sub-classing the test fixture to create different contexts -- maybe I would want to create a context where I used Mocks, and another for real dependencies. I started to debate whether it would be wise to put the Mocks in a subclass or in the base when it occurred to me why the test was confusing in the first place: the Mocks are an implementation detail that are getting in the way of understanding the dependencies to the subject. The Mocks aren’t important at all – it’s the dependencies that matter!

So, here’s the same setup with the Mocks moved out of the way, only referenced in the initialization of the test’s Context.

using Moq;
using Model.Services;
using MyContextSpecFramework;
using Microsoft.VisualStudio.TestTools.UnitTesting;

public class TemplateResolverSpecs : ContextSpecFor<TemplateResolver>
{
   protected IDataProvider DataProvider;
   protected IUserContext UserContext;

   public override void Context()
   {
       DataProvider = new Mock<IDataProvider>().Object;
       UserContext = new Mock<IUserContext>().Object;
   }

   public override TemplateResolver InitializeSubject()
   {
       return new TemplateResolver(DataProvider, UserContext);
   }
}

Much better, don’t you think? Note, I’ve also removed the underscore and changed the case on my fields because they’re protected and that goes a long way to improve readability, too.

Where’d my Mock go?

So you’re probably thinking “that’s stupid great Bryan, but I was actually using those mocks” – and that’s a valid observation, but the best part is you don’t really need them anymore. Moq has a cool feature that let’s you obtain the Mock wrapper from the mocked-object anytime you want, so you only need the mock when it’s time to use it.

Simply use the static Get method on the Mock class to obtain a reference to your mock:

Mock.Get(DataProvider)
   .Setup( dataProvider => dataProvider.GetTemplate(It.IsAny<string>())
   .Returns( new TemplateRecord() );

For contrast sake, here’s what the original would have looked like:

_mockDataProvider.Setup( dataProvider => dataProvider.GetTemplate(It.IsAny<string>())
                .Returns( new TemplateRecord() );

They’re basically the same, but the difference is I don’t have to remember the name of the variable for the mock anymore. And as an added bonus, our Mock.Setup calls will all line up with the same indentation, regardless of the length of the dependencies’ variable name.

Conclusion

While the above is just an example of how tests can be trimmed down for added readability, my hope is that this readability influences developers to declare their dependencies up front rather than weighing themselves and their tests down with the mechanics of the tests. If you find yourself suddenly requiring a Mock in the middle of the test, or creating Mocks for items that aren’t immediate dependencies, it should serve as a red flag that you might not be testing the subject in isolation and you may want to step-back and re-think things a bit.

submit to reddit

Tuesday, January 12, 2010

The Three Step Developer Flow

A long time ago, a mentor of mine passed on some good advise for developers that has stuck well with me: “Make it work.  Make it right.  Make it fast.”  While this simple mantra is likely influenced by Donald Knuth’s famous and misquoted statement that “premature optimization is the root of all evil, it’s more about how a developer should approach development altogether.

Breaking it down…

What I’ve always loved about this simple advice is that if a developer takes the steps out of order, such as putting emphasis on design or performance, there’s a very strong possibility that the code will never work.

Make it work…

Developers should take the most pragmatic solution possible to get the solution to work.  In some cases this should be considered prototype code that should be thrown away before going into production.  Sadly, I’m sure that 80% of all production code is prototype code with unrealized design.

Make it right…

Now that you know you know how to get the code to work, take some time to get into a shape that you can live with.  Interesting enough, emphasis should not be placed on designing for performance at this point.  If you can’t get to this stage, it should be considered technical debt to be resolved later.

Make it fast….

At this point you should have working code that looks clean and elegant, but how does it stack up when it’s integrated with production components or put under load?  If you had spent any time in the last two steps optimizing the code to handle load of a thousand users but it’s only called once in the application – you may have wasted your time and optimized prematurely.  To truly know, code should be examined under a profiler to determine if the code meets the performance goals of the application.  This is all a part of embedding a “culture of Performance” into your organization.

Aligned with Test Driven Development

It’s interesting that this concept overlaps with Test Driven Development’s mantra “Red, Green, Refactor” quite well.  Rather than developing prototype code as a console app, I write tests to prove that it works.  When it’s time to clean up the code and make it right, I’m refactoring both the tests and the code in small increments – after each change, I can verify that it still works. 

Later, if we identify performance issues with the code, I can use the tests as production assets to help me understand what the code needs to do.  This provides guidance when ripping out chunks of poorly performing code.

By following either the “red / green / refactor” or “make it work / right / fast" mantras mean that I don’t incorporate best practices or obvious implementation when writing the code? Hardly. I’ll write what I think needs to be written, but it’s important not to get carried away.

Write tests.  Test often.

submit to reddit

Wednesday, December 02, 2009

NUnit for Visual Studio Addin

I recently stumbled upon this great addin for Visual Studio that uses the Visual Studio Test Adapter pattern to include NUnit tests within Visual Studio as MS Tests.  They appear in the Test List Editor and execute equivalent to MS Test, including those handy Run and Debug keyboard shortcuts I described in my last post.

Since they operate as MS Tests, the project requires some additional meta-data in the csproj file in order to have Visual Studio recognize this project as a Test library.  My last post has the details.

Curious to see how far the addin could act as a stand-in for NUnit, I fired up Visual Studio, a few beers, and the NUnit attribute documentation to put it through the works.  I’ve compiled my findings here in the table below.

In all fairness, there are a lot of attributes in NUnit, some of these you probably didn’t know existed.

NUnit Attribute Supported Comments
Category No Sadly, the addin does not register a new column definition for Category.  Though this feature is not tied to any functional behavior, it would be greatly welcomed to improve upon Visual Studio’s Test Lists.
Combinatorial Yes  
Culture / SetCulture No Tests that would normally be excluded by NUnit fail.
Datapoint / Theory No Test names do not match NUnit runtime.  All Datapoints produce result Not Runnable in the Test Results
Description No Value does not appear in the Test List Editor
Explicit No Explicit Tests are executed and appear as Enabled in the Test List Editor
ExpectedException Yes  
Ignore Partial Ignored tests are excluded from the Test List Editor, so they are ignored, but they do not appear as Enabled = False.
MaxTime / Timeout Partial Functions properly though supplied setting does not appear in the Timeout column in the Test List Editor
Platform No Tests are executed regardless of the specified platform.
Property - Custom properties do not appear in the output of the TRX file, which is where I’m assuming they would appear.  Not entirely sure if the schema would support custom properties however.
Random No Tests are generated, though the names contain only numbers.  Executing these tests produce the result Not Runnable in the Test Results.
Range No Tests are generated, though the names contain only numbers.  Executing these tests produce the result Not Runnable in the Test Results.
Repeat Yes  
RequiredAddin - Not tested.
RequiresMTA / RequiresSTA / RequiresThread Yes  
Sequential Yes  
Setup / Teardown Yes  
SetupFixture No Setup methods for a given namespace do not execute.
Suite - Not tested (requires command-line switch)
TestFixtureSetup / TestFixtureTeardown Yes  
Test Yes Of course!
TestCase Yes Tested different argument types (int, string), TestName, ExpectedException.
TestCaseSource Yes  

There’s quite a few No’s in this list, but the major players (Test, Setup/Teardown, TestFixtureSetup/Teardown) are functional.  I’m actually pleased that NUnit 2.5.2 features such as parameterized tests (TestCase, TestCaseSource) and Combinatorial / Sequential / Values are in place, as well as former addin features that were bundled into the framework (MaxTime / Repeat).

In respect to the malformed test names and non-runnable tests for the Theory / Range / Random attributes, hopefully this is a small issue that can be resolved.  The cosmetic issues with Ignore / Description / Category don’t pose any major concerns though they would be large wins in terms of full compatibility with MS Test user interface and features.

I’ve never used the SetupFixture nor the culture attributes, so I’m not losing much sleep over these.

However, for me, the main issue for me is that Explicit tests are always executed.  I’ve worked on many projects where a handful of tests either brought down the build server or couldn’t be run with other tests.  Rather than solve the problem, developers tagged the tests as Explicit – they work, but you better have a good reason to be running them.

Hats off to the NUnitForVS team.

submit to reddit

Tuesday, December 01, 2009

Manually creating a MS Test Project

Although I’ve always been a huge proponent of NUnit, I’m finding I’m using MS Test more frequently for the following reasons:

  • My organization is a large Microsoft partner, so there’s often some preference for Microsoft tools in our projects.
  • Support for open source tools is a concern for some organizations I work with.  Although Tests are not part of the production deliverables, some organizations are very risk adverse and reasonably do not want to tie themselves to products without support or guarantee of backward compatibility.
  • Severe Resharper withdrawal.  After spending several years with Resharper tools, I’ve spent the last year with a barebones Visual Studio 2008 installation.  Without the tight integration between Visual Studio and NUnit, attaching and debugging a process isn’t involuntary. If the JetBrains guys are listening, hook me up.

Reluctantly, I’ve started to use Visual Studio Test warts-n-all.  Out of the box, MS Test has two very handy keyboard shortcuts where you can either Run (CTRL+R, T) or Debug (CTRL+R, CTRL+T) the current test, fixture or solution, depending on where your mouse is currently focused. 

Oddly enough, I’ve found myself in a position where I’ve manually created a Test project by adding the appropriate references, but none of the Visual Studio Test features work, including these handy short cuts.  Any attempt to run these tests using these shortcut produces an error:

No tests were run because no tests were loaded or the selected tests are disabled.

This error is produced because the Test Adapter is looking for a few meta attributes in the project that are added when you using the New Test Project template.

To manually create a MS Test project, in Visual Studio:

  1. Create a new Class Library project
  2. Add a reference to: Microsoft.VisualStudio.QualityTools.UnitTestFramework.dll
  3. Right-click your project and choose “Unload Project”.

    unload-project
  4. Right-click on your project and choose “Edit <ProjectName>”

    edit-project
  5. Add the following ProjectTypeGuids element to the first ProjectGroup element:

      <PropertyGroup>
        <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
        <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
        <ProductVersion>9.0.30729</ProductVersion>
        <SchemaVersion>2.0</SchemaVersion>
        <ProjectGuid>{4D38A077-23EE-4E9F-876A-43C33433FFEB}</ProjectGuid>
        <OutputType>Library</OutputType>
        <AppDesignerFolder>Properties</AppDesignerFolder>
        <RootNamespace>Example.ManualTestProject</RootNamespace>
        <AssemblyName>Example.ManualTestProject</AssemblyName>
        <TargetFrameworkVersion>v3.5</TargetFrameworkVersion>
        <FileAlignment>512</FileAlignment>
        <ProjectTypeGuids>{3AC096D0-A1C2-E12C-1390-A8335801FDAB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
      </PropertyGroup>
  6. Right-click on the Project and choose "Reload <Project Name>"

Once reloaded, the handy shortcuts work as expected.

Note for the curious:

  • Guid {FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}: refers to C# project
  • Guid {3AC096D0-A1C2-E12C-1390-A8335801FDAB}: refers to the “Test Project Flavor”

From my limited understanding of how Visual Studio Test package works, it scans the solution looking for projects that can contain tests.  Without the magic ProjectTypeGuid, the class library is excluded from this process.

Happy coding.

submit to reddit