Basic TDD in Your New PHP Package

PHP
2

I have technical feedback on the article.

First, regarding your additions to composer.json’s autoload-dev section, a minor
point: you should append a “/” to the directory name. This normalization ensures
it doesn’t attempt to match against a file prefixed with the value presented.

Second, for your first set of tests, you would be better served using data
providers and the setExpectedException method:

public function invalidTokens()
{
    return [
        'empty'        => [ '' ],
        'a'            => [ 'a' ],
        'ab'           => [ 'ab' ],
        'abc'          => [ 'abc' ],
        'digit'        => [ 1 ],
        'double-digit' => [ 12 ],
        'triple-digit' => [ 123 ],
        'bool'         => [ true ],
        'array'        => [ ['token'] ],
    ];
}

public function validTokens()
{
    return [
        'token'      => [ 'token' ],
        'short-hash' => [ '123456789' ],
        'full-hash'  => [ 'akrwejhtn983z420qrzc8397r4' ],
    ];
}

/**
 * @dataProvider invalidTokens
 */
public function testSetTokenRaisesExceptionOnInvalidToken($token)
{
    $this->setExpectedException('InvalidArgumentException');
    Diffbot::setToken($token);
}

/**
 * @dataProvider validTokens
 */
public function testSetTokenSucceedsOnValidToken($token)
{
    Diffbot::setToken($token);
}

(Ideally, you will also provide an assertion on the latter; just calling the
method without an error is not enough.)

Data providers are a built-in mechanism of PHPUnit designed specifically for the
scenario of running the same tests over different values. Using keys helps
identify which specific invocation caused a failure. (This same approach should
be used in your later example demonstrating testing an abstract class.)

The second tests, which test instantiation, should be split into three, one for
each scenario.

public function testInstantiationWithNoGlobalTokenAndNoArgumentRaisesAnException()
{
    $this->setExpectedException('DiffbotException');
    new Diffbot();
}

public function testInstantiationWithGlobalTokenAndNoArgumentSucceeds()
{
    Diffbot::setToken('token');
    $bot = new Diffbot();
    $this->assertInstanceOf('Swader\Diffbot\Diffbot', $bot);
}

public function testInstantiationWithNoGlobalTokenButWithArgumentSucceeds()
{
    $bot = new Diffbot('token');
    $this->assertInstanceOf('Swader\Diffbot\Diffbot', $bot);
}

The above ensures there are no side effects present in the different scenarios,
clearly delineats the different scenarios via discrete test cases, and leverages
PHPUnit’s exception handling strategies.

Next, you suggest that developers use the @runTestsInSeparateProcess flag for
their tests, vs using setup or teardown logic. This is a terrible
recommendation; it substantially slows down test runs (and tests should execute
quickly!). Additionally, your stated reason (resetting static members) is an
indication of poor design; if static state is mutable, you should likely be
using concrete instances with instance methods instead.

1 Like