Unit Testing: Method per test versus data provider

Question

I'm having a bit of a philosophical argument with one of my colleagues regarding the "right" way to do unit tests (in this case with PHPUnit). I'm of the opinion that you should write one test method in the unit test per test you want to run.

// Obviously a very contrived example!
class AdderTest extends TestCase
{
    private $object = NULL;

    protected function setup ()
    {
        $this -> object = new Adder ();
    }

    /**
     * Test that 2 + 2 = 4
     */
    public function testAddTwoTwoFour ()
    {
        $this -> assertEquals ($this -> object -> add (2, 2), 4);
    }

    /**
     * Test that 2 + 3 = 5
     */
    public function testAddTwoThreeFive ()
    {
        $this -> assertEquals ($this -> object -> add (2, 3), 5);
    }

    // and a bunch of other test cases
}

My colleague, however, thinks that it's better to use data providers.

class AdderTest extends TestCase
{
    private $object = NULL;

    protected function setup ()
    {
        $this -> object = new Adder ();
    }

    /**
     * Test add method
     *
     * @dataProvider addTestDataSource
     */
    public function testAdd ($expected, $a, $b)
    {
        $this -> assertEquals ($this -> object -> add ($a, $b), $expected);
    }

    private function addTestDataSource ()
    {
        return [
            [4, 2, 2], // Equivalent to testAddTwoTwoFour
            [5, 2, 3], // Equivalent to testAddTwoThreeFive
        ];
    }
}

My personal opinion is that the former is better because:

• One test method = 1 test, and every unit test should verify one and only one fact about the unit under test is correct.
• Failures are easy to spot because the name of the failed test method is displayed
• I'm of the opinion that unit tests should under no circumstances attempt to be at all "clever" on the grounds that a test failure means that either the code under test is wrong, or the test itself is wrong*, and the cleverer the test is, the more difficult it is to rule the test out as the source of the failure. You're supposed to be debugging the code, not its unit tests.
• In the case of PHPUnit it relies on "magic" to work (namely PHPUnit decoding the @dataProvider tag)
• If you want to do different types of assertions you have to write additional test methods anyway.

My colleague prefers his method because:

• One unit test per case violates Don't Repeat Yourself
• One unit test per case encourages copy and pasting, which is normally considered bad practice
• If the interface of the unit under test changes then you have to change a lot of test methods (which can be error-prone) whereas you only have to change one test method and one data providing method in the data provider case
• The unit test is a lot more concise and therefore easier to understand.

While I (naturally) think I'm correct, my colleague does raise some valid points. Do you consider my approach to be better, or would you prefer his? What is your reasoning for your choice, especially if that reasoning isn't in the lists I provided above?

*assuming no bugs in the unit test framework itself, of course!

Answer 1

It depends.

You should ask yourself: what property of the code is this particular test testing? What do I know when I know that this test is green?

I don't mean this as a philosophical exercise, but in a very practical sense. Take your Adder, for example. You can ask it "Can you add 2 and 2 correctly?" and "Can you add 2 and 3 correctly?" So you would write these tests:

public function knowsTwoPlusTwoIsFour ()
public function knowsTwoPlusThreeIsFive ()

But maybe you don't want to know that. Maybe what you really want to know is "Can you add two ordinay numbers correctly?" (I'll explain what "ordinary" means later) Now your test should use a data provider. The test would be

public function canAddOrdinaryNumbers()

and your data source should include the usual "normal" cases, i.e. adding zero, adding negative numbers, adding its complement:

[4, 2, 2],    // Duh
[-1, 2, -3],  // negative summand
[0, 0, 0],    // just to make sure
[2, 2, 0],    // neutral element
[0, 2, -2],   // inverse

Now your test tells you that Adder has no problems with normal numbers. That's basically one piece of information, hence it should be one test method.

What about numbers that are not ordinary?

public function canAddOneToNaN ()
public function knowsWhatInfPlusNegInfIs ()
public function croaksOnFLoatingPointInput ()
public function whatIsOnePlusSqrtOfMinusOneAnyway ()

I'll leave the implementation of these test cases as an exercise to the reader, and would like to focus on this point: All unit test methods in a class should convey about the same amount of information. That is a subjective measure, of course, but I think it's a good rule: look at the tests and ask yourself: are some of these tests only telling me stuff others already did before? Do some tests seem to have more meaning than others? If so, then you might want to do some refactoring.

Answer 2

Many of your points come from weaknesses of your testing framework. If we look at Spock, it addresses a number of your concerns. Let's see how to write your test in Spock

@Unroll
final "add #a and #b gives #expected"() {
    expect:
        add(a, b) == expected
    where:
        a  | b  || expected
        2  | 2  || 4
        2  | 3  || 5
        // etc.
}

One test method = 1 test, and every unit test should verify one and only one fact about the unit under test is correct.

"One and only one fact" can be a nebulous concept. Data driven testing, like property based testing, can be used to get at higher-level facts that no single test case can really evince: e.g. add is commutative / associative / has an identity at 0.

Failures are easy to spot because the name of the failed test method is displayed

Spock allows you to make your failed test name depend on the value. Even without this feature, a good assert message will make clear what values failed.

I'm of the opinion that unit tests should under no circumstances attempt to be at all "clever"

A data driven test need be no more clever than a for-each loop. Indeed, without testing framework support you can implement data-driven testing with a simple loop. And you can always fall back on a for-each loop if the "magic" of the framework is too clever. Nevertheless, that magic as in Spock can give highly readable tests.

If you want to do different types of assertions you have to write additional test methods anyway.

Yes, but I don't see this as a problem. As you say, you have to write additional methods regardless and the nice thing about data-driven testing is that you can extract your data and feed it into multiple methods.

Generally, I think your co-workers points of "DRY/eliminating copy-paste" as well as readability are big wins of data driven testing. Another big win for data driven testing is that it makes adding a test case very cheap.