phpunit icon indicating copy to clipboard operation
phpunit copied to clipboard

`TestCase` should not extend `Assert`

Open BafS opened this issue 2 years ago • 2 comments

TestCase is currently 2276 lines long and contains 129 functions, we can argue that this class concentrates too much responsibilities and is a god class. Unfortunately this class extends Assert which is 2293 lines long and contains 175 functions.

  • Any phpunit test must extends the TestCase (thus Assert) and inherit from too much logic/functions
  • It's not possible to remove assertions from userland tests
    • Personally and where I work we always use the FQCN for assertions (for example Assert::assertTrue(...)) to use composition. Unfortunately to enforce this convention you need to rely on external tools.
  • To add new assertions, people often add them is some userland abstract test classes to keep the same syntax self::assertXxx(). This bloat classes even more, I saw it many times in the wild. If Assert was not available by default it would help to promote usage of composition over inheritance and create its own "Assert".

My question is: could we remove the Assert inheritance from TestCase? What is the advantage of extending Assert? Writing self:: instead of Assert:: doesn't seem to be a good reason.

Proposal

  • Move the assertions in a trait, AssertTrait
  • Remove inheritance of Assert in TestCase but "use" AssertTrait to not have any BC in the next major
  • Promote usage of Assert::

Because there is a trait, it would be easy to have our own Assert by using use AssertTrait and add methods.

class MyAssert {
    use PhpUnitAssert;

   // custom methods...
}

In a future version we could imagine to remove the trait from TestCase. By doing so, devs would be free to either use AssertTrait to keep the "legacy" syntax or to use composition (Assert::).

class MyTest extends TestCase {
    use PhpUnitAssert;

    // We can use "self::assertXxx()"
}

or

class MyTest extends TestCase {
    // We can use "Assert::assertXxx()"
}

BafS avatar Jun 29 '23 05:06 BafS

In the long run: yes.

sebastianbergmann avatar Jun 29 '23 05:06 sebastianbergmann

Maybe offer to write Test classes without "extends TestCase".

Example:

<?php declare(strict_types=1);

use PHPUnit\Framework\Assert;
use PHPUnit\Framework\TestCaseInterface;

final class GreeterTest
{
    private static $dbh;
    
    private TestCaseInterface $testCase;

    public function __construct(TestCaseInterface $testCase) // setUp()
    {
        $this->testCase = $testCase;
    }

    public function __destruct(): void // tearDown()
    {
    }

    public static function setUpBeforeClass(): void
    {
        self::$dbh = new PDO('sqlite::memory:');
    }

    public static function tearDownAfterClass(): void
    {
        self::$dbh = null;
    }

    public function testGreetsWithName(): void
    {
        $greeter = new Greeter;

        $greeting = $greeter->greet('Alice');

        $this->testCase->expectException(Exception::class);
        
        $this->testCase->assertSame('Hello, Alice!', $greeting); // alternative 1
        Assert::assertSame('Hello, Alice!', $greeting); // alternative 2
    }
}

thomas-0816 avatar Jul 05 '23 23:07 thomas-0816