Faking AppSettings in tests

If you have some code that uses configuration from an app.config or web.config file, and you need to retrospectively write tests for it, there are several approaches you can take to get fake setting into your tests.

Given this code

using System;
using System.Configuration;
using System.IO;

namespace Demo
{
    class Program
    {
        static void Main(string[] args)
        {
            using (var writer = new StringWriter())
            {
                new SeasonWriter().WriteSeason(writer);
                Console.WriteLine(writer);
                Console.ReadLine();
            }
        }
    }

    public class SeasonWriter
    {
        public void WriteSeason(TextWriter textWriter)
        {
            var month = ConfigurationManager.AppSettings["Month"];
            switch (month)
            {
                case "January":
                case "February":
                case "December":
                    textWriter.Write("Winter");
                    break;
                case "March":
                case "April":
                case "May":
                    textWriter.Write("Spring");
                    break;
                case "June":
                case "July":
                case "August":
                    textWriter.Write("Summer");
                    break;
                case "September":
                case "October":
                case "November":
                    textWriter.Write("Autumn");
                    break;
                default:
                    throw new ConfigurationErrorsException("No setting defined for 'Month'."
                        + Environment.NewLine
                        + "Please add a setting to the .exe.config file under the section AppSettings."
                        + Environment.NewLine
                        + "For more info on the .exe.config file, follow this link "
                        + "http://msdn.microsoft.com/en-us/library/vstudio/1fk1t1t0(v=vs.110).aspx");
            }
        }
    }
}

In order to get tests around the SeasonWriter class, the AppSettings entry "Month" needs to be faked or abstracted. Some options are:

Create an app.config file in the test project

This is the most obvious starting point. Create a test project to contain the SeasonWriterTests file, with an app.config containing a "Month" entry:

<appSettings>
  <add key="Month" value="January"/>
</appSettings>

using NUnit.Framework;
using System.IO;

namespace Demo.Test
{
    [TestFixture]
    public class SeasonWriterTests
    {
        [Test]
        public void WriteSeasonExpectWinter()
        {
            var writer = new StringWriter();
            var target = new SeasonWriter();
            target.WriteSeason(writer);
            Assert.That(writer.ToString(), Is.EqualTo("Winter"));
        }
    }
}

This will allow the test to run, but will limit you to testing a single path through the code. It's also flaky, because your test depends on a magic string in a separate file.

Pass the configuration setting in

This solution is a refactor of the WriteSeason method, so it takes the config setting in as a string. This sounds like a clean fix, with benefits

• the responsibility for getting that value has been handed off to the user of the method
• the code is easier to test

However, there are downsides

• the refactor is an untested change
• there could be many users of this method, and each one would be responsible for getting the month value
• the exception type in the WriteSeason method is no longer correct

Set the config value

The easiest way to fake an AppSettings value is just to set it in your test class. ConfigurationManager.AppSettings is just a NameValueCollection, and items can be written as well as read:

using NUnit.Framework;
using System.IO;
using System.Configuration;

namespace Demo.Test
{
    [TestFixture]
    public class SeasonWriterTests
    {
        [TestCase("January", "Winter")]
        [TestCase("February", "Winter")]
        [TestCase("March", "Spring")]
        [TestCase("April", "Spring")]
        [TestCase("May", "Spring")]
        [TestCase("June", "Summer")]
        [TestCase("July", "Summer")]
        [TestCase("August", "Summer")]
        [TestCase("September", "Autumn")]
        [TestCase("October", "Autumn")]
        [TestCase("November", "Autumn")]
        [TestCase("December", "Winter")]
        public void WriteSeasonExpectCorrectSeasonForMonth(string month, string expectedSeason)
        {

          ConfigurationManager.AppSettings["Month"] = month;

            var writer = new StringWriter();
            var target = new SeasonWriter();
            target.WriteSeason(writer);
            Assert.That(writer.ToString(), Is.EqualTo(expectedSeason));
        }
    }
}

Refactoring legacy code to add tests

Imagine you have some legacy code in your product. Code that's been added to over the years by several different developers, none of who were certain they understood the code. There is no separation of concerns; the code has business logic and database access baked in. There's no useful documentation and the code comments are no more than sarcastic outbursts of cynicism.

