tags:

views:

76

answers:

3

It seems as though the general consensus of the testing community is to not test private methods. Instead, you should test private methods by testing the public methods that invoke them. However, something just doesn't feel right to me. Let's take this method for example:

/**
 * Returns the base name of the output generator class. If the class is named
 * Reno_OutputGenerator_HTML, this would return "HTML".
 *
 * @return string
 */
protected function getName()
{
    $class = get_class($this);
    $matches = array();

    if (preg_match('/^Reno_OutputGenerator_(.+)$', $class, $matches))
    {
        return $matches[1];
    }
    else
    {
        throw new Reno_OutputGenerator_Exception('Class name must follow the format of Reno_OutputGenerator_<name>.');
    }
}

This particular function is used in a couple of places in my class. I'd like to test both branches of the if statement in this function, which would mean for each public function I'd have to test those 2 situations plus whatever else the public method itself does.

This is what feels weird for me. If I'm testing to see if getName() throws an Exception when a certain specific condition is met, then that means that I have to know implementation details of the private method. If I have to know that, then why shouldn't I just extend the class, make the method public, and test it that way?

(BTW: If you're wondering why such a weird method exists, this is used to automagically figure out what directory this class's template files are stored in).

+2  A: 

The way I understand unit testing, this is exactly the kind of testing I would want to do. I have always looked at unit testing as white-box testing; if there's a branch point in my code, that means I need two unit tests to address it. I think the worst case I ever wound up with was a single method with 32 permutations.

The challenge with unit-testing is that if you don't explore all the edge cases by examining your code and figuring out all the different paths, you wind up missing one or more cases and possibly introducing subtle bugs into your application.

So, no, I don't see what you're proposing as weird. The method can stay internal, and you can add an extra test case - you probably only need the one with the exception, right?

Alternatively, you could refactor the functionality into a separate object that takes your generator object and returns its name (based on the algorithm above). That would justify separating the tests, because you'd have a name-extractor object, and the output generator implementations. I'm still not sure that this would save you a lot, because you'd still have to test the output generators to make sure they were using the name extractor correctly, but it would separate your functional and testing concerns.

mlschechter
A: 

I too sometimes struggle with the notion of not testing private methods, because the code may contain logic that should be tested directly, as opposed to indirectly. I program in .NET languages, and to get around this I sometimes make methods internal instead of private when no better alternative exists. Then I allow the assembly to grant permission to the testing assembly to have access to internals.

That said, in your example why not just change the original method to be public from the start? If the method is important enough to want to test directly and requires knowledge of how it works why not make it part of the class's contract? This way it is less likely to change compared to a private method where it's implied that the implementation doesn't matter nearly as much as the result of the operation.

Matt Spinelli
A: 

You could also test this function by deriving from the class in your testclass like this:

namespace TheProject
{
    public class ClassUnderTest
    {
        protected string GetName()
        {
            return "The name";
        }
    }
}

namespace TestProject
{
    [TestClass]
    public class TheTest:TheProject.ClassUnderTest
    {
        [TestMethod]
        public void TestGetName()
        {
            string expected = "The name";
            string actual = GetName();
            Assert.AreEqual(expected, actual);
        }
    }
}

That way you keep your method private and you don't need to refactor your code to another class.

slamidtfyn