views:

297

answers:

3

I have an extension method with the following signature (in BuildServerExtensions class)::

public static IEnumerable<BuildAgent> GetEnabledBuildAgents(this IBuildServer buildServer, string teamProjectName)
{
    // Omitted agrument validation and irrelevant code
    var buildAgentSpec = buildServer.CreateBuildAgentSpec(teamProjectName);
}

And another method which calls the first (in BuildAgentSelector class):

public BuildAgent Select(IBuildServer buildServer, string teamProjectName)
{
    // Omitted agrument validation
    IEnumerable<BuildAgent> serverBuildAgents = buildServer.GetEnabledBuildAgents(teamProjectName);

    // Omitted - test doesn't get this far
}

And I am trying to test it using MSTest and Rhino.Mocks (v3.4) with:

    [TestMethod]
    public void SelectReturnsNullOnNullBuildAgents()
    {
        Mocks = new MockRepository();
        IBuildServer buildServer = Mocks.CreateMock<IBuildServer>();

        BuildAgentSelector buildAgentSelector = new BuildAgentSelector();
        using (Mocks.Record())
        {
            Expect.Call(buildServer.GetEnabledBuildAgents(TeamProjectName)).Return(null);
        }

        using (Mocks.Playback())
        {
            BuildAgent buildAgent = buildAgentSelector.Select(buildServer, TeamProjectName);

            Assert.IsNull(buildAgent);
        }
    }

When I run this test I get: System.InvalidOperationException: Previous method 'IBuildServer.CreateBuildAgentSpec("TeamProjectName");' requires a return value or an exception to throw.

This is obviously calling the real extension method rather than the test implementation. My next inclination was to try:

Expect.Call(BuildServerExtensions.GetEnabledBuildAgents(buildServer, TeamProjectName)).Return(null);

Then I noticed that my expectations for Rhino.Mocks to intercept this were probably misplaced.

The question is: How do I eliminate this dependency and make the Select method testable?

Note that the extension method and BuildAgentSelector classes are in the same assembly and I would prefer avoiding changing this or having to turn to something besides an extension method, though another mocking framework is something I would consider if I knew it would handle this situation.

A: 

The test calls the real extension method because that is the only one. No test implementation is created when you mock an IBuildServer because the method is not a member of IBuildServer.

There is no clean solution to this using the setup you have now.

Theoretically TypeMock will mock the static class, but refactoring out the extension methods would provide greater testability.

Jay
+2  A: 

Your extension method is actually written fairly well. Its a side-effect free method, and is extending an interface, rather than a concrete class. Your almost there, but you just need to go a little farther. You are trying to mock the .GetEnabledBuildAgents(...) extension method...however thats not actually mockable (by anything except TypeMock Isolator, which is the only thing that can actually mock statics at the moment...however its fairly pricy.)

You are actually interested in mocking the method on IBuildAgent that your extension method calls internally: .CreateBuildAgentSpec(...). If you think it through, mocking the CreateBuildAgentSpec method will solve your problem. The extension method is "pure", and so really doesn't need to be mocked. It has no state and causes no side effects. It calls a single method on the IBuildAgent interface...which is the first clue that directs you to what really needs to be mocked.

Try the following:

[TestMethod]
public void SelectReturnsNullOnNullBuildAgents()
{
    Mocks = new MockRepository();
    IBuildServer buildServer = Mocks.CreateMock<IBuildServer>();

    BuildAgent agent = new BuildAgent { ... }; // Create an agent
    BuildAgentSelector buildAgentSelector = new BuildAgentSelector();
    using (Mocks.Record())
    {
        Expect.Call(buildServer.CreateBuildAgentSpec(TeamProjectName)).Return(new List<BuildAgent> { agent });
    }

    using (Mocks.Playback())
    {
        BuildAgent buildAgent = buildAgentSelector.Select(buildServer, TeamProjectName);

        Assert.IsNull(buildAgent);
    }
}

By creating a BuildAgent instance, and returning it in a List<BuildAgent>, you effectively return an IEnumerable<BuildAgent> that your Select method may operate on. That should get you going. You may need to do some additional mocking in case simply returning a basic BuildAgent instance isn't sufficient, or in case you need more than one. When it comes to mocking results to be returned, Rhino.Mocks can be a REAL pain in the rear to work with. If you run into troubles (which, given my experience with it, you are quite likely to), I recommend you give Moq a try, as it is a much nicer and more tester-friendly framework to work with. It doesn't require a repository, and eliminates the Record/Playback and using() statement heavy notation that Rhino.Mocks required. Moq also provides additional capabilities that other frameworks don't offer yet that, once you get into heavier mocking scenarios, you will fall in love with (i.e. the It.* methods.)

Hope this helps.

jrista
+1  A: 

After a break and coming back with a fresh head, I realized that I am actually mixing concerns in the BuildAgentSelector class a little. I am getting the agents and selecting them. By separating these two concerns and passing the agents to select directly to the BuildAgentSelector constructor (or a delegate/interface to do so), I am able to separate the concerns, remove the dependencies on both the buildServer and teamProjectName parameters, and simplify the interface in the process. It also achieved the testability results I was looking for on the BuildAgentSelector class. I can test the extension method separately nicely as well.

However, in the end, it merely transferred the testing problem elsewhere. It is better because the concern is better placed, but jrista's answer resolves the issue no matter where the concern is placed.

It is still a bit ugly to have to mock the second layer beneath the code under test. I essentially have to take the mock of the successful path from my extension method testing and reuse this code in my other tests - not difficult, but a bit annoying.

I will give MOQ a try and be careful about getting too happy with writing extension methods.