maciej łebkowski
Maciej Łebkowski

Unit tests code style

in Professional

We’re investing a lot of time in testing recently. We have both behat and unit tests, and I always liked the latter more. And lately, I think I found a sweet spot regarding the code style and overall architecture of our tests.

High-end vs low-end units

There are different kinds of classes. Those rather on the lower end are simple and small. Take one input and return a simple output. They don’t rely on too many dependencies, don’t have side effects, etc.

On the other end are higher-level classes. They are closer to representing business processes: having tons of dependencies, side effects. So testing them becomes different. Rather than slapping a @dataProvider to feed the SUT’s methods, you replace its dependencies for different ones: one that returns something, one that returns null, one that throws, etc.

Aren’t those integration tests already?

Let me give a little backstory. Sometimes I’m just in a crunch. I write. I don’t think about architecture, either because I have no good idea what will be a good solution in a given moment, or I just want to save it for last. So I end up with this 300 lines, 8 dependencies monstrosity. I don’t even want to count the complexity level on that one.

The first natural step is to level up the abstraction and hide implementation details. Psst, I just mean splitting the class into a bunch of private methods. Then, those methods are prime candidates to be extracted to separate classes. My language doesn’t support package-private classes, but let’s treat them like ones.

So I end up splitting my 8 deps and hundred lines of code into a bunch of smaller classes. They still form the same unit, but each of them is small enough (in relative terms) to pass through review. This means that no interfaces are separating them. The smaller chunks are not meant to be replaced. There are no interfaces there, and the classes are final (as per our code style).

This means I need to test the main class with all those deps. So it would seem like this is an integration test, but it actually isn’t. From the public interface standpoint, this is still a single responsibility, single public interface. The code just got split for readability (and to test edge cases separately on those smaller chunks).

Creating system under test

This is what I ended up with:

Creating system under test

// region $sut = new WaitingScriptFactory()
$sut = new WaitingScriptFactory(
    new CallerContextFactory(
        new StaticWaitingMusicRepository(self::MUSIC_URL),
        new StaticLinePositionAnnouncementConfiguration(),
        new StaticAnnouncementFactory($this->announcement),
    ),
    new StaticResponseBuilderFactory($this->responseBuilder),
    new AnnouncementScript(
        new StaticVoiceSettingsRepository(),
        new TestLogger(),
    ),
    new TrimmedAudioFileFactory(
        new SimpleMusicTrimmer(),
        new StaticLinePositionAnnouncementConfiguration(),
        new TestLogger(),
    ),
    new StaticWaitingUrlGenerator(),
);
// endregion

Do you see all those Static, Simple and Test prefixes? Those are obviously test doubles implementing an interface, but the other ones are concrete, final implementations. This is why I can’t — and don’t want to — mock them, and this is why I have 6 transitive dependencies here.

Gherkin syntax

First of all, @zelazowy introduced gherkin syntax in our unit tests a while back. This is the best thing since sliced bread.

Gherkin syntax

public function test no announcement during the initial phase(): void
{
    $this->given it’s only the initial phase();
    $this->when i build();
    $this->then i expect the waiting music to be cut to n seconds(3);
    $this->and a redirect to next phase(WaitPhase::ANNOUNCEMENT());
}

public function test i can hear the announcement during the announcement phase(): void
{
    $this->given it’s the announcement phase();
    $this->when i build();
    $this->then i expect an announcement();
    $this->and the waiting music to be cut to n seconds(10);
    $this->and a redirect to next phase(WaitPhase::ANNOUNCEMENT());
}

Yes, and we use spaces in our method names. You’ll need to get off my back on that one.

The setup

The first step is to initialize some starting conditions. You remember the creating system under test part? As more test cases are added, it becomes more customizable. For example, the StaticFooRepository gets replaced with a ThrowingFooRepository, etc. We gather all of the common stuff in the setUp() method, and then use givens to update them according to the use case.

Gherkin syntax

protected function setUp(): void
{
    $this->responseBuilder = new StaticResponseBuilder();
    $this->phase = WaitPhase::INITIAL();
    $this->facilityId = FacilityIdMother::enabled();
    $this->announcement = Announcement::of(MessageText::ofWords(self::ANNOUNCEMENT_MESSAGE));
}

private function given feature is disabled(): void
{
    $this->facilityId = FacilityIdMother::disabled();
}

private function given phase does not allow announcements(): void
{
    $this->phase = WaitPhase::NO_ANNOUNCEMENTS();
}

private function given caller has no announcement during phase(WaitPhase $phase)
{
    $this->phase = $phase;
    $this->announcement = null;
}

This is where the magic happens. So those are kind of like data providers, but mostly replace dependencies instead. And you use gherkin syntax to describe the scenarios. Pure sugar.

You can also spot the mother pattern there, so the boilerplate is extracted to a separate class.

Expectations (assertions)

The only thing left is some assertions about the results / public side effects:

Gherkin syntax

private function and a redirect to next phase(WaitPhase $phase): void
{
    $instruction = array_shift($this->responseBuilder->instructions);
    self::assertNotNull($instruction, 'Missing expected redirect instruction');
    self::assertSame('post', $instruction[0], 'Oops, there is no redirect to the next phase');
    self::assertStringEndsWith($phase->getValue(), $instruction[1], 'Invalid phase detected for the next iteration');
}

private function i expect an announcement(): void
{
    $instruction = array_shift($this->responseBuilder->instructions);
    self::assertNotNull($instruction, 'Missing expected announce instruction');
    self::assertSame('announce', $instruction[0], 'Expected an announce item in script');
    self::assertSame(self::ANNOUNCEMENT_MESSAGE, $instruction[3], 'The announce message does not match');

    $instruction = array_shift($this->responseBuilder->instructions);
    self::assertNotNull($instruction, 'Missing expected pause instruction');
    self::assertSame('pause', $instruction[0], 'The announcement should end in a pause');
}

The end result

Following this pattern, we end up with test cases that:

  • reflect business use cases rather than the technical details
  • are easy to understand and modify
  • use the same process for each test and only modify the outside context

Give it a try. I’d highly recommend it.

Was this interesting?

About the author

My name is Maciej Łebkowski. I’m a full stack software engineer, a writer, a leader, and I play board games in my spare time. I’m currently in charge of the technical side of a new project called Docplanner Phone.

Creative Commons License
This work is licensed under a Creative Commons Attribution-ShareAlike 4.0 International License. This means that you may use it for commercial purposes, adapt upon it, but you need to release it under the same license. In any case you must give credit to the original author of the work (Maciej Łebkowski), including a URI to the work.