Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

TDD with filesystem dependencies

I have an integration test LoadFile_DataLoaded_Successfully(). And I want to refactor it to the unit test for breaking dependency with filesytem.

P.S. I am new in TDD:

Here are my production class :

public class LocalizationData
{
    private bool IsValidFileName(string fileName)
    {
        if (fileName.ToLower().EndsWith("xml"))
        {
            return true;
        }
        return false;
    }

    public XmlDataProvider LoadFile(string fileName)
    {
        if (IsValidFileName(fileName))
        {
            XmlDataProvider provider = 
                            new XmlDataProvider
                                 {
                                      IsAsynchronous = false,
                                      Source = new Uri(fileName, UriKind.Absolute)
                                 };

            return provider;
        }
        return null;
    }
}

and my test class (Nunit)

[TestFixture]
class LocalizationDataTest
{
    [Test]
    public void LoadFile_DataLoaded_Successfully()
    {
        var data = new LocalizationData();
        string fileName = "d:/azeri.xml";
        XmlDataProvider result = data.LoadFile(fileName);
        Assert.IsNotNull(result);
        Assert.That(result.Document, Is.Not.Null);
    }
}

Any idea how to refactor it to break filesystem dependency

like image 255
Polaris Avatar asked Sep 09 '10 14:09

Polaris


3 Answers

What you're missing here is inversion of control. For instance, you can introduce the dependency injection principle into your code:

public interface IXmlDataProviderFactory
{
    XmlDataProvider Create(string fileName);
}
public class LocalizationData
{
    private IXmlDataProviderFactory factory;
    public LocalizationData(IXmlDataProviderFactory factory)
    {
        this.factory = factory;
    }

    private bool IsValidFileName(string fileName)
    {
        return fileName.ToLower().EndsWith("xml");
    }

    public XmlDataProvider LoadFile(string fileName)
    {
        if (IsValidFileName(fileName))
        {
            XmlDataProvider provider = this.factory.Create(fileName);
            provider.IsAsynchronous = false;
            return provider;
        }
        return null;
    }
}

In the code above the creation of the XmlDataProvider is abstracted away using an IXmlDataProviderFactory interface. An implementation of that interface can be supplied in the constructor of the LocalizationData. You can now write your unit test as follows:

[Test]
public void LoadFile_DataLoaded_Succefully()
{
    // Arrange
    var expectedProvider = new XmlDataProvider();
    string validFileName = CreateValidFileName();
    var data = CreateNewLocalizationData(expectedProvider);

    // Act
    var actualProvider = data.LoadFile(validFileName);

    // Assert
    Assert.AreEqual(expectedProvider, actualProvider);
}

private static LocalizationData CreateNewLocalizationData(
    XmlDataProvider expectedProvider)
{
    return new LocalizationData(FakeXmlDataProviderFactory()
    {
        ProviderToReturn = expectedProvider
    });
}

private static string CreateValidFileName()
{
    return "d:/azeri.xml";
}

The FakeXmlDataProviderFactory looks like this:

class FakeXmlDataProviderFactory : IXmlDataProviderFactory
{
    public XmlDataProvider ProviderToReturn { get; set; }

    public XmlDataProvider Create(string fileName)
    {
        return this.ProviderToReturn;
    }
}

Now in your test environment you can (and probably should) always create the class under test manually. However, you want to abstract the creation away in factory methods to prevent you having to change many tests when the class under test changes.

In your production environment however, it can become very cumbersome very soon when you manually have to create the class. Especially when it contains many dependencies. This is where IoC / DI frameworks shine. They can help you with this. For instance, when you want to use the LocalizationData in your production code, you might write code like this:

var localizer = ServiceLocator.Current.GetInstance<LocalizationData>();

var data = data.LoadFile(fileName);

Note that I'm using the Common Service Locator as an example here.

The framework will take care of the creation of that instance for you. Using such a dependency injection framework however, you will have to let the framework know which 'services' your application needs. For instance, when I use the Simple Service Locator library as an example (shameless plug that is), your configuration might look like this:

var container = new SimpleServiceLocator();

container.RegisterSingle<IXmlDataProviderFactory>(
    new ProductionXmlDataProviderFactory());

ServiceLocator.SetLocatorProvider(() => container);

This code will usually go in the startup path of your application. Of course the only missing piece of the puzzle is the actual ProductionXmlDataProviderFactory class. Here is it:

