Unit tests are incredibly useful today. I think they are in most of the recently created projects. Unit tests are essential in enterprise applications with a lot of business logic because they are fast and can tell us right away if our implementation is correct. However, I often run into problems with good tests, although they are extremely useful. I'll give you some tips with examples on how to write good unit tests.
Content
- Test takes
- Names
- AAA Template
- Object mother
- Parameterized test
- Two schools of unit testing
- Mocks and plugs
- Three styles of unit testing
- Functional architecture and tests
- Observed Behavior and Implementation Details
- Unit of conduct
- Humble template
- Useless test
- Fragile test
- Test fixes
- Common Testing Antipatterns
- Don't chase full coverage
- Recommended books
Test takes
These are fake dependencies used in tests.
Plugs (Stub)
Dummy
The simulator is just a simple implementation that does nothing.
final class Mailer implements MailerInterface
{
public function send(Message $message): void
{
}
}
Fake
A fake is a simplified implementation that emulates the desired behavior.
final class InMemoryCustomerRepository implements CustomerRepositoryInterface
{
/**
* @var Customer[]
*/
private array $customers;
public function __construct()
{
$this->customers = [];
}
public function store(Customer $customer): void
{
$this->customers[(string) $customer->id()->id()] = $customer;
}
public function get(CustomerId $id): Customer
{
if (!isset($this->customers[(string) $id->id()])) {
throw new CustomerNotFoundException();
}
return $this->customers[(string) $id->id()];
}
public function findByEmail(Email $email): Customer
{
foreach ($this->customers as $customer) {
if ($customer->getEmail()->isEqual($email)) {
return $customer;
}
}
throw new CustomerNotFoundException();
}
}
Stub
A stub is the simplest implementation with the behavior specified in the code.
final class UniqueEmailSpecificationStub implements UniqueEmailSpecificationInterface
{
public function isUnique(Email $email): bool
{
return true;
}
}
$specificationStub = $this->createStub(UniqueEmailSpecificationInterface::class);
$specificationStub->method('isUnique')->willReturn(true);
Mock
Spy
Spy - An implementation to test a specific behavior.
final class Mailer implements MailerInterface
{
/**
* @var Message[]
*/
private array $messages;
public function __construct()
{
$this->messages = [];
}
public function send(Message $message): void
{
$this->messages[] = $message;
}
public function getCountOfSentMessages(): int
{
return count($this->messages);
}
}
Mock
Mock is a configured simulation to test calls to interacting objects.
$message = new Message('test@test.com', 'Test', 'Test test test');
$mailer = $this->createMock(MailerInterface::class);
$mailer
->expects($this->once())
->method('send')
->with($this->equalTo($message));
! Use a stub to validate inbound interactions and mock to validate outbound interactions. More on this in the Mocks and Stubs chapter .
Names
Poorly:
public function test(): void
{
$subscription = SubscriptionMother::new();
$subscription->activate();
self::assertSame(Status::activated(), $subscription->status());
}
Be explicit about what you are testing.
public function sut(): void
{
// sut = System under test
$sut = SubscriptionMother::new();
$sut->activate();
self::assertSame(Status::activated(), $sut->status());
}
Poorly:
public function it_throws_invalid_credentials_exception_when_sign_in_with_invalid_credentials(): void
{
}
public function testCreatingWithATooShortPasswordIsNotPossible(): void
{
}
public function testDeactivateASubscription(): void
{
}
Better:
- Using underscores improves readability.
- The name should describe the behavior, not the implementation.
- Use names without technical terms. They should be understandable to non-programmers.
public function sign_in_with_invalid_credentials_is_not_possible(): void
{
}
public function creating_with_a_too_short_password_is_not_possible(): void
{
}
public function deactivating_an_activated_subscription_is_valid(): void
{
}
public function deactivating_an_inactive_subscription_is_invalid(): void
{
}
Describing behavior is important when testing subject scenarios. If your code is utilitarian, then this is no longer so important.
Why is it important for non-programmers to be able to read unit tests? If the project has complex subject logic, then this logic should be obvious to everyone, and for this the tests should describe details without technical terms, so that you can speak with business representatives in the same language that is used in the tests. Remove the terms and all the code related to the domain, otherwise non-programmers will not be able to understand these tests. There is no need to write in the comments "returns null", "throws an exception", etc. Such information is not related to the subject area.
AAA Template
Also known as "Given, When, Then".
Highlight three stages in tests:
- Arrange : bring the system under test to the desired state. Prepare dependencies, arguments, and create a SUT.
- Act : extract the item under test.
- Assert : check the result, final state or interaction with other objects.
public function aaa_pattern_example_test(): void
{
//Arrange|Given
$sut = SubscriptionMother::new();
//Act|When
$sut->activate();
//Assert|Then
self::assertSame(Status::activated(), $sut->status());
}
Object mother
This template helps you create specific objects that can be used in multiple tests. This makes the "arrange" step concise and the entire test more readable.
final class SubscriptionMother
{
public static function new(): Subscription
{
return new Subscription();
}
public static function activated(): Subscription
{
$subscription = new Subscription();
$subscription->activate();
return $subscription;
}
public static function deactivated(): Subscription
{
$subscription = self::activated();
$subscription->deactivate();
return $subscription;
}
}
final class ExampleTest
{
public function example_test_with_activated_subscription(): void
{
$activatedSubscription = SubscriptionMother::activated();
// do something
// check something
}
public function example_test_with_deactivated_subscription(): void
{
$deactivatedSubscription = SubscriptionMother::deactivated();
// do something
// check something
}
}
Parameterized test
A parameterized test is a good way to test a multi-parameter SUT without repeating code. But these tests are less readable. To improve the situation a little, negative and positive examples need to be scattered across different tests.
final class ExampleTest extends TestCase
{
/**
* @test
* @dataProvider getInvalidEmails
*/
public function detects_an_invalid_email_address(string $email): void
{
$sut = new EmailValidator();
$result = $sut->isValid($email);
self::assertFalse($result);
}
/**
* @test
* @dataProvider getValidEmails
*/
public function detects_an_valid_email_address(string $email): void
{
$sut = new EmailValidator();
$result = $sut->isValid($email);
self::assertTrue($result);
}
public function getInvalidEmails(): array
{
return [
['test'],
['test@'],
['test@test'],
//...
];
}
public function getValidEmails(): array
{
return [
['test@test.com'],
['test123@test.com'],
['Test123@test.com'],
//...
];
}
}
Two schools of unit testing
Classic (Detroit School)
- A module is a unit of behavior that can consist of several interconnected classes.
- All tests should be isolated from each other. It should be possible to call them in parallel or in any order.
final class TestExample extends TestCase
{
/**
* @test
*/
public function suspending_an_subscription_with_can_always_suspend_policy_is_always_possible(): void
{
$canAlwaysSuspendPolicy = new CanAlwaysSuspendPolicy();
$sut = new Subscription();
$result = $sut->suspend($canAlwaysSuspendPolicy);
self::assertTrue($result);
self::assertSame(Status::suspend(), $sut->status());
}
}
Mokovaya (London School)
- A module is one class.
- The module must be isolated from interacting objects.
final class TestExample extends TestCase
{
/**
* @test
*/
public function suspending_an_subscription_with_can_always_suspend_policy_is_always_possible(): void
{
$canAlwaysSuspendPolicy = $this->createStub(SuspendingPolicyInterface::class);
$canAlwaysSuspendPolicy->method('suspend')->willReturn(true);
$sut = new Subscription();
$result = $sut->suspend($canAlwaysSuspendPolicy);
self::assertTrue($result);
self::assertSame(Status::suspend(), $sut->status());
}
}
The classic approach is better at avoiding brittle tests.
Dependencies
[TODO]
Mocks and plugs
Example:
final class NotificationService
{
public function __construct(
private MailerInterface $mailer,
private MessageRepositoryInterface $messageRepository
) {}
public function send(): void
{
$messages = $this->messageRepository->getAll();
foreach ($messages as $message) {
$this->mailer->send($message);
}
}
}
Poorly:
- Test interactions with stubs result in brittle tests.
final class TestExample extends TestCase
{
/**
* @test
*/
public function sends_all_notifications(): void
{
$message1 = new Message();
$message2 = new Message();
$messageRepository = $this->createMock(MessageRepositoryInterface::class);
$messageRepository->method('getAll')->willReturn([$message1, $message2]);
$mailer = $this->createMock(MailerInterface::class);
$sut = new NotificationService($mailer, $messageRepository);
$messageRepository->expects(self::once())->method('getAll');
$mailer->expects(self::exactly(2))->method('send')
->withConsecutive([self::equalTo($message1)], [self::equalTo($message2)]);
$sut->send();
}
}
Okay:
final class TestExample extends TestCase
{
/**
* @test
*/
public function sends_all_notifications(): void
{
$message1 = new Message();
$message2 = new Message();
$messageRepository = $this->createStub(MessageRepositoryInterface::class);
$messageRepository->method('getAll')->willReturn([$message1, $message2]);
$mailer = $this->createMock(MailerInterface::class);
$sut = new NotificationService($mailer, $messageRepository);
// Removed asserting interactions with the stub
$mailer->expects(self::exactly(2))->method('send')
->withConsecutive([self::equalTo($message1)], [self::equalTo($message2)]);
$sut->send();
}
}
Three styles of unit testing
Result
The best way:
- Best resistance to refactoring.
- Best Accuracy.
- Least of all maintenance efforts.
- Use this type of test if possible.
final class ExampleTest extends TestCase
{
/**
* @test
* @dataProvider getInvalidEmails
*/
public function detects_an_invalid_email_address(string $email): void
{
$sut = new EmailValidator();
$result = $sut->isValid($email);
self::assertFalse($result);
}
/**
* @test
* @dataProvider getValidEmails
*/
public function detects_an_valid_email_address(string $email): void
{
$sut = new EmailValidator();
$result = $sut->isValid($email);
self::assertTrue($result);
}
public function getInvalidEmails(): array
{
return [
['test'],
['test@'],
['test@test'],
//...
];
}
public function getValidEmails(): array
{
return [
['test@test.com'],
['test123@test.com'],
['Test123@test.com'],
//...
];
}
}
condition
Worse option:
- Poor refactoring resistance.
- Worse accuracy.
- More difficult to maintain.
final class ExampleTest extends TestCase
{
/**
* @test
*/
public function adding_an_item_to_cart(): void
{
$item = new CartItem('Product');
$sut = new Cart();
$sut->addItem($item);
self::assertSame(1, $sut->getCount());
self::assertSame($item, $sut->getItems()[0]);
}
}
Interaction
Worst case:
- Worst resistance to refactoring.
- Worst accuracy.
- The most difficult thing is to be accompanied.
final class ExampleTest extends TestCase
{
/**
* @test
*/
public function sends_all_notifications(): void
{
$message1 = new Message();
$message2 = new Message();
$messageRepository = $this->createStub(MessageRepositoryInterface::class);
$messageRepository->method('getAll')->willReturn([$message1, $message2]);
$mailer = $this->createMock(MailerInterface::class);
$sut = new NotificationService($mailer, $messageRepository);
$mailer->expects(self::exactly(2))->method('send')
->withConsecutive([self::equalTo($message1)], [self::equalTo($message2)]);
$sut->send();
}
}
Functional architecture and tests
Poorly:
final class NameService
{
public function __construct(private CacheStorageInterface $cacheStorage) {}
public function loadAll(): void
{
$namesCsv = array_map('str_getcsv', file(__DIR__.'/../names.csv'));
$names = [];
foreach ($namesCsv as $nameData) {
if (!isset($nameData[0], $nameData[1])) {
continue;
}
$names[] = new Name($nameData[0], new Gender($nameData[1]));
}
$this->cacheStorage->store('names', $names);
}
}
How do you test code like this? This can only be done with integration tests, because they directly use the infrastructure code related to the filesystem.
Good:
As with functional architecture, we need to separate code with side effects from code that only contains logic.
final class NameParser
{
/**
* @param array $namesData
* @return Name[]
*/
public function parse(array $namesData): array
{
$names = [];
foreach ($namesData as $nameData) {
if (!isset($nameData[0], $nameData[1])) {
continue;
}
$names[] = new Name($nameData[0], new Gender($nameData[1]));
}
return $names;
}
}
final class CsvNamesFileLoader
{
public function load(): array
{
return array_map('str_getcsv', file(__DIR__.'/../names.csv'));
}
}
final class ApplicationService
{
public function __construct(
private CsvNamesFileLoader $fileLoader,
private NameParser $parser,
private CacheStorageInterface $cacheStorage
) {}
public function loadNames(): void
{
$namesData = $this->fileLoader->load();
$names = $this->parser->parse($namesData);
$this->cacheStorage->store('names', $names);
}
}
final class ValidUnitExampleTest extends TestCase
{
/**
* @test
*/
public function parse_all_names(): void
{
$namesData = [
['John', 'M'],
['Lennon', 'U'],
['Sarah', 'W']
];
$sut = new NameParser();
$result = $sut->parse($namesData);
self::assertSame(
[
new Name('John', new Gender('M')),
new Name('Lennon', new Gender('U')),
new Name('Sarah', new Gender('W'))
],
$result
);
}
}
Observed Behavior and Implementation Details
Poorly:
final class ApplicationService
{
public function __construct(private SubscriptionRepositoryInterface $subscriptionRepository) {}
public function renewSubscription(int $subscriptionId): bool
{
$subscription = $this->subscriptionRepository->findById($subscriptionId);
if (!$subscription->getStatus()->isEqual(Status::expired())) {
return false;
}
$subscription->setStatus(Status::active());
$subscription->setModifiedAt(new \DateTimeImmutable());
return true;
}
}
final class Subscription
{
private Status $status;
private \DateTimeImmutable $modifiedAt;
public function __construct(Status $status, \DateTimeImmutable $modifiedAt)
{
$this->status = $status;
$this->modifiedAt = $modifiedAt;
}
public function getStatus(): Status
{
return $this->status;
}
public function setStatus(Status $status): void
{
$this->status = $status;
}
public function getModifiedAt(): \DateTimeImmutable
{
return $this->modifiedAt;
}
public function setModifiedAt(\DateTimeImmutable $modifiedAt): void
{
$this->modifiedAt = $modifiedAt;
}
}
final class InvalidTestExample extends TestCase
{
/**
* @test
*/
public function renew_an_expired_subscription_is_possible(): void
{
$modifiedAt = new \DateTimeImmutable();
$expiredSubscription = new Subscription(Status::expired(), $modifiedAt);
$repository = $this->createStub(SubscriptionRepositoryInterface::class);
$repository->method('findById')->willReturn($expiredSubscription);
$sut = new ApplicationService($repository);
$result = $sut->renewSubscription(1);
self::assertSame(Status::active(), $expiredSubscription->getStatus());
self::assertGreaterThan($modifiedAt, $expiredSubscription->getModifiedAt());
self::assertTrue($result);
}
/**
* @test
*/
public function renew_an_active_subscription_is_not_possible(): void
{
$modifiedAt = new \DateTimeImmutable();
$activeSubscription = new Subscription(Status::active(), $modifiedAt);
$repository = $this->createStub(SubscriptionRepositoryInterface::class);
$repository->method('findById')->willReturn($activeSubscription);
$sut = new ApplicationService($repository);
$result = $sut->renewSubscription(1);
self::assertSame($modifiedAt, $activeSubscription->getModifiedAt());
self::assertFalse($result);
}
}
Okay:
final class ApplicationService
{
public function __construct(private SubscriptionRepositoryInterface $subscriptionRepository) {}
public function renewSubscription(int $subscriptionId): bool
{
$subscription = $this->subscriptionRepository->findById($subscriptionId);
return $subscription->renew(new \DateTimeImmutable());
}
}
final class Subscription
{
private Status $status;
private \DateTimeImmutable $modifiedAt;
public function __construct(\DateTimeImmutable $modifiedAt)
{
$this->status = Status::new();
$this->modifiedAt = $modifiedAt;
}
public function renew(\DateTimeImmutable $modifiedAt): bool
{
if (!$this->status->isEqual(Status::expired())) {
return false;
}
$this->status = Status::active();
$this->modifiedAt = $modifiedAt;
return true;
}
public function active(\DateTimeImmutable $modifiedAt): void
{
//simplified
$this->status = Status::active();
$this->modifiedAt = $modifiedAt;
}
public function expire(\DateTimeImmutable $modifiedAt): void
{
//simplified
$this->status = Status::expired();
$this->modifiedAt = $modifiedAt;
}
public function isActive(): bool
{
return $this->status->isEqual(Status::active());
}
}
final class ValidTestExample extends TestCase
{
/**
* @test
*/
public function renew_an_expired_subscription_is_possible(): void
{
$expiredSubscription = SubscriptionMother::expired();
$repository = $this->createStub(SubscriptionRepositoryInterface::class);
$repository->method('findById')->willReturn($expiredSubscription);
$sut = new ApplicationService($repository);
$result = $sut->renewSubscription(1);
// skip checking modifiedAt as it's not a part of observable behavior. To check this value we
// would have to add a getter for modifiedAt, probably only for test purposes.
self::assertTrue($expiredSubscription->isActive());
self::assertTrue($result);
}
/**
* @test
*/
public function renew_an_active_subscription_is_not_possible(): void
{
$activeSubscription = SubscriptionMother::active();
$repository = $this->createStub(SubscriptionRepositoryInterface::class);
$repository->method('findById')->willReturn($activeSubscription);
$sut = new ApplicationService($repository);
$result = $sut->renewSubscription(1);
self::assertTrue($activeSubscription->isActive());
self::assertFalse($result);
}
}
The first subscription model has a poor architecture. Three methods need to be called to call one business operation. It is also not recommended to use getter methods to validate the operation. In this example, the change check is skipped
modifiedAt
. Possibly specifying a specific one
modifiedAt
during an operation
renew
can be tested by using an obsolescence business transaction. The
modifiedAt
receiver method is not required. Of course, there are situations in which it is very difficult to find a way to avoid using getter methods only for tests, but they should be avoided at all costs.
Unit of conduct
Poorly:
class CannotSuspendExpiredSubscriptionPolicy implements SuspendingPolicyInterface
{
public function suspend(Subscription $subscription, \DateTimeImmutable $at): bool
{
if ($subscription->isExpired()) {
return false;
}
return true;
}
}
class CannotSuspendExpiredSubscriptionPolicyTest extends TestCase
{
/**
* @test
*/
public function it_returns_true_when_a_subscription_is_expired(): void
{
$policy = new CannotSuspendExpiredSubscriptionPolicy();
$subscription = $this->createStub(Subscription::class);
$subscription->method('isExpired')->willReturn(true);
self::assertFalse($policy->suspend($subscription, new \DateTimeImmutable()));
}
/**
* @test
*/
public function it_returns_false_when_a_subscription_is_not_expired(): void
{
$policy = new CannotSuspendExpiredSubscriptionPolicy();
$subscription = $this->createStub(Subscription::class);
$subscription->method('isExpired')->willReturn(false);
self::assertTrue($policy->suspend($subscription, new \DateTimeImmutable()));
}
}
class CannotSuspendNewSubscriptionPolicy implements SuspendingPolicyInterface
{
public function suspend(Subscription $subscription, \DateTimeImmutable $at): bool
{
if ($subscription->isNew()) {
return false;
}
return true;
}
}
class CannotSuspendNewSubscriptionPolicyTest extends TestCase
{
/**
* @test
*/
public function it_returns_false_when_a_subscription_is_new(): void
{
$policy = new CannotSuspendNewSubscriptionPolicy();
$subscription = $this->createStub(Subscription::class);
$subscription->method('isNew')->willReturn(true);
self::assertFalse($policy->suspend($subscription, new \DateTimeImmutable()));
}
/**
* @test
*/
public function it_returns_true_when_a_subscription_is_not_new(): void
{
$policy = new CannotSuspendNewSubscriptionPolicy();
$subscription = $this->createStub(Subscription::class);
$subscription->method('isNew')->willReturn(false);
self::assertTrue($policy->suspend($subscription, new \DateTimeImmutable()));
}
}
class CanSuspendAfterOneMonthPolicy implements SuspendingPolicyInterface
{
public function suspend(Subscription $subscription, \DateTimeImmutable $at): bool
{
$oneMonthEarlierDate = \DateTime::createFromImmutable($at)->sub(new \DateInterval('P1M'));
return $subscription->isOlderThan(\DateTimeImmutable::createFromMutable($oneMonthEarlierDate));
}
}
class CanSuspendAfterOneMonthPolicyTest extends TestCase
{
/**
* @test
*/
public function it_returns_true_when_a_subscription_is_older_than_one_month(): void
{
$date = new \DateTimeImmutable('2021-01-29');
$policy = new CanSuspendAfterOneMonthPolicy();
$subscription = new Subscription(new \DateTimeImmutable('2020-12-28'));
self::assertTrue($policy->suspend($subscription, $date));
}
/**
* @test
*/
public function it_returns_false_when_a_subscription_is_not_older_than_one_month(): void
{
$date = new \DateTimeImmutable('2021-01-29');
$policy = new CanSuspendAfterOneMonthPolicy();
$subscription = new Subscription(new \DateTimeImmutable('2020-01-01'));
self::assertTrue($policy->suspend($subscription, $date));
}
}
class Status
{
private const EXPIRED = 'expired';
private const ACTIVE = 'active';
private const NEW = 'new';
private const SUSPENDED = 'suspended';
private string $status;
private function __construct(string $status)
{
$this->status = $status;
}
public static function expired(): self
{
return new self(self::EXPIRED);
}
public static function active(): self
{
return new self(self::ACTIVE);
}
public static function new(): self
{
return new self(self::NEW);
}
public static function suspended(): self
{
return new self(self::SUSPENDED);
}
public function isEqual(self $status): bool
{
return $this->status === $status->status;
}
}
class StatusTest extends TestCase
{
public function testEquals(): void
{
$status1 = Status::active();
$status2 = Status::active();
self::assertTrue($status1->isEqual($status2));
}
public function testNotEquals(): void
{
$status1 = Status::active();
$status2 = Status::expired();
self::assertFalse($status1->isEqual($status2));
}
}
class SubscriptionTest extends TestCase
{
/**
* @test
*/
public function suspending_a_subscription_is_possible_when_a_policy_returns_true(): void
{
$policy = $this->createMock(SuspendingPolicyInterface::class);
$policy->expects($this->once())->method('suspend')->willReturn(true);
$sut = new Subscription(new \DateTimeImmutable());
$result = $sut->suspend($policy, new \DateTimeImmutable());
self::assertTrue($result);
self::assertTrue($sut->isSuspended());
}
/**
* @test
*/
public function suspending_a_subscription_is_not_possible_when_a_policy_returns_false(): void
{
$policy = $this->createMock(SuspendingPolicyInterface::class);
$policy->expects($this->once())->method('suspend')->willReturn(false);
$sut = new Subscription(new \DateTimeImmutable());
$result = $sut->suspend($policy, new \DateTimeImmutable());
self::assertFalse($result);
self::assertFalse($sut->isSuspended());
}
/**
* @test
*/
public function it_returns_true_when_a_subscription_is_older_than_one_month(): void
{
$date = new \DateTimeImmutable();
$futureDate = $date->add(new \DateInterval('P1M'));
$sut = new Subscription($date);
self::assertTrue($sut->isOlderThan($futureDate));
}
/**
* @test
*/
public function it_returns_false_when_a_subscription_is_not_older_than_one_month(): void
{
$date = new \DateTimeImmutable();
$futureDate = $date->add(new \DateInterval('P1D'));
$sut = new Subscription($date);
self::assertTrue($sut->isOlderThan($futureDate));
}
}
Don't write 1: 1 code: one class, one test. This leads to brittle tests making refactoring difficult.
Okay:
final class CannotSuspendExpiredSubscriptionPolicy implements SuspendingPolicyInterface
{
public function suspend(Subscription $subscription, \DateTimeImmutable $at): bool
{
if ($subscription->isExpired()) {
return false;
}
return true;
}
}
final class CannotSuspendNewSubscriptionPolicy implements SuspendingPolicyInterface
{
public function suspend(Subscription $subscription, \DateTimeImmutable $at): bool
{
if ($subscription->isNew()) {
return false;
}
return true;
}
}
final class CanSuspendAfterOneMonthPolicy implements SuspendingPolicyInterface
{
public function suspend(Subscription $subscription, \DateTimeImmutable $at): bool
{
$oneMonthEarlierDate = \DateTime::createFromImmutable($at)->sub(new \DateInterval('P1M'));
return $subscription->isOlderThan(\DateTimeImmutable::createFromMutable($oneMonthEarlierDate));
}
}
final class Status
{
private const EXPIRED = 'expired';
private const ACTIVE = 'active';
private const NEW = 'new';
private const SUSPENDED = 'suspended';
private string $status;
private function __construct(string $status)
{
$this->status = $status;
}
public static function expired(): self
{
return new self(self::EXPIRED);
}
public static function active(): self
{
return new self(self::ACTIVE);
}
public static function new(): self
{
return new self(self::NEW);
}
public static function suspended(): self
{
return new self(self::SUSPENDED);
}
public function isEqual(self $status): bool
{
return $this->status === $status->status;
}
}
final class Subscription
{
private Status $status;
private \DateTimeImmutable $createdAt;
public function __construct(\DateTimeImmutable $createdAt)
{
$this->status = Status::new();
$this->createdAt = $createdAt;
}
public function suspend(SuspendingPolicyInterface $suspendingPolicy, \DateTimeImmutable $at): bool
{
$result = $suspendingPolicy->suspend($this, $at);
if ($result) {
$this->status = Status::suspended();
}
return $result;
}
public function isOlderThan(\DateTimeImmutable $date): bool
{
return $this->createdAt < $date;
}
public function activate(): void
{
$this->status = Status::active();
}
public function expire(): void
{
$this->status = Status::expired();
}
public function isExpired(): bool
{
return $this->status->isEqual(Status::expired());
}
public function isActive(): bool
{
return $this->status->isEqual(Status::active());
}
public function isNew(): bool
{
return $this->status->isEqual(Status::new());
}
public function isSuspended(): bool
{
return $this->status->isEqual(Status::suspended());
}
}
final class SubscriptionSuspendingTest extends TestCase
{
/**
* @test
*/
public function suspending_an_expired_subscription_with_cannot_suspend_expired_policy_is_not_possible(): void
{
$sut = new Subscription(new \DateTimeImmutable());
$sut->activate();
$sut->expire();
$result = $sut->suspend(new CannotSuspendExpiredSubscriptionPolicy(), new \DateTimeImmutable());
self::assertFalse($result);
}
/**
* @test
*/
public function suspending_a_new_subscription_with_cannot_suspend_new_policy_is_not_possible(): void
{
$sut = new Subscription(new \DateTimeImmutable());
$result = $sut->suspend(new CannotSuspendNewSubscriptionPolicy(), new \DateTimeImmutable());
self::assertFalse($result);
}
/**
* @test
*/
public function suspending_an_active_subscription_with_cannot_suspend_new_policy_is_possible(): void
{
$sut = new Subscription(new \DateTimeImmutable());
$sut->activate();
$result = $sut->suspend(new CannotSuspendNewSubscriptionPolicy(), new \DateTimeImmutable());
self::assertTrue($result);
}
/**
* @test
*/
public function suspending_an_active_subscription_with_cannot_suspend_expired_policy_is_possible(): void
{
$sut = new Subscription(new \DateTimeImmutable());
$sut->activate();
$result = $sut->suspend(new CannotSuspendExpiredSubscriptionPolicy(), new \DateTimeImmutable());
self::assertTrue($result);
}
/**
* @test
*/
public function suspending_an_subscription_before_a_one_month_is_not_possible(): void
{
$sut = new Subscription(new \DateTimeImmutable('2020-01-01'));
$result = $sut->suspend(new CanSuspendAfterOneMonthPolicy(), new \DateTimeImmutable('2020-01-10'));
self::assertFalse($result);
}
/**
* @test
*/
public function suspending_an_subscription_after_a_one_month_is_possible(): void
{
$sut = new Subscription(new \DateTimeImmutable('2020-01-01'));
$result = $sut->suspend(new CanSuspendAfterOneMonthPolicy(), new \DateTimeImmutable('2020-02-02'));
self::assertTrue($result);
}
}
Humble template
How to properly unit test this class?
class ApplicationService
{
public function __construct(
private OrderRepository $orderRepository,
private FormRepository $formRepository
) {}
public function changeFormStatus(int $orderId): void
{
$order = $this->orderRepository->getById($orderId);
$soapResponse = $this->getSoapClient()->getStatusByOrderId($orderId);
$form = $this->formRepository->getByOrderId($orderId);
$form->setStatus($soapResponse['status']);
$form->setModifiedAt(new \DateTimeImmutable());
if ($soapResponse['status'] === 'accepted') {
$order->setStatus('paid');
}
$this->formRepository->save($form);
$this->orderRepository->save($order);
}
private function getSoapClient(): \SoapClient
{
return new \SoapClient('https://legacy_system.pl/Soap/WebService', []);
}
}
It is necessary to split the overly complicated code into separate classes.
final class ApplicationService
{
public function __construct(
private OrderRepositoryInterface $orderRepository,
private FormRepositoryInterface $formRepository,
private FormApiInterface $formApi,
private ChangeFormStatusService $changeFormStatusService
) {}
public function changeFormStatus(int $orderId): void
{
$order = $this->orderRepository->getById($orderId);
$form = $this->formRepository->getByOrderId($orderId);
$status = $this->formApi->getStatusByOrderId($orderId);
$this->changeFormStatusService->changeStatus($order, $form, $status);
$this->formRepository->save($form);
$this->orderRepository->save($order);
}
}
final class ChangeFormStatusService
{
public function changeStatus(Order $order, Form $form, string $formStatus): void
{
$status = FormStatus::createFromString($formStatus);
$form->changeStatus($status);
if ($form->isAccepted()) {
$order->changeStatus(OrderStatus::paid());
}
}
}
final class ChangingFormStatusTest extends TestCase
{
/**
* @test
*/
public function changing_a_form_status_to_accepted_changes_an_order_status_to_paid(): void
{
$order = new Order();
$form = new Form();
$status = 'accepted';
$sut = new ChangeFormStatusService();
$sut->changeStatus($order, $form, $status);
self::assertTrue($form->isAccepted());
self::assertTrue($order->isPaid());
}
/**
* @test
*/
public function changing_a_form_status_to_refused_not_changes_an_order_status(): void
{
$order = new Order();
$form = new Form();
$status = 'new';
$sut = new ChangeFormStatusService();
$sut->changeStatus($order, $form, $status);
self::assertFalse($form->isAccepted());
self::assertFalse($order->isPaid());
}
}
However
ApplicationService
, it probably needs to be verified with a mock integration test
FormApiInterface
.
Useless test
Poorly:
final class Customer
{
public function __construct(private string $name) {}
public function getName(): string
{
return $this->name;
}
public function setName(string $name): void
{
$this->name = $name;
}
}
final class CustomerTest extends TestCase
{
public function testSetName(): void
{
$customer = new Customer('Jack');
$customer->setName('John');
self::assertSame('John', $customer->getName());
}
}
final class EventSubscriber
{
public static function getSubscribedEvents(): array
{
return ['event' => 'onEvent'];
}
public function onEvent(): void
{
}
}
final class EventSubscriberTest extends TestCase
{
public function testGetSubscribedEvents(): void
{
$result = EventSubscriber::getSubscribedEvents();
self::assertSame(['event' => 'onEvent'], $result);
}
}
Testing code that does not contain any complex logic is not only pointless, but also leads to fragile tests.
Fragile test
Poorly:
final class UserRepository
{
public function __construct(
private Connection $connection
) {}
public function getUserNameByEmail(string $email): ?array
{
return $this
->connection
->createQueryBuilder()
->from('user', 'u')
->where('u.email = :email')
->setParameter('email', $email)
->execute()
->fetch();
}
}
final class TestUserRepository extends TestCase
{
public function testGetUserNameByEmail(): void
{
$email = 'test@test.com';
$connection = $this->createMock(Connection::class);
$queryBuilder = $this->createMock(QueryBuilder::class);
$result = $this->createMock(ResultStatement::class);
$userRepository = new UserRepository($connection);
$connection
->expects($this->once())
->method('createQueryBuilder')
->willReturn($queryBuilder);
$queryBuilder
->expects($this->once())
->method('from')
->with('user', 'u')
->willReturn($queryBuilder);
$queryBuilder
->expects($this->once())
->method('where')
->with('u.email = :email')
->willReturn($queryBuilder);
$queryBuilder
->expects($this->once())
->method('setParameter')
->with('email', $email)
->willReturn($queryBuilder);
$queryBuilder
->expects($this->once())
->method('execute')
->willReturn($result);
$result
->expects($this->once())
->method('fetch')
->willReturn(['email' => $email]);
$result = $userRepository->getUserNameByEmail($email);
self::assertSame(['email' => $email], $result);
}
}
Testing repositories like this leads to brittle tests and makes refactoring difficult. Test repositories with integration tests.
Test fixes
Poorly:
final class InvalidTest extends TestCase
{
private ?Subscription $subscription;
public function setUp(): void
{
$this->subscription = new Subscription(new \DateTimeImmutable());
$this->subscription->activate();
}
/**
* @test
*/
public function suspending_an_active_subscription_with_cannot_suspend_new_policy_is_possible(): void
{
$result = $this->subscription->suspend(new CannotSuspendNewSubscriptionPolicy(), new \DateTimeImmutable());
self::assertTrue($result);
}
/**
* @test
*/
public function suspending_an_active_subscription_with_cannot_suspend_expired_policy_is_possible(): void
{
$result = $this->subscription->suspend(new CannotSuspendExpiredSubscriptionPolicy(), new \DateTimeImmutable());
self::assertTrue($result);
}
/**
* @test
*/
public function suspending_a_new_subscription_with_cannot_suspend_new_policy_is_not_possible(): void
{
// Here we need to create a new subscription, it is not possible to change $this->subscription to a new subscription
}
}
Okay:
final class ValidTest extends TestCase
{
/**
* @test
*/
public function suspending_an_active_subscription_with_cannot_suspend_new_policy_is_possible(): void
{
$sut = $this->createAnActiveSubscription();
$result = $sut->suspend(new CannotSuspendNewSubscriptionPolicy(), new \DateTimeImmutable());
self::assertTrue($result);
}
/**
* @test
*/
public function suspending_an_active_subscription_with_cannot_suspend_expired_policy_is_possible(): void
{
$sut = $this->createAnActiveSubscription();
$result = $sut->suspend(new CannotSuspendExpiredSubscriptionPolicy(), new \DateTimeImmutable());
self::assertTrue($result);
}
/**
* @test
*/
public function suspending_a_new_subscription_with_cannot_suspend_new_policy_is_not_possible(): void
{
$sut = $this->createANewSubscription();
$result = $sut->suspend(new CannotSuspendNewSubscriptionPolicy(), new \DateTimeImmutable());
self::assertFalse($result);
}
private function createANewSubscription(): Subscription
{
return new Subscription(new \DateTimeImmutable());
}
private function createAnActiveSubscription(): Subscription
{
$subscription = new Subscription(new \DateTimeImmutable());
$subscription->activate();
return $subscription;
}
}
- It is best to avoid using a common condition across multiple tests.
- To reuse items across multiple tests, use:
Common Testing Antipatterns
Disclosure of a private state
Poorly:
final class Customer
{
private CustomerType $type;
private DiscountCalculationPolicyInterface $discountCalculationPolicy;
public function __construct()
{
$this->type = CustomerType::NORMAL();
$this->discountCalculationPolicy = new NormalDiscountPolicy();
}
public function makeVip(): void
{
$this->type = CustomerType::VIP();
$this->discountCalculationPolicy = new VipDiscountPolicy();
}
public function getCustomerType(): CustomerType
{
return $this->type;
}
public function getPercentageDiscount(): int
{
return $this->discountCalculationPolicy->getPercentageDiscount();
}
}
final class InvalidTest extends TestCase
{
public function testMakeVip(): void
{
$sut = new Customer();
$sut->makeVip();
self::assertSame(CustomerType::VIP(), $sut->getCustomerType());
}
}
Okay:
final class Customer
{
private CustomerType $type;
private DiscountCalculationPolicyInterface $discountCalculationPolicy;
public function __construct()
{
$this->type = CustomerType::NORMAL();
$this->discountCalculationPolicy = new NormalDiscountPolicy();
}
public function makeVip(): void
{
$this->type = CustomerType::VIP();
$this->discountCalculationPolicy = new VipDiscountPolicy();
}
public function getPercentageDiscount(): int
{
return $this->discountCalculationPolicy->getPercentageDiscount();
}
}
final class ValidTest extends TestCase
{
/**
* @test
*/
public function a_vip_customer_has_a_25_percentage_discount(): void
{
$sut = new Customer();
$sut->makeVip();
self::assertSame(25, $sut->getPercentageDiscount());
}
}
Inserting additional production code (such as a getter method
getCustomerType()
) just for the sake of checking state in tests is bad practice. The state needs to be checked with another important subject value (in this case -
getPercentageDiscount()
). Of course, sometimes it is difficult to find another way to verify the operation, and we may find ourselves forced to add additional production code to check the correctness of the tests, but we must try to avoid this.
Leaked domain details
final class DiscountCalculator
{
public function calculate(int $isVipFromYears): int
{
Assert::greaterThanEq($isVipFromYears, 0);
return min(($isVipFromYears * 10) + 3, 80);
}
}
Poorly:
final class InvalidTest extends TestCase
{
/**
* @dataProvider discountDataProvider
*/
public function testCalculate(int $vipDaysFrom, int $expected): void
{
$sut = new DiscountCalculator();
self::assertSame($expected, $sut->calculate($vipDaysFrom));
}
public function discountDataProvider(): array
{
return [
[0, 0 * 10 + 3], //leaking domain details
[1, 1 * 10 + 3],
[5, 5 * 10 + 3],
[8, 80]
];
}
}
Okay:
final class ValidTest extends TestCase
{
/**
* @dataProvider discountDataProvider
*/
public function testCalculate(int $vipDaysFrom, int $expected): void
{
$sut = new DiscountCalculator();
self::assertSame($expected, $sut->calculate($vipDaysFrom));
}
public function discountDataProvider(): array
{
return [
[0, 3],
[1, 13],
[5, 53],
[8, 80]
];
}
}
Do not duplicate production logic in your tests. Check the results using the values ββwritten in the code.
Mocking specific classes
Poorly:
class DiscountCalculator
{
public function calculateInternalDiscount(int $isVipFromYears): int
{
Assert::greaterThanEq($isVipFromYears, 0);
return min(($isVipFromYears * 10) + 3, 80);
}
public function calculateAdditionalDiscountFromExternalSystem(): int
{
// get data from an external system to calculate a discount
return 5;
}
}
class OrderService
{
public function __construct(private DiscountCalculator $discountCalculator) {}
public function getTotalPriceWithDiscount(int $totalPrice, int $vipFromDays): int
{
$internalDiscount = $this->discountCalculator->calculateInternalDiscount($vipFromDays);
$externalDiscount = $this->discountCalculator->calculateAdditionalDiscountFromExternalSystem();
$discountSum = $internalDiscount + $externalDiscount;
return $totalPrice - (int) ceil(($totalPrice * $discountSum) / 100);
}
}
final class InvalidTest extends TestCase
{
/**
* @dataProvider orderDataProvider
*/
public function testGetTotalPriceWithDiscount(int $totalPrice, int $vipDaysFrom, int $expected): void
{
$discountCalculator = $this->createPartialMock(DiscountCalculator::class, ['calculateAdditionalDiscountFromExternalSystem']);
$discountCalculator->method('calculateAdditionalDiscountFromExternalSystem')->willReturn(5);
$sut = new OrderService($discountCalculator);
self::assertSame($expected, $sut->getTotalPriceWithDiscount($totalPrice, $vipDaysFrom));
}
public function orderDataProvider(): array
{
return [
[1000, 0, 920],
[500, 1, 410],
[644, 5, 270],
];
}
}
Okay:
interface ExternalDiscountCalculatorInterface
{
public function calculate(): int;
}
final class InternalDiscountCalculator
{
public function calculate(int $isVipFromYears): int
{
Assert::greaterThanEq($isVipFromYears, 0);
return min(($isVipFromYears * 10) + 3, 80);
}
}
final class OrderService
{
public function __construct(
private InternalDiscountCalculator $discountCalculator,
private ExternalDiscountCalculatorInterface $externalDiscountCalculator
) {}
public function getTotalPriceWithDiscount(int $totalPrice, int $vipFromDays): int
{
$internalDiscount = $this->discountCalculator->calculate($vipFromDays);
$externalDiscount = $this->externalDiscountCalculator->calculate();
$discountSum = $internalDiscount + $externalDiscount;
return $totalPrice - (int) ceil(($totalPrice * $discountSum) / 100);
}
}
final class ValidTest extends TestCase
{
/**
* @dataProvider orderDataProvider
*/
public function testGetTotalPriceWithDiscount(int $totalPrice, int $vipDaysFrom, int $expected): void
{
$externalDiscountCalculator = $this->createStub(ExternalDiscountCalculatorInterface::class);
$externalDiscountCalculator->method('calculate')->willReturn(5);
$sut = new OrderService(new InternalDiscountCalculator(), $externalDiscountCalculator);
self::assertSame($expected, $sut->getTotalPriceWithDiscount($totalPrice, $vipDaysFrom));
}
public function orderDataProvider(): array
{
return [
[1000, 0, 920],
[500, 1, 410],
[644, 5, 270],
];
}
}
Having to mock a particular class to replace part of its behavior means that the class is probably too complex and violates the principle of single responsibility.
Testing private methods
final class OrderItem
{
public function __construct(private int $total) {}
public function getTotal(): int
{
return $this->total;
}
}
final class Order
{
/**
* @param OrderItem[] $items
* @param int $transportCost
*/
public function __construct(private array $items, private int $transportCost) {}
public function getTotal(): int
{
return $this->getItemsTotal() + $this->transportCost;
}
private function getItemsTotal(): int
{
return array_reduce(
array_map(fn (OrderItem $item) => $item->getTotal(), $this->items),
fn (int $sum, int $total) => $sum += $total,
0
);
}
}
Poorly:
final class InvalidTest extends TestCase
{
/**
* @test
* @dataProvider ordersDataProvider
*/
public function get_total_returns_a_total_cost_of_a_whole_order(Order $order, int $expectedTotal): void
{
self::assertSame($expectedTotal, $order->getTotal());
}
/**
* @test
* @dataProvider orderItemsDataProvider
*/
public function get_items_total_returns_a_total_cost_of_all_items(Order $order, int $expectedTotal): void
{
self::assertSame($expectedTotal, $this->invokePrivateMethodGetItemsTotal($order));
}
public function ordersDataProvider(): array
{
return [
[new Order([new OrderItem(20), new OrderItem(20), new OrderItem(20)], 15), 75],
[new Order([new OrderItem(20), new OrderItem(30), new OrderItem(40)], 0), 90],
[new Order([new OrderItem(99), new OrderItem(99), new OrderItem(99)], 9), 306]
];
}
public function orderItemsDataProvider(): array
{
return [
[new Order([new OrderItem(20), new OrderItem(20), new OrderItem(20)], 15), 60],
[new Order([new OrderItem(20), new OrderItem(30), new OrderItem(40)], 0), 90],
[new Order([new OrderItem(99), new OrderItem(99), new OrderItem(99)], 9), 297]
];
}
private function invokePrivateMethodGetItemsTotal(Order &$order): int
{
$reflection = new \ReflectionClass(get_class($order));
$method = $reflection->getMethod('getItemsTotal');
$method->setAccessible(true);
return $method->invokeArgs($order, []);
}
}
Okay:
final class ValidTest extends TestCase
{
/**
* @test
* @dataProvider ordersDataProvider
*/
public function get_total_returns_a_total_cost_of_a_whole_order(Order $order, int $expectedTotal): void
{
self::assertSame($expectedTotal, $order->getTotal());
}
public function ordersDataProvider(): array
{
return [
[new Order([new OrderItem(20), new OrderItem(20), new OrderItem(20)], 15), 75],
[new Order([new OrderItem(20), new OrderItem(30), new OrderItem(40)], 0), 90],
[new Order([new OrderItem(99), new OrderItem(99), new OrderItem(99)], 9), 306]
];
}
}
Tests should only validate the public API.
Time as a fickle addiction
Time is a fickle dependency because of its indeterminacy. Each call produces a different result.
Poorly:
final class Clock
{
public static \DateTime|null $currentDateTime = null;
public static function getCurrentDateTime(): \DateTime
{
if (null === self::$currentDateTime) {
self::$currentDateTime = new \DateTime();
}
return self::$currentDateTime;
}
public static function set(\DateTime $dateTime): void
{
self::$currentDateTime = $dateTime;
}
public static function reset(): void
{
self::$currentDateTime = null;
}
}
final class Customer
{
private \DateTime $createdAt;
public function __construct()
{
$this->createdAt = Clock::getCurrentDateTime();
}
public function isVip(): bool
{
return $this->createdAt->diff(Clock::getCurrentDateTime())->y >= 1;
}
}
final class InvalidTest extends TestCase
{
/**
* @test
*/
public function a_customer_registered_more_than_a_one_year_ago_is_a_vip(): void
{
Clock::set(new \DateTime('2019-01-01'));
$sut = new Customer();
Clock::reset(); // you have to remember about resetting the shared state
self::assertTrue($sut->isVip());
}
/**
* @test
*/
public function a_customer_registered_less_than_a_one_year_ago_is_not_a_vip(): void
{
Clock::set((new \DateTime())->sub(new \DateInterval('P2M')));
$sut = new Customer();
Clock::reset(); // you have to remember about resetting the shared state
self::assertFalse($sut->isVip());
}
}
Okay:
interface ClockInterface
{
public function getCurrentTime(): \DateTimeImmutable;
}
final class Clock implements ClockInterface
{
private function __construct()
{
}
public static function create(): self
{
return new self();
}
public function getCurrentTime(): \DateTimeImmutable
{
return new \DateTimeImmutable();
}
}
final class FixedClock implements ClockInterface
{
private function __construct(private \DateTimeImmutable $fixedDate) {}
public static function create(\DateTimeImmutable $fixedDate): self
{
return new self($fixedDate);
}
public function getCurrentTime(): \DateTimeImmutable
{
return $this->fixedDate;
}
}
final class Customer
{
private \DateTimeImmutable $createdAt;
public function __construct(\DateTimeImmutable $createdAt)
{
$this->createdAt = $createdAt;
}
public function isVip(\DateTimeImmutable $currentDate): bool
{
return $this->createdAt->diff($currentDate)->y >= 1;
}
}
final class ValidTest extends TestCase
{
/**
* @test
*/
public function a_customer_registered_more_than_a_one_year_ago_is_a_vip(): void
{
$sut = new Customer(FixedClock::create(new \DateTimeImmutable('2019-01-01'))->getCurrentTime());
self::assertTrue($sut->isVip(FixedClock::create(new \DateTimeImmutable('2020-01-02'))->getCurrentTime()));
}
/**
* @test
*/
public function a_customer_registered_less_than_a_one_year_ago_is_not_a_vip(): void
{
$sut = new Customer(FixedClock::create(new \DateTimeImmutable('2019-01-01'))->getCurrentTime());
self::assertFalse($sut->isVip(FixedClock::create(new \DateTimeImmutable('2019-05-02'))->getCurrentTime()));
}
}
In domain code, you cannot directly generate time and random numbers. To test the behavior, you need deterministic results, so you need to inject these values ββinto the domain object, as in the example above.
Don't chase full coverage
Full coverage is not the goal, or even desirable, because otherwise the tests are likely to be very fragile and refactoring very difficult. Mutation testing provides more useful feedback on test quality. More details .
Recommended books
- Test Driven Development: By Example / Kent Beck is a classic.
- Unit Testing Principles, Practices, and Patterns / Vladimir Khorikov is the best book I know about testing.