How could I refactor this factory-type method and database call to be testable?

Question

I'm trying to learn how to do Unit Testing and Mocking. I understand some of the principles of TDD and basic testing. However, I'm looking at refactoring the below code that was written without tests and am trying to understand how it needs to change in order to make it testable.

public class AgentRepository
{

public Agent Select(int agentId)
{
    Agent tmp = null;
    using (IDataReader agentInformation = GetAgentFromDatabase(agentId))
    {
        if (agentInformation.Read())
        {
            tmp = new Agent();
            tmp.AgentId = int.Parse(agentInformation["AgentId"].ToString());
            tmp.FirstName = agentInformation["FirstName"].ToString();
            tmp.LastName = agentInformation["LastName"].ToString();
            tmp.Address1 = agentInformation["Address1"].ToString();
            tmp.Address2 = agentInformation["Address2"].ToString();
            tmp.City = agentInformation["City"].ToString();
            tmp.State = agentInformation["State"].ToString();
            tmp.PostalCode = agentInformation["PostalCode"].ToString();
            tmp.PhoneNumber = agentInformation["PhoneNumber"].ToString();
        }
    }

    return tmp;
}

private IDataReader GetAgentFromDatabase(int agentId)
{
    SqlCommand cmd = new SqlCommand("SelectAgentById");
    cmd.CommandType = CommandType.StoredProcedure;

    SqlDatabase sqlDb = new SqlDatabase("MyConnectionString");
    sqlDb.AddInParameter(cmd, "AgentId", DbType.Int32, agentId);
    return sqlDb.ExecuteReader(cmd);
}

}

These two methods are in a single class. The database-related code in the GetAgentFromDatabase is related to Enterprise Libraries.

How would I be able to go about making this testable? Should I abstract out the GetAgentFromDatabase method into a different class? Should GetAgentFromDatabase return something other than an IDataReader? Any suggestions or pointers to external links would be greatly appreciated.

Answer 1

You're correct about moving GetAgentFromDatabase() into a separate class. Here's how I redefined AgentRepository:

public class AgentRepository {
    private IAgentDataProvider m_provider;

    public AgentRepository( IAgentDataProvider provider ) {
        m_provider = provider;
    }

    public Agent GetAgent( int agentId ) {
        Agent agent = null;
        using( IDataReader agentDataReader = m_provider.GetAgent( agentId ) ) {
            if( agentDataReader.Read() ) {
                agent = new Agent();
                // set agent properties later
            }
        }
        return agent;
    }
}

where I defined the IAgentDataProvider interface as follows:

public interface IAgentDataProvider {
    IDataReader GetAgent( int agentId );
}

So, AgentRepository is the class under test. We'll mock IAgentDataProvider and inject the dependency. (I did it with Moq, but you can easily redo it with a different isolation framework).

[TestFixture]
public class AgentRepositoryTest {
    private AgentRepository m_repo;
    private Mock<IAgentDataProvider> m_mockProvider;

    [SetUp]
    public void CaseSetup() {
        m_mockProvider = new Mock<IAgentDataProvider>();
        m_repo = new AgentRepository( m_mockProvider.Object );
    }

    [TearDown]
    public void CaseTeardown() {
        m_mockProvider.Verify();
    }

    [Test]
    public void AgentFactory_OnEmptyDataReader_ShouldReturnNull() {
        m_mockProvider
            .Setup( p => p.GetAgent( It.IsAny<int>() ) )
            .Returns<int>( id => GetEmptyAgentDataReader() );
        Agent agent = m_repo.GetAgent( 1 );
        Assert.IsNull( agent );
    }

    [Test]
    public void AgentFactory_OnNonemptyDataReader_ShouldReturnAgent_WithFieldsPopulated() {
        m_mockProvider
            .Setup( p => p.GetAgent( It.IsAny<int>() ) )
            .Returns<int>( id => GetSampleNonEmptyAgentDataReader() );
        Agent agent = m_repo.GetAgent( 1 );
        Assert.IsNotNull( agent );
                    // verify more agent properties later
    }

    private IDataReader GetEmptyAgentDataReader() {
        return new FakeAgentDataReader() { ... };
    }

    private IDataReader GetSampleNonEmptyAgentDataReader() {
        return new FakeAgentDataReader() { ... };
    }
}

(I left out the implementation of class FakeAgentDataReader, which implements IDataReader and is trivial -- you only need to implement Read() and Dispose() to make the tests work.)

The purpose of AgentRepository here is to take IDataReader objects and turn them into properly formed Agent objects. You can expand the above test fixture to test more interesting cases.

After unit-testing AgentRepository in isolation from the actual database, you will need unit tests for a concrete implementation of IAgentDataProvider, but that's a topic for a separate question. HTH

Answer 2

The problem here is deciding what is SUT and what is Test. With your example you are trying to Test the Select() method and therefore want to isolate that from the database. You have several choices,

1. Virtualise the GetAgentFromDatabase() so you can provide a derived class with code to return the correct values, in this case creating an object that provides IDataReaderFunctionaity without talking to the DB i.e.

   class MyDerivedExample : YourUnnamedClass
   {
       protected override IDataReader GetAgentFromDatabase()
       {
           return new MyDataReader({"AgentId", "1"}, {"FirstName", "Fred"},
             ...);
       }
   }
   

2. As Gishu suggested instead of using IsA relationships (inheritance) use HasA (object composition) where you once again have a class that handles creating a mock IDataReader, but this time without inheriting.

   However both of these result in lots of code that simply defines a set of results that we be returned when queried. Admittedly we can keep this code in the Test code, instead of our main code, but its an effort. All you are really doing is define a result set for particular queries, and you know what’s really good at doing that... A database

3. I used LinqToSQL a while back and discovered that the DataContext objects have some very useful methods, including DeleteDatabase and CreateDatabase.

   public const string UnitTestConnection = "Data Source=.;Initial Catalog=MyAppUnitTest;Integrated Security=True";
   
   
   [FixtureSetUp()]
   public void Setup()
   {
     OARsDataContext context = new MyAppDataContext(UnitTestConnection);
   
     if (context.DatabaseExists())
     {
       Console.WriteLine("Removing exisitng test database");
       context.DeleteDatabase();
     }
     Console.WriteLine("Creating new test database");
     context.CreateDatabase();
   
     context.SubmitChanges();
   }
   

Consider it for a while. The problem with using a database for unit tests is that the data will change. Delete your database and use your tests to evolve your data that can be used in future tests.

There are two things to be careful of Make sure your tests run in the correct order. The MbUnit syntax for this is [DependsOn("NameOfPreviousTest")]. Make sure only one set of tests is running against a particular database.