PHP Unit Testing – Method Per Test vs Data Provider

PHPphpunitunit testing

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!

Best Answer

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.

Related Topic