A bug report has come in that suggests some of the SQL being executed from this method is invalid. So, what's the plan? Hack the SQL construction around until it feels like it does what you want, hoping you haven't broken anything else in the class? Or take a more structured approach?

To illustrate the problem, I have written some legacy-style code so I can show how to refactor it to make it testable. The techniques here are a pragmatic approach to getting legacy code under test, while making as few untested changes as necessary. Because as you will already know, a class is only able to be refactored when it's under test.

The code for the class is available on GitHub here, and looks like this

using System.Collections;

namespace CombinationOfConcerns
{
    public class Data
    {
        private readonly string _type;

        public Data(string type)
        {
            _type = type;
        }

        public ArrayList GetData(string code, string gdr, string filterType, bool getAge)
        {
            ArrayList results = null;

            var sql = "select id from people where type = " + _type + " and gender ='" + gdr + "'";
            string values = DatabaseWrapper.GetSingleResult(sql);
            for (int i = 0; i < values.Split(',').Length; i++)
            {
                switch (code)
                {
                    case "128855":
                        sql = "select name";
                        if (getAge)
                            sql += ",age";                    
                        sql += ",gender from people where type in(" + values + ")";
                        if (filterType == "45")
                        {
                            sql += " and gender='" + gdr + "'";
                        }
                        if (filterType == "87")
                        {
                            sql += " and shoesize = 43";
                        }
                        results = DatabaseWrapper.GetManyResults(sql);
                        break;
                    case "1493":
                        sql = "select dogs.name, dogbreeds.breed from";
                        sql += " dogs inner join dogbreeds on";
                        sql += " dogs.breed = breed.breed where dogs.ownerId in(" + values + ")";
                        if (filterType == "12")
                        {
                            sql += " and coat='curly'";
                        }
                        results = DatabaseWrapper.GetManyResults(sql);
                        break;
                    default:
                        sql = "select name, population from countries,people on where people.countryname = countries.name and people.id in(" + values + ")";
                        if (filterType == "f")
                        {
                            sql += " and countries.continent='South America'";
                        }
                        if (filterType == "642")
                        {
                            sql += " and countries.continent='Europe'";
                        }
                        results = DatabaseWrapper.GetManyResults(sql);
                        break;
                }
            }
            return results;
        }
    }
}

Run the code

Some great advice from Michael Feathers goes something like this: "if you need to see what some code does, run it." In practice, this means write a test that exercises the code, and see how far it gets without failing. If it doesn't fail, that route through the code has no dependencies and will be easier to write tests for. If it does fail, it will highlight dependencies that need to be abstracted.

To run some code, the first job is to find a suitable entry point to the method. If the method to test is public or internal, that's the entry point. If not, is the method called from a public or internal method? If not, the method should be made internal to allow it to be called directly.

The method on the Data class that needs testing is the public GetData method. Here's the first test:

        [TestFixture]
        public class GetData
        {
            [Test]
            public void WithEmptyArgs()
            {
                var target = new Data("");
                target.GetData("", "", "", false);
            }
        }

