Correct Seam for Stub

Question

In Chapter 3 of his book The Art of Unit Testing: with Examples in C#, Roy Osherove illustrates the issue of undesirable external dependencies in code under test.

He shows this with a method named IsValidLogFileName which takes in a filename string as an argument, reads in a config file from the local filesystem and checks whether the given filename extension exists in the config file. Here's what such a class would look like:

   public class LogAnalyzer
    {
        public bool IsValidLogFileName(string fileName)
        {

            string [] extensions = System.IO.File.ReadAllLines(@"C:\Users\Public\ext.config");

            foreach (string ext in extensions)
            {
                if (fileName.EndsWith(ext))
                {
                    return true;
                }
            }
            return false;
        }
    }

The issue here is that testing IsValidLogFileName has a dependency on the local filesystem.

The author introduces the concept of stubs as a means of removing this dependency. In particular, he describes the following design:

Create an interface:

    interface IExtensionManager
    {
        bool IsValid(string fileName);
    }

an implementing production class:

    class FileExtensionManager : IExtensionManager
    {
        bool IsValid(string fileName)
        {
            string [] extensions = System.IO.File.ReadAllLines(@"C:\Users\Public\ext.config");

            foreach (string ext in extensions)
            {
                if (fileName.EndsWith(ext))
                {
                    return true;
                }
            }
            return false;
        }
    }

a stub:

    class FakeExtensionManager : IExtensionManager
    {
        bool IsValid(string fileName)
        {
            string [] extensions = { "txt", "c", "cpp", "cs"};

            foreach (string ext in extensions)
            {
                if (fileName.EndsWith(ext))
                {
                    return true;
                }
            }
            return false;
        }
    }

refactor LogAnalyzer as follows:

   public class LogAnalyzer
    {
        IExtensionManager em;

        public LogAnalyzer(IExtensionManager extensionManager)
        {
            em = extensionManager;
        }

        public bool IsValidLogFileName(string fileName)
        {
            return em.IsValid(fileName);
        }
    }

run production code like this:

    LogAnalyzer la = new LogAnalyzer(new FileExtensionManager);

and test code like this:

    LogAnalyzer la = new LogAnalyzer(new FakeExtensionManager);

The part I don't understand is why we have to extract the validation logic from the original IsValidLogFileName by making an ExtensionManager interface. Wouldn't it make more sense to create something like a ConfigManager interface and refactor as follows:

interface:

    interface IConfigManager
    {
        string [] GetFileExtensionConfig();
    }

an implementing production class:

    class LocalConfigManager : IConfigManager
    {
        string [] GetFileExtensionConfig( )
        {
            return System.IO.File.ReadAllLines(@"C:\Users\Public\ext.config");
        }
    }

a stub:

    class FakeConfigManager : IConfigManager
    {
        string [] GetFileExtensionConfig( )
        {
            string [] extensions = { "txt", "c", "cpp", "cs"};
            return extensions;
        }
    }

refactor LogAnalyzer as follows:

   public class LogAnalyzer
    {
        IConfigManager cm;

        public LogAnalyzer(IConfigManager configManager)
        {
            cm = configManager;
        }

        public bool IsValidLogFileName(string fileName)
        {
            string [] extensions = cm.GetFileExtensionConfig( );

            foreach (string ext in extensions)
            {
                if (fileName.EndsWith(ext))
                {
                    return true;
                }
            }
            return false;
    }

run production code like this:

    LogAnalyzer la = new LogAnalyzer(new LocalConfigManager);

and test code like this:

    LogAnalyzer la = new LogAnalyzer(new FakeConfigManager);

Answer 1

Roy here (book author). I like your implementation of solving this problem. For one, it only abstracts away the IO part, and not the logic, leaving the logic in a testable location. Second, in your implementation the LogAnalyzer class still does something. In my implementation in the book, after refactoring, LogAnalyzer does nothing except being a forwarding mechanism toward the Extension manager.

One thing to notice in your original explanation of what I show in the book:

I don't recommend that the fake extension manager do any logic at all, but instead take its queues from the test to know what to return. So logic is not implemented twice (in prod and fake code).

In essence, you got the idea. Fake out the dependency code. It's a good implementation of the solution for such a small and simple piece of code and I would have no problem with it if I saw you do it in real world production code. I'd probably say "Good job!".

Then I would go on to say that the fake extension config should not have hard coded extensions, but should be configured by the test so that :

1. the test is more readable

2. the fake class is more reusable in other tests where you might want to simulate other configurations

   FakeConfigMgr fakeConfig  = new FakeConfigMgr();
   fakeConfig.ValidExtensionsWillBe = new []("txt","ext");
   LogAnalyzer la = new LogAnalyzer(fakeConfig);
   

Answer 2

It does, and this type of thing is roughly what Dependency Injection is all about, but.. as you've seen it starts to get really quite complex even for such a simple, simple function that you're describing. Imagine what it would be like if every class in a large program was made as complex as this!

Now its not so bad using DI in practice as you tend to use it for certain layers of configuration and they have tools to help you manage it all, but it still is more complex than just writing the code.

The problem really is that you cannot just replace the System.IO.File.ReadAllLines call in your tests using the traditional unit testing tools. Ideally, you'd write a test and simply tell the compiler to use a different ReadAllLines call temporarily. Turns out you can do this, use Fakes from Microsoft. This uses a low-level approach to rewrite the compiled code (in a way reminiscent of hackers replacing calls to copy protection with nothing). So you can literally replace the system calls in your tests. The classic example is replacing calls to DateTime.Now() with whatever you want the current time to be.

Frankly, I would scrap all your stub creation testing tools and use Fakes across the board. The old-technology tools that rely on interfaces are obsolete and need to die so you can write the code the way you started (nice and simple) without having to mangle it in artificial ways just to please the tools. Fakes allows you all the benefits of simple, clean code with all the benefits of isolated unit tests!