class ProductionXmlDataProviderFactory : IXmlDataProviderFactory
{
    public XmlDataProvider Create(string fileName)
    {
        return new XmlDataProvider
        {
            Source = new Uri(fileName, UriKind.Absolute)
        };
    }
}

Please also not that you will probably don't want to new up your LocalizationData in your production code yourself, because this class is probably used by other classes that depend on this type. What you would normally do is ask the framework to create the top most class for you (for instance the command that implements a complete use case) and execute it.

I hope this helps.

like image 189
Steven Avatar answered Oct 08 '22 13:10

Steven


The problem here is that you are not doing TDD. You wrote the production code first, and now you want to test it.

Erase all that code and start again. Write a test first, and then write the code that passes that test. Then write the next test, etc.

What is your goal? Given a string that ends in "xml" (why not ".xml"?) you want an XML data provider based upon a file whose name is that string. Is that your goal?

The first tests would be the degenerate case. Given a string like "name_with_wrong_ending" your function should fail. How should it fail? Should it return null? Or should it throw an exception? You get to think about this and decide in your test. Then you make the test pass.

Now, what about a string like this: "test_file.xml" but in the case where no such file exists? What do you want the function to do in that case? Should it return null? Should it throw an exception?

The simplest way to test this, of course, is to actually run the code in a directory that does not have that file in it. However, if you'd rather write the test so that it does not use the file system (a wise choice) then you need to be able to ask the question "Does this file exist", and then your test needs to force the answer to be "false".

You can do that by creating a new method in your class named "isFilePresent" or "doesFileExist". Your test can override that function to return 'false'. And now you can test that your 'LoadFile' function works correctly when the file doesn't exist.

Of course now you'll have to test that the normal implementation of "isFilePresent" works correctly. And for that you'll have to use the real file system. However, you can keep file system tests out of your LocalizationData tests by creating a new class named FileSystem and moving your 'isFilePresent' method into that new class. Then your LocalizationData test can create a derivative of that new FileSystem class and override 'isFilePresent' to return false.

You still have to test the regular implementation of FileSystem, but that's in a different set of tests, that only get run once.

OK, what's the next test? What does your 'loadFile' function do when the file does exist, but does not contain valid xml? Should it do anything? Or is that a problem for the client? You decide. But if you decide to check it, you can use the same strategy as before. Make a function named isValidXML and have the test override it to return false.

Finally we need to write the test that actually returns the XMLDataProvider. So the final function that 'loadData' should call, after all those other function is, createXmlDataProvider. And you can override that to return an empty or dummy XmlDataProvider.

Notice that in your tests you have never gone to the real file system and really created an XMLDataProvider based on a file. But what you have done is to check every if statement in your loadData function. You've tested the loadData function.

Now you should write one more test. A test that uses the real file system and a real valid XML file.

like image 31
Robert Martin Avatar answered Oct 08 '22 13:10

Robert Martin


When I look at the following code:

public class LocalizationData
{
    private static bool IsXML(string fileName)
    {
        return (fileName != null && fileName.ToLower().EndsWith("xml"));
    }

    public XmlDataProvider LoadFile(string fileName)
    {
        if (!IsXML(fileName)) return null*;
        return new XmlDataProvider{
                                     IsAsynchronous = false,
                                     Source = new Uri(fileName, UriKind.Absolute)
                                  };
    }
}
  • (* I'm not thrilled about the return null. Yuck! that smells.)

Anyway, I would ask the following questions to myself:

  • What could possibly break with this code? Are there any complex logic or fragile code that I should safe-guard myself against?
  • Is there anything complicated to understand or worth highlighting via a test that the code is not able to communicate?
  • Once I've written this code, how frequently do I think I'll revisit (change) it?

The IsXML function is extremely trivial. Probably does not even belong to this class.

The LoadFile function creates a synchronous XmlDataProvide if it gets a valid XML filename.

I would first search who uses LoadFile and from where fileName is being passed. If its external to our program, then we need some validation. If its internal and somewhere else we are already doing the validation, then we are good to go. As Martin suggested, I would recommend refactoring this to take Uri as the parameter instead of a string.

Once we address that, then all we need to know is if there is any special reason why the XMLDataProvider is in the synchronous mode.

Now, is there anything worth testing? XMLDataProvider is not a class we built, we expect it to work fine when we give a valid Uri.

So frankly, I would not waste my time writing test for this. In the future, if we see more logic creeping in, we might revisit this again.

like image 26
Naresh Jain Avatar answered Oct 08 '22 14:10

Naresh Jain