Saturday, 11 March 2023

PHP / PHPUnit / TDD: unit testing abstract classes. Or not.

G'day:

One of my colleagues at work asked me about this, but it's a good topic to think about, so am gonna write about it here.

The question was pretty much as stated in the subject line there "if we have an abstract class… how do we go about unit testing that?". There's a coupla things to unpack here, one practical and one theoretical. I'll do the practical bit first.

Practical

I have got an abstract class (thanks to ChatGPT for writing the example code for me today, btw ;-))

abstract class Shape
{

    public function __construct(protected string $colour)
    {
    }

    public function getColour(): string
    {
        return $this->colour;
    }

    abstract public function getArea();
}

And this will eventually have implementing classes like circles and squares and what not. The abstraction conceit being that all shapes have an area, but the algorithms for defining said area vary from shape to shape. That's easy to understand and pretty ubiquitous in "here's an example of abstract classes in action" situations. For now we're just testing the abstract class, so I'm not worrying about the implementations yet.

We want to test that getColour returns the shape's colour.

For a non-abstract class, we could do this:

/** @testdox getColour returns the colour set by the constructor */
public function testGetColour()
{
    $testColour = "red";
    $shape = new Shape($testColour);

    $actualColour = $shape->getColour();

    $this->assertEquals($testColour, $actualColour);
}

Job done. Except Shape is an abstract class, so we get this instead:

Error: Cannot instantiate abstract class adamcameron\php8\Shapes\Shape
/var/www/tests/Unit/Shapes/ShapeTest.php:15

All is not lost. Obviously this is well-trod ground and PHPUnit already deals with this sort of thing: We can use a partial mock to create an implemetation class at runtime:

public function testGetColour()
{
    $green = "karariki";
    $shape = $this->getMockForAbstractClass(Shape::class, [$green]);

    $actualColour = $shape->getColour();

    $this->assertEquals($green, $actualColour);
}

getMockForAbstractClass()

The getMockForAbstractClass() method returns a mock object for an abstract class. All abstract methods of the given abstract class are mocked. This allows for testing the concrete methods of an abstract class.

Easy. There's also a mock-builder variant of this too:

public function testGetColourUsingMockBuilder()
{
    $blue = "kikorangi";
    $shape = $this
        ->getMockBuilder(Shape::class)
        ->setConstructorArgs([$blue])
        ->getMockForAbstractClass();

    $actualColour = $shape->getColour();

    $this->assertEquals($blue, $actualColour);
}

That's it, really. Original question answered.


Theory

The problem is that "how to test methods of an abstract class" begs the question. If we're doing TDD: how do we get to a point where we have an abstract class to test anyhow? It sounds to me like we're getting ahead of ourselves. I hasten to add the question from my team mate was a theoretical one: something that just popped into his head, and it was a good thing to know the answer for. But it's also good to reason the situation through from a TDD perspective.

Let's say the end of the story is "an abstract Shape class, and concrete classes for Square and Circle". But at the beginning of the story we had no classes at all, and no code. We had a requirement, which was probably along the lines of "we need to be able to get the colour of our Circle". Why is it a Circle and not a Shape? Because it's unlikely we're going to have a real world requirement that deals in abstract terms. It's more likely there'll be a concrete requirement to start with. But it's a good question: I'll think about that. We write our first test, for the Circle:

/** @testdox getColour returns the Circle's colour */
public function testGetColour()
{
    $orange = "karaka";
    $circle = new Circle($orange, 1);

    $actualColour = $circle->getColour();

    $this->assertEquals($orange, $actualColour);
}

And once we see this failing, we implement what we need to get it to pass, which would be along these lines:

class Circle
{
    public function __construct(protected string $colour)
    {
    }

    public function getColour(): string
    {
        return $this->colour;
    }
}

Cool.

The next requirement comes along, which is that we need to get the circle's area. Fine, more of the same sort of thing:

/** @testdocs getArea returns the Circle's area */
public function testGetArea()
{
    $circle = new Circle("NOT_TESTED", 2);

    $actualArea = $circle->getArea();

    $this->assertEquals(pi() * 4, $actualArea);
}

And implementation:

class Circle
{
    public function __construct(private readonly string $colour, private readonly float $radius)
    {
    }

    // …

    public function getArea(): float
    {
        return pi() * $this->radius ** 2;
    }
}

Now the twist comes in. The next requirement is "OK, sometimes the shapes will be squares instead of circles, but otherwise behave the same". Remember red-green-refactor here. After TDDing the Square's behaviour, we would end up with this implementation:

class Square
{

    public function __construct(private readonly string $colour, private readonly float $side)
    {
    }

    public function getColour(): string
    {
        return $this->colour;
    }

    public function getArea(): float
    {
        return $this->side ** 2;
    }
}

That's "red" & "green" done. Now for "refactor". Clearly we come to the conclusion that Circles and Squares are both Shapes; the colour behaviour is identical in both, but whilst both have the concept of "area", how it's derived is different, and the naming of the property that we use to derive the area - radius or side, respectively - also differs. This is when we decide we need our Shape abstract class. During refactoring. But the thing is during refactoring, the tests don't change. We extract the colour-handling into a base class, but leave the area handling in the implementation classes. We just make sure the base class says "I don't care how you do it, but you need to be able to answer the question 'what is your area'".

abstract class Shape
{

    public function __construct(protected string $colour)
    {
    }

    public function getColour(): string
    {
        return $this->colour;
    }

    abstract public function getArea();
}
class Circle extends Shape
{

    public function __construct(string $colour, protected float $radius)
    {
        parent::__construct($colour);
    }

    public function getArea(): float
    {
        return pi() * $this->radius ** 2;
    }
}
class Square extends Shape
{

    public function __construct(string $colour, protected float $side)
    {
        parent::__construct($colour);
    }

    public function getArea(): float
    {
        return $this->side ** 2;
    }
}

We're not done refactoring yet. And this comes back to the requirement to test methods of an abstarct class. Currently in CircleTest and SquareTest we have these:

public function testGetColour()
{
    $orange = "karaka";
    $circle = new Circle($orange, 1);

    $actualColour = $circle->getColour();

    $this->assertEquals($orange, $actualColour);
}
public function testGetColour()
{
    $red = "whero";
    $square = new Square($red, 1);

    $actualColour = $square->getColour();

    $this->assertEquals($red, $actualColour);
}

Other than the colours I've chosen to use and one creates a Circle and one creates a Square, these are identical. As they should be as they're testing behaviour of their base class. So it does make a kind of sense to de-dupe this stuff, and push the one test up into ShapeTest. The test is the one from the first section of this article.

However I'm split either way on this. That we decided to use an abstract Shape class here is - IMO - "implementation detail", and we don't generally directly test implementation detail. So if we refactor those two tests, we might be catering to implementation detail too directly. I'm unsure. Also these tests happen to be very simple, so I think in a way this refactoring would be a case of "bad DRY" (see my earlier article "DRY: don't repeat yourself"). Maybe if the tests were more complicated then there'd be a better case for de-duping the complexity ("good DRY"). I think in this case, either way would be A-OK. Personally I'm a pedant, and a bit "a place for everything, and everything in its place", so I'm gonna go ahead and do the refactoring, and have a unit test testing an abstract class.

The code for the tests is here: /tests/Unit/Shapes, and for the source code: /src/Shapes.

Righto.

--
Adam