views:

155

answers:

5

I was reading and found this code as an aswer to a question

public List<T> LoadPlugin<T>(string directory)
{
    Type interfaceType = typeof(T);
    List<T> implementations = new List<T>();

    //TODO: perform checks to ensure type is valid

    foreach (var file in System.IO.Directory.GetFiles(directory))
    {
        //TODO: add proper file handling here and limit files to check
        //try/catch added in place of ensure files are not .dll
        try
        {
            foreach (var type in System.Reflection.Assembly.LoadFile(file).GetTypes())
            {
                if (interfaceType.IsAssignableFrom(type) && interfaceType != type)
                { 
                    //found class that implements interface
                    //TODO: perform additional checks to ensure any
                    //requirements not specified in interface
                    //ex: ensure type is a class, check for default constructor, etc
                    T instance = (T)Activator.CreateInstance(type);
                    implementations.Add(instance);
                }
            }
        }
        catch { }
    }

    return implementations;
}

and it got me wondering what the best way to unit test this code would be?

A: 

I would abstract out the detection of and loading of the dynamic assemblies in such a way that I could mock that part and load a test assembly as part of my unit tests.

Or, since you can specify a directory, simply construct, as part of your unit test code, a temporary directory in the TEMP directory of your computer, copy a single assembly from your unit test project to it, and ask the plugin system to scan that directory.

Lasse V. Karlsen
Is it not heresy to suggest using the file system in a unit test ;)
Sam Holder
It depends. Personally I don't view unit testing as religious as others, and will happily use bits and pieces others consider off limits. Though, I must say, mocking the file system away would be my first choice, if at all possible.
Lasse V. Karlsen
+4  A: 

By refactoring it this way:

public List<T> LoadPlugin<T>(Type[] types)
{
    Type interfaceType = typeof(T);
    List<T> implementations = new List<T>();

    //TODO: perform checks to ensure type is valid
    try
    {
        foreach (var type in types)
        {
            if (interfaceType.IsAssignableFrom(type) && interfaceType != type)
            { 
                //found class that implements interface
                //TODO: perform additional checks to ensure any
                //requirements not specified in interface
                //ex: ensure type is a class, check for default constructor, etc
                T instance = (T)Activator.CreateInstance(type);
                implementations.Add(instance);
            }
        }
    }
    catch { }

    return implementations;
}
Darin Dimitrov
A: 

You didn't say which unit testing framework you're using, so I'll assume Visual Studio's built-in capabilities here.

You would need to add the assemblies in question to the list of deployment items for them to get copied to the unit test working directory.

The problem I see is running the unit tests on all the different implementations. Testing all implementations in a single test would make it unclear which of the implementations are failing. You want to run the tests once for each implementation.

You could horribly abuse the data-driven test mechanism to do this however. You could insert a numbers 0...x-1 in some temporary database table after loading all the x implementations (in your Class_Initialize or whatever). Set up that table for each of your tests, and the test will be run x times, with a number as input data. Use that as an index to your implementations List (stored in a member variable) and run the test on that implementation.

Yes, very ugly.. and you lose the ability to do an actual data-driven test without adding more ugliness.

Thorarin
+1  A: 

I would extract the inner loop body into a method. I'd work towards having that method return the T instance, or null if it failed the test. Now I can write unit tests.

if (interfaceType.IsAssignableFrom(type) && interfaceType != type)
    return (T)Activator.CreateInstance(type);
else
    return null;

Now I can feed this new function types for which I expect a non-null instance to be returned, and types for which I expect a null return. All of the rest of the code seems to be using system calls, and I tend to trust those. But if you don't - then test those, in isolation. Test that GetFiles() gives you back the correct list of files; test that GetTypes() gives you the right types in a given file, etc.

Carl Manaster
+1  A: 

There are three unrelated things that are done in that one method (see SRP). Each of them should be separated to their own class, which implements some interface, so that you can mock them for better testability. The tree things are:

  1. Finding out the .dll files from the plugin directory.

  2. Loading the .dll and getting the types that it contains. This should be a one-liner that calls the API methods. You don't really need to test this (at least not in unit tests), because you can reasonably assume that the programming language's libraries work right.

  3. Creating instances of the plugin types.

When the algorithm is separated into these three parts, you can unit test parts 1 and 3 in isolation (although technically the tests for part 1 are not unit tests, because it touches the file system, unless C# has some way to mock the file system, like Java 7's NIO2 file system API should be mockable). You can also unit test the code that puts them all together, by mocking part 2.

Esko Luontola