Unit test; correct approach?

Question

At work I've been asked to increase the amount of code coverage we have in one of our software products. I've never done unit testing before and have read a number of tutorials online; these have been helpful as a starting point but all follow the same pattern - they are testing very simple methods/classes like a calculator or a bank account.

I've found a simple method in our code to start, but the issue is it's still much more complicated than the examples I've read about and I'm unsure where to start. Here's the method:

public static void moveFiles {
    string rootDir = ConfigurationManager.AppSettings["RootLoc"];
    string dropboxLoc = ConfigurationManager.AppSettings["DropBoxLocation"];
    DirectoryInfo dropbox = new DirectoryInfo(dropboxLoc);
    folders = dropbox.GetDirectories("*", SearchOption.AllDirectories);

    string path = "";
    string prevPath = "";

    foreach (DirectoryInfo di in folders)
    {
        FileInfo[] files = di.GetFiles();
        foreach (FileInfo fi in files)
        {
            prevPath = fi.FullName;
            string[] p = prevPath.Split(new string[] { dropbox.Name }, StringSplitOptions.None);
            path = rootDir + p[1];
            fi.MoveTo(path);
        }

    }
}

I have created a small test method for this:

[TestMethod]
public void GetDirectories_ValidLocation_SetsDropboxLocation()
{
        string dropboxLoc = ConfigurationManager.AppSettings["DropBoxLocation"];
        DirectoryInfo dropbox = new DirectoryInfo(dropboxLoc);
        Assert.IsTrue(dropbox.Exists);
}

Is this along the right lines of what should be tested? Or am I looking at this the wrong way?

I might be overlooking something, but the method you want to test is never called in your test... — Heinzi, Dec 18 '15 at 16:42
Your code appears to get every file from every directory at a given location and move them to a different location. The tests you write have to confirm that behavior. Your current test confirms the starting location exists, which is nice but is 0.1% of your problem. — Anthony Pegram, Dec 18 '15 at 16:44
@Heinzi This is where I'm getting confused; in a lot of the simple examples that is exactly what happens. However when reading up on complex code it wasn't recommended to call complex methods since you are no longer focusing on a single unit of work, but rather lots of units of work within a method. So I was unsure if my attempt was the correct approach :) — Novastorm, Dec 18 '15 at 16:45
This is where you discover the need to refactor the code, many times. You find your code gets the job done, but it's hard to test, so you break it into smaller pieces, often with dependencies that you can manage and replace. (This is also where some people/organizations decide to skip unit testing.) — Anthony Pegram, Dec 18 '15 at 16:47
Proper unit testing of such method would involve building file system Mock and adding dependency on it (instead of directly calling DirectoryInfo / Move). This is really much bigger undertaking than I'd recommend for initial tests.... [Heinzi's answer](http://stackoverflow.com/a/34360232/477420) is somewhat workaround for that, but some may not call such approach "unit test" but rather integration test. — Alexei Levenkov, Dec 18 '15 at 16:48
Don't want to nitpick here, just a sidenote that this will be an [integration test](http://stackoverflow.com/questions/20265369/how-to-do-integration-testing-in-net-with-real-files) rather than a unit test, since you are not testing your code in complete isolation. — kayess, Dec 18 '15 at 16:48
I would recommend going through a book like 'The Art Of Unit Testing' by Osherove -- I for one am all for unit testing, but if it's not done correctly it can hurt in the long run with hard to maintain tests. The problem with this being an integration test is that the code could be correct and the test could still potentially fail due to the file system (an element outside of the tests control). The previous comment about using a mocking framework (like Moq) would be one way to make this unit test more repeatable although I agree it would be an undertaking. — PerryC, Dec 18 '15 at 17:29

Heinzi · Accepted Answer · 2015-12-18T17:05:40.990

Your unit tests should test that the method does what it is supposed to do. Your moveFiles method is supposed to move files, so you need to test whether it moves files.

This is how it could be done:

In your test project's app.config, set RootLoc and DropBoxLocation to test directories.
In your test method:

a. Set up your test environment by creating some test files in RootLoc. Ensure that DropBoxLocation is empty.

b. Call your moveFiles method.

c. Assert that DropBoxLocation contains the files you expect.

d. Clean up (i.e. remove the test files).

Ideally, steps a and d could be done in TestInitialize and TestCleanup methods, respectively. Also, you might want to split your method into two methods:

public static void moveFiles(string source, string destination)
{
    ...
}

public static void moveFiles()
{
    moveFiles(ConfigurationManager.AppSettings["RootLoc"],
              ConfigurationManager.AppSettings["DropBoxLocation"]);
}

This would allow you to test your moveFiles method in isolation, without the dependency on an external config file. This is a very simple example of dependency injection - you might want to read up on that if your methods turn out to be hard to test.

Fantastic answer! Thank you for the heads up on dependency injection, I have a feeling there's going to be a lot of that in this piece of software. — Novastorm, Dec 18 '15 at 16:55

Unit test; correct approach?

1 Answers1