Pass local variable to method in same class for testability?

Question

I have a method that accesses variables local to the class itself. I'm wondering if that's a design flaw because now my unit tests are dependent on those variables being set. Is it weird/wrong to pass those variables to the method, even though that method has direct access to those variables? (If the method accepts those variables as parameters, then the unit test can also pass its own variables to the method.)

For example, here are the variables local to the class:

private static List<string> _whitelistNames = GetWhitelistNamesFromConfig();
private static List<string> _blacklistNames = GetBlacklistNamesFromConfig();

And the method looks something like this:

    private static bool ThisProcessorHandlesThisFoo(Foo foo)
    {
        if (_whitelistNames.Count > 0)
        {
            // We're doing this instead of Contains() so we can ignore case sensitivity.
            bool found = ListContainsString(_whitelistNames, foo.Name, StringComparison.OrdinalIgnoreCase);

            // Some logging here

            return found;
        }

        if (_blacklistNames.Count > 0)
        {
            bool found = ListContainsString(_blacklistNames, foo.Name, StringComparison.OrdinalIgnoreCase);

            // Some logging

            return !found;
        }

        throw new InvalidOperationException("some message");
    }

In order to do what I'm suggesting, I would need to change the method signature to this:

private static bool ThisProcessorHandlesThisFoo(Foo foo, List<string> whitelistNames, List<string> blacklistNames)

Then the calling code (in the same class) would have to pass the local variables to the method. This would allow my test code to send its own parameters. Am I missing something? Is there a better way to do this? (It just seems odd to pass parameters that the method already has access to. But perhaps this is a good decoupling technique.)

score 1 · Accepted Answer · answered May 16 '14 at 16:37

You shouldn't have to worry about this.

Your unit tests should be testing the interface to your class (public methods and properties), and should not depend on any implementation detail (like a private class property). This allows the implementation to change without breaking other code (and hopefully without breaking existing tests).

Those private static fields should be initialized when the class is instantiated (according to your code, they have initializers). Are they initialized when it's instantiated for your unit test? They should be... It's a generally accepted idea that when the constructor for an object is finished running, the class should be in a usable state, and you're indicating that this isn't always the case.

This is an example of where something like Dependency Injection could be useful. You then pass these kinds of things to the constructor of your class (or through another means), and allow the part of the program that creates it (either your normal program or your unit test) to "inject" them into the class.

Looks like testing private methods is a good battle that have folks on both sides of the issue: http://stackoverflow.com/a/34586/279516 — Bob Horn, May 16 '14 at 16:52

score 1 · Answer 2 · answered May 16 '14 at 16:46

You should not test private methods, test the public ones in the way that tests cover all the class' code branches(including ones in private methods).
I'd recommend you to change method signature(to ThisProcessorHandlesThisFoo(Foo foo, List<string> whitelistNames, List<string> blacklistNames)) rather than use fields. As per my experience, such approach makes classes much easier to read and maintain.

Pass local variable to method in same class for testability?

2 Answers2