When this runs, the GetData method fails on a call to the DatabaseWrapper class, showing that there's a dependency on that class that needs to be faked for testing.

        public ArrayList GetData(string code, string gdr, string filterType, bool getAge)
        {
            ArrayList results = null;

            var sql = "select id from people where type = " + _type + " and gender ='" + gdr + "'";
            string values = DatabaseWrapper.GetSingleResult(sql);
            //Code fails in call to GetSingleResult

Deal with dependencies

If the DatabaseWrapper class was an instance, the best way to deal with it would be to extract its interface and use that interface through the code, allowing a stub to be used from a test. However, in this example DatabaseWrapper is a static class, so needs to be dealt with in a different way.

One way would be to add an instance wrapper class around the static class (DatabaseWrapperWrapper..?). This would call the static method from within that class's instance methods, and allow an interface to be used in the GetData method.

Another way is to add a protected virtual method in the Data class that wraps the static method; this allows the test code to use a subclass of the Data class, and override the virtual method. I'll demonstrate that. Here's the new virtual method and its usage in the GetData method:

        protected virtual string GetSingleResult(string sql)
        {
            return DatabaseWrapper.GetSingleResult(sql);
        }

        public ArrayList GetData(string code, string gdr, string filterType, bool getAge)
        {
            ArrayList results = null;

            var sql = "select id from people where type = " + _type + " and gender ='" + gdr + "'";

            //Now calls virtual method
            string values = GetSingleResult(sql);

and here's the usage in the test project. There's a new class, FakeData, that extends Data. This has an override of the virtual GetSingleResult method, returning an empty string for now. Note that the FakeData class is the new target for the test.

        [TestFixture]
        public class GetData
        {
            [Test]
            public void WithEmptyArgs()
            {
                var target = new FakeData("");
                target.GetData("", "", "", false);
            }

            private class FakeData : Data
            {
                public FakeData(string type)
                    : base(type)
                { }

                protected override string GetSingleResult(string sql)
                {
                    return "";
                }
            }
        }

The code under test now runs up to the call to DatabaseWrapper.GetManyResults, so the GetManyResults method needs a virtual wrapper the same as GetSingleResult did:

        protected virtual ArrayList GetManyResults(string sql)
        {
            return DatabaseWrapper.GetManyResults(sql);
        }

the calls to DatabaseWrapper.GetManyResults need to be changed to the local GetManyResults:

...
    default:
        sql = "select name, population from countries,people on where people.countryname = countries.name and people.id in(" + values + ")";
        if (filterType == "f")
        {
            sql += " and countries.continent='South America'";
        }
        if (filterType == "642")
        {
            sql += " and countries.continent='Europe'";
        }

        // Now calls the virtual method
        results = GetManyResults(sql);
        break;
...

and the FakeData class in the tests needs to override the method:

            private class FakeData : Data
            {
                public FakeData(string type)
                    : base(type)
                { }

                protected override string GetSingleResult(string sql)
                {
                    return "";
                }

                protected override ArrayList GetManyResults(string sql)
                {
                    return new ArrayList();
                }
            }

Now there's a test that mocks a call to the database, and runs through the method from start to finish. But because of the many responsibilities of the code, what needs to be asserted - the SQL that is being executed on the database - is not exposed outside the GetData method.

Sensing variables

In order to see the value of the SQL being executed, the test class will use a "sensing variable". This is a variable that stores a runtime value so that it is available for a test to perform an assertion on. In this example, the sensing variable will be on the FakeData class, and will store the value of the SQL string passed to the GetManyResults method.

        [TestFixture]
        public class GetData
        {
            [Test]
            public void WithEmptyArgs()
            {
                var target = new FakeData("");
                target.GetData("", "", "", false);
                // Use the sensing variable in the assertion
                Assert.That(target.ExecutedSql, Is.EqualTo(""));
            }

            private class FakeData : Data
            {
                // The sensing variable
                public string ExecutedSql { get; private set; }

                public FakeData(string type)
                    : base(type)
                { }

                protected override string GetSingleResult(string sql)
                {
                    return "";
                }

                protected override ArrayList GetManyResults(string sql)
                {
                    //Store the value in the sensing variable
                    ExecutedSql = sql;
                    return new ArrayList();
                }
            }
        }

A well-placed breakpoint allows the sensing variable's value to be harvested, and used as the expected result for that test.

            [Test]
            public void WithEmptyArgs()
            {
                var target = new FakeData("");
                target.GetData("", "", "", false);
                
                const string expected = "select name, population "
                    + "from countries,people on where people.countryname = "
                    + "countries.name and people.id in()";
                
                Assert.That(target.ExecutedSql, Is.EqualTo(expected));
            }

Which gives the first green test!

Although this single test is not particularly useful, the journey to get to it has shown some techniques that are available when tackling legacy code. In future posts I'll write more tests and get to a point where it's safe to refactor the code. The I'll refactor the hell out of it.

In Summary

1. Run the code
   • Identify the start point
   • Make method visible (public/internal)
   • Write a test to exercise the code
2. Deal with dependencies
   • Introduce interface for instance class
   • Wrap a static class in an instance class
   • Create virtual method to hide static method calls
3. Sensing variables
   • Use to surface runtime values

If there are other techniques that you employ to refactor legacy code, I'd like to know what they are - they are all useful.

The before and after code used in this example is on GitHub at orangutanboy/LegacyCodeUnderTest

Don't write new unit tests during the refactoring phase

If you're doing unit tests, you will be familiar with the Red-Green-Refactor cycle - write a failing test (red), make it pass (green) then drive out the design of the code (refactor).

When it comes to the refactor phase, you shouldn't need to write new tests - the refactor phase is when you change the implementation of the code, not the behaviour.

Take the following green code, which returns the min and max temperatures for three states of water:

using System;
namespace H2OLib
{
    public class H2O
    {
        public enum State
        {
            Gas,
            Liquid,
            Solid
        }

        private readonly State _state;

        public H2O(State state)
        {
            if (state != State.Gas && state != State.Liquid && state != State.Solid)
            {
                throw new ArgumentOutOfRangeException();
            }
            _state = state;
        }

        public int MaxTemp
        {
            get
            {
                switch (_state)
                {
                    case State.Gas:
                        return 374;
                    case State.Liquid:
                        return 100;
                    default:
                        return 0;
                }
            }
        }

        public int MinTemp
        {
            get
            {
                switch (_state)
                {
                    case State.Gas:
                        return 100;
                    case State.Liquid:
                        return 0;
                    default:
                        return -230;
                }
            }
        }
    }
}

This code is tested by the following tests:

    [TestFixture]
    public class H2OTests
    {
        [Test]
        public void GivenGasStateThenExpectCorrectTemps()
        {
            var h2o = new H2O(H2O.State.Gas);
            Assert.That(h2o.MinTemp, Is.EqualTo(100));
            Assert.That(h2o.MaxTemp, Is.EqualTo(374));
        }

        [Test]
        public void GivenLiquidStateThenExpectCorrectTemps()
        {
            var h2o = new H2O(H2O.State.Liquid);
            Assert.That(h2o.MinTemp, Is.EqualTo(0));
            Assert.That(h2o.MaxTemp, Is.EqualTo(100));
        }

        [Test]
        public void GivenSolidStateThenExpectCorrectTemps()
        {
            var h2o = new H2O(H2O.State.Solid);
            Assert.That(h2o.MinTemp, Is.EqualTo(-230));
            Assert.That(h2o.MaxTemp, Is.EqualTo(0));
        }

        [Test]
        public void GivenUnknownStateThenExceptionThrown()
        {
            Assert.Throws<ArgumentOutOfRangeException>(() => new H2O((H2O.State)99));
        }
    }

The code behaves exactly as planned, but the switch statements are a bit smelly. One solution is to refactor to the State pattern, with a factory to instantiate the correct state class based on the value that is passed to the H2O constructor. So, what's the next thing you do? Is it a) write a test for the StateFactory:

namespace H2OLib.Test
{
    [TestFixture]
    public class StateFactoryTests
    {
        [Test]
        public void ConstructExpectNoError()
        {
            Assert.DoesNotThrow(() => new StateFactory());
        }
    }
}

or b) don't write a test for the StateFactory:

// This line left intentionally blank

If you read the title of the post, you'll probably know that the answer is b. Given the tests that are in place, the code can be refactored to the following with no additional tests required:

using System;

namespace H2OLib
{
    public class H2O
    {
        private static StateFactory _stateFactory = new StateFactory();

        public enum State
        {
            Gas,
            Liquid,
            Solid
        }

        private readonly IState _state;

        public H2O(State state)
        {
            _state = _stateFactory.CreateState(state);
        }

        public int MaxTemp
        {
            get
            {
                return _state.MaxTemp;
            }
        }

        public int MinTemp
        {
            get
            {
                return _state.MinTemp;
            }
        }
    }

    public interface IState
    {
        int MinTemp { get; }
        int MaxTemp { get; }
    }

    public class StateFactory
    {
        public IState CreateState(H2OLib.H2O.State state)
        {
            switch (state)
            {
                case H2O.State.Gas:
                    return new GasState();

                case H2O.State.Liquid:
                    return new LiquidState();

                case H2O.State.Solid:
                    return new SolidState();

                default:
                    throw new ArgumentOutOfRangeException();
            }
        }
    }

    public class GasState : IState
    {
        public int MinTemp
        {
            get { return 100; }
        }

        public int MaxTemp
        {
            get { return 374; }
        }
    }

    public class LiquidState : IState
    {
        public int MinTemp
        {
            get { return 0; }
        }

        public int MaxTemp
        {
            get { return 100; }
        }
    }

    public class SolidState : IState
    {
        public int MinTemp
        {
            get { return -230; }
        }

        public int MaxTemp
        {
            get { return 0; }
        }
    }
}

When you write a new test you're not refactoring; it's a new red phase.