Avoiding Fragile Tests – Drawing the Line

Isolation frameworks are intrusive by nature. Yes, Isolator included. That means, they have “knowledge” of the code inside the component under test. Big surprise there, right?

One the major detriments for people who have already ventured into the land of unit-testing is cost of test maintenance. It could be that the tests are not unit-testy enough (more like integration tests). Or, the test contains too many isolation statements. By using intrusive elements in the test code, you’re introducing this risk

Is writing tests for this code worth it? I’ve looked at procedural VB code (only 3 years old), with no tests, where functions were hundreds of lines long. Tests for these functions will not be short either.

The developers of this VB code cannot refactor it without the tests (they actually need to get permission to refactor, as this is a medical application, and they are not likely to get it without making sure there’s no risk involved in changing the code). I think tests are worth writing to have the flexibility to change the code, although it may incur the price of maintaining the test code later when refactoring. Overall, I think the cost would be lower.

So let’s say I have code to test. It has layers – Data access, business logic, dependency injection, CSLA, whatever. I usually want to test logic, so I’ll concentrate on a business logic component. With Isolator, I can choose where to draw the line – where to isolate the dependencies. With this choice I can make a brittle test, that will break when the inside code changes, or more robust one.

Here’s a simple example. I have a data class, called Orders:

public class Orders
{
private List<Order> orderList = new List<Order>();

public List<Order> OrderList
{
get { return orderList; }
}

public void ReadOrderData(string connectionString)
{

string queryString =
"SELECT OrderID, CustomerID FROM dbo.Orders;";

using (SqlConnection connection =
new SqlConnection(connectionString))
{
SqlCommand command =
new SqlCommand(queryString, connection);
connection.Open();

SqlDataReader reader = command.ExecuteReader();

while (reader.Read())
{
orderList.Add(
new Order((int) reader[0], (int) reader[1]));
}

reader.Close();
}
}
}

As you can see, the ReadOrderData method goes to the database, fills the orderList with Order objects it creates from the database. So in my test for a business logic component that uses Orders, I can write the following lines to fake all database access:

[TestMethod]
public void FakeDataObjects()
{
var fakeConnection = Isolate.Fake.Instance<SqlConnection>();
Isolate.Swap.NextInstance<SqlConnection>().With(fakeConnection);

var fakeCommand = Isolate.Fake.Instance<SqlCommand>();
Isolate.Swap.NextInstance<SqlCommand>().With(fakeCommand);

var fakeReader = fakeCommand.ExecuteReader();

Isolate.WhenCalled(()=>fakeReader.Read()).WillReturn(true);

Isolate.WhenCalled(()=>fakeReader[0]).WillReturn(1);
Isolate.WhenCalled(()=>fakeReader[1]).WillReturn(2);

Isolate.WhenCalled(()=>fakeReader.Read()).WillReturn(false);

Orders orders = new Orders();
orders.ReadOrderData("MyDB");

Assert.AreEqual(1, orders.OrderList.Count);

}

See that? Even with Isolator’s API (which reduces the necessity to specify isolation for every line) it still has 9 lines of isolation statement. Each line has indirect relation to a line code inside the Orders object. That’s at least 9 possibilities of test failure, if and when I change the data access code.

I can also draw the isolation line at a more higher level – skip the data access bit, and return a fake List:

[TestMethod]
public void FakeOrder()
{
Orders orders = new Orders();

List<Order> fakeList = new List<Order>();
fakeList.Add(new Order(1,2));

Isolate.WhenCalled(()=>orders.OrderList).WillReturn(fakeList);

Assert.AreEqual(1, orders.OrderList.Count);

}

Now I only have a single line that relates to the code under test. We’ve reduced the risk of the test code breaking because of changes in the production code.

In general, I’d say the line of isolation should be as close to the tested code. You will use less isolation statements, that will make for a more robust test.

  • Andrew Woodward

    Gil,

    The question that will be asked when trying to sell the virtues of Unit Testing will be, what are you actually testing? That a method returns a list you tell it to return?

  • Gil Zilberfeld

    Andrew,

    Obviously, the Assert is contrived, and is used to show the what the result would be.

    A real business object would call the same methods on the Orders object, not asserting on it, and my test would be about the logic of the business object.

  • adiel

    So how do you suggest to test the ReadOrderData method?

  • Gil Zilberfeld

    Adiel,

    Ah. Good question.
    What I described is when the data object is a dependency of the test code (i.e. the business logic).

    If I want to test the ReadOrderData, then the database access objects are the dependency, and then I would have to fake those methods.

    What I would do in that case, is fake the database components, and use Isolate.Verify.WasCalledWithExactArguments
    to make sure the correct arguments are sent.

  • Andrew Woodward

    Gil,

    Would be good to show how this looked with the two test split out. The db tests will still be brittle which is often used as a reason not to do it.

  • Gil Zilberfeld

    Andrew,

    If I write a test for ReadOrderData, it will be as brittle as if I wrote one for the business component ,with faking the database bits. The difference is with the business component I have a choice where to draw the line.

    So should you do it? It comes back to cost effectiveness. My first answer would be no, especially for data layer code that was auto-generated. But if you don’t trust your code to work – write a test. Once you feel you maintain it too much – rethink what you’re doing.

    After all, brittle tests are not reasons not to write tests. When a test breaks, you get feedback, which prompts a response. Then you decide what to do with it.

TOP