Any well functioning development team ought to be able to discuss software design and the various approaches to raising quality without the matter becoming an HR issue. Incredibly, the following disagreement took on a life of its own, eventually creating a monster! Consider the following:
class DAL_Untestable
{
public void Save(string currency1, string currency2, bool spotRate)
{
using(var conn = new SqlConnection("connectionString"))
using (var cmd = new SqlCommand("SpotRate_Insert", conn))
{
conn.Open();
cmd.Parameters.Add("@currency1", SqlDbType.VarChar).Value = currency1;
cmd.Parameters.Add("@currency2", SqlDbType.VarChar).Value = currency2;
cmd.ExecuteNonQuery();
}
}
}
It's pretty clear that the code above isn't written in such a way that it can be unit tested. The question here is whether this is indeed important. By no means am I saying that testing has no place here, rather, let's look at an example of something a colleague might do?
class BO_DTO
{
public string Currency1 { get; set; }
public string Currency2 { get; set; }
public decimal SpotRate { get; set; }
public BO_DTO(string currency1, string currency2, decimal spotRate)
{
this.Currency1 = currency1;
this.Currency2 = currency2;
this.SpotRate = spotRate;
}
}
class DAL_Testable
{
public IDbConnection Connection { get; set; }
public IDbCommand Command { get; set; }
public void Save(BO_DTO dto)
{
Command.Connection = Connection;
Command.CommandText = "SpotRate_Insert";
Command.CommandType = CommandType.StoredProcedure;
var currency1Param = Command.CreateParameter();
currency1Param.ParameterName = "@currency1";
currency1Param.DbType = DbType.String;
currency1Param.Direction = ParameterDirection.Input;
currency1Param.Value = dto.Currency1;
var currency2Param = Command.CreateParameter();
currency2Param.ParameterName = "@currency2";
currency2Param.DbType = DbType.String;
currency2Param.Direction = ParameterDirection.Input;
currency2Param.Value = dto.Currency2;
var spotRateParam = Command.CreateParameter();
spotRateParam.ParameterName = "@spotRate";
spotRateParam.DbType = DbType.String;
spotRateParam.Direction = ParameterDirection.Input;
spotRateParam.Value = dto.SpotRate;
Connection.Open();
Command.ExecuteNonQuery();
}
}
By exposing the Connection and Command objects we're able to supply mocked instances in order to isolate any tests from ever making a database call. Nothing clever here. The use of the DTO (data transfer object) serves only to better mimic the test I'm reproducing. And these are examples of the tests suggested:
[TestFixture]
class DAL_Testable_Tests
{
public void Save_Should_Call_Correct_Stored_Procedure()
{
var mockConn = new DynamicMock(typeof(IDbConnection));
var mockCmd = new DynamicMock(typeof(IDbCommand));
var dal = new DAL_Testable();
dal.Connection = (IDbConnection)mockConn.MockInstance;
dal.Command = (IDbCommand) mockCmd.MockInstance;
var parameter1 = new SqlParameter();
mockCmd.ExpectAndReturn("CreateParameter", parameter1);
var parameter2 = new SqlParameter();
mockCmd.ExpectAndReturn("CreateParameter", parameter2);
var parameter3 = new SqlParameter();
mockCmd.ExpectAndReturn("CreateParameter", parameter3);
mockCmd.Expect("set_CommandText", "SpotRate_Insert");
mockCmd.Expect("ExecuteNonQuery");
var dto = new BO_DTO("N/A", "N/A", 0.0m);
dal.Save(dto);
mockCmd.Verify();
}
public void Save_Should_Use_Correct_Parameters()
{
var mockConn = new DynamicMock(typeof(IDbConnection));
var mockCmd = new DynamicMock(typeof(IDbCommand));
var dal = new DAL_Testable();
dal.Connection = (IDbConnection)mockConn.MockInstance;
dal.Command = (IDbCommand)mockCmd.MockInstance;
var parameter1 = new DynamicMock(typeof(DbParameter));
mockCmd.ExpectAndReturn("CreateParameter", parameter1.MockInstance);
parameter1.Expect("set_Value", "EUR");
var parameter2 = new DynamicMock(typeof(DbParameter));
mockCmd.ExpectAndReturn("CreateParameter", parameter2.MockInstance);
parameter2.Expect("set_Value", "USD");
var parameter3 = new DynamicMock(typeof(DbParameter));
mockCmd.ExpectAndReturn("CreateParameter", parameter3.MockInstance);
parameter3.Expect("set_Value", 1.4m);
var dto = new BO_DTO("EUR", "USD", 1.4m);
dal.Save(dto);
parameter1.Verify();
parameter2.Verify();
parameter3.Verify();
}
}
Do we really need to test that a DAL (data access layer) is capable of calling the correct stored procedure with the correct parameters given a DTO? Is this the most value we can add assuming that automated tests need to be written? This isn't a million miles from the question about whether getters & setters require unit tests. In trivial cases such as this one above I don't understand why we'd choose unit tests over integration tests given our particular circumstance.
We have a database against which we can run integration test. The integration tests can be automated to run periodically and if they exist in their own project then the responsive feedback aspect of unit tests is preserved. Integration tests will reveal issues beyond the scope of this one method such as whether the correct data types are being used, whether the signature on the stored procedure is correct, whether the tables used by the stored procedure exist and, ultimately, whether the method call results in the correct spot rate value being saved for a given currency pair.
Once we've understood the rules about granularity and breaking up code (methods) into the smallest pieces that make sense I think we've reached a stage where we can do some chunking for more suitable testing. We care whether we're calling the correct stored procedure and the test for that should be determined by values in the database. Yes, it's possible to have written the stored procedure incorrectly so that it fails to persist any or all of the data passed through but this also happens to matter! If we decide to rename the stored procedure as part of a database refactoring then the unit test is flawed until we change the expectation of the first test. This suggests that we are in fact testing implementation rather than logic and, as far as I understand it, this is not the idea behind unit testing.
Google unit test behaviour not implementation and have a look at what various folks say.