Don't test constructors

Matthias Noback

@ediar asked me on Twitter if I still think a constructor should not be tested. It depends on the type of object you’re working with, so I think it’ll be useful to elaborate here.

Would you test the constructor of a service that just gets some dependencies injected? No. You’ll test the behavior of the service by calling one of its public methods. The injected dependencies are collaborating services and the service as a whole won’t work if anything went wrong in the constructor.

Would you test the constructor of a DTO? No. A DTO is by definition a simple object with no behavior, which allows you to pass data in a structured way. The data will be copied to the DTO and used in some other place, so when something goes wrong with assigning values to properties, you will know it when you test the service that uses the DTO.

Would you test the constructor of an entity? Yes, but in very specific ways, none of which is this one:

public function testConstructor(): void
{
    $user = new User('Name', 'M');
    
    self::assertSame('Name', $user->getName());
    self::assertSame('M', $user->getGender());
}

(“Why do we need to know the user’s gender?” Great question. We don’t. We’ll get back to it.)

I like to call a test like this: “you can take out what you put in”, or: “it assigns the values to the right properties”. It’s a bad test and I’ll explain why.

Common constructor problems will be caught by your static analyzer

If you want to make sure the provided constructor arguments end up being assigned to the right properties, then just rely on static analyzers like PHPStan or Psalm (or even PhpStorm). They will point out that something is wrong, e.g. when you assign a value of the wrong type, or you rely on it to be non-optional, and so on. You don’t need to test these low-level things about your code.

Exposing state breaks encapsulation

If you still want to verify that a constructor assigns the right values to the right properties, then you either have to use reflection to peek into the private properties of the object, or you have to add public getters to the object. Both options break encapsulation. The object can no longer keep its internal data structures and inner workings to itself.

If you choose reflection, the test becomes fragile: it breaks whenever you change anything about the properties.

If you choose getters, you end up with methods that are initially only introduced for testing purposes, but eventually clients will start using those methods and relying on them, even if they weren’t meant to have access to all those values. These getters are not intention-revealing methods anyway, which causes the entity to have a very confusing API with too many public methods that don’t serve any real use case, except testing.

The test doesn’t explain why you need the property assignments

Even if you would allow breaking encapsulation by exposing too much state, the constructor test is still a bad test. It doesn’t answer why you need those property assignments. Do you need them so the getter can return the assigned value? That would be a circular argument. Take the constructor test away and you don’t need them anymore.

Asking “why” means we have to look at a higher abstraction level. Why do you need that property assignment?

  • Because that property will be mapped to a database column and you need that data to be in your database.

This answer needs some further soul searching. Why do you need this value to be in your database?

  1. Because you want to select the value and show it on some page.
  2. Because a legacy system relies on that particular value to be in that particular column.
  3. Because one of the stakeholders has told you to collect and save that data.

Replace the constructor unit test with some higher-level test

The rather unhelpful unit test that we have for the constructor doesn’t mention any of these reasons. Which is how knowledge about a system gets lost over time. Codifying all the reasons as tests, test descriptions, or possibly just comments in tests, would be a great way to preserve knowledge. It also helps us refrain from testing the implementation details of a class. Instead, we’ll be focussing on the behavior of the system as a whole, as we should. For example, providing a high-level test for reason 1:

Given the user registers themself as "Matthias"
When they login
Then they should see "Welcome back, Matthias!"

Providing a test for reason 2 could be more low-level, given that’s it’s mostly a technical concern:

public function testNameEndsUpInTheRightColumn(): void
{
    $user = new User('Name');
    $this->userRepository->save($user);
    
    /*
     * Our legacy system needs the user's name to be in the USname column
     * of the LegacyUsers table because the SOAP API still reads the user
     * name from that column and we can't make a new release at the moment.
     */
    self::assertSame(
        'Name',
        $this->dbConnection
            ->execQuery('SELECT USname FROM LegacyUsers')
            ->fetchColumn(0)
    );
}

Specifying and testing the “why” instead of the “what” of a system even allows developers to question the reasons provided by those tests. Getting back to the question “Why do we need to know the user’s gender?” We can talk about it with our stakeholders, and most likely we don’t really need to. So we can get rid of it. Do we need to know the user’s name? Yes, we need some way to address them, e.g. on the login page, as we just saw.

What if I just want to protect some domain invariant?

Besides needing an assigned value to end up in your database, another good reason for assigning a value to a property is when you later need it to protect a domain invariant. That is, you want to make sure the object can only be used in the right way, providing the right data. As an example, you may want to suspend the user’s account. That only makes sense if the account wasn’t suspended already. So then you want to write this test:

public function testUserAccountCanNotBeSuspendedIfItAlreadyIs(): void
{
    $suspendedUser = new User('A name');
    $suspendedUser->suspend();
    
    $this->expectedException(CouldNotSuspendUser::class);
    
    $suspendedUser->suspend();
}

At this point, the test “forces” you to keep track of the is-suspended state somehow, and of course, you’d do that with an object property.

With test-driven development the idea would be that you write a test first, see it fail, then let the test force you to write just enough production code to make it pass. This way we can ask: why this production code?

  • Because of this test.

Ah, great. You started with a test, then wrote some production code. However, the why needs to be addressed for the test as well.

Why this test?

Yes, why this test?

  • Because I always write this type of test for this type of class (that’s me!).
  • Because I couldn’t think of anything else to test (me too).
  • Because I didn’t really write it first, I just needed to have some test for this class so I wrote a “constructor test” (ah, me too).

Okay, that’s cynical. And true; I have done all of this, and if I don’t think about it, I do it all the time. I really hope the real answer is and more often becomes: because the use case requires it. And then we’re back to explaining why that is the case, and to codify that knowledge in a higher-level test, and not in a unit test. Because after all, all these classes are just implementation details of the system as a whole. What use cases this system has to offer to its users and other stakeholders is what matters. Not if you assign the right values to the right properties.