DEV Community

Cover image for How I Refactored My Test Output Architecture for MicroUnit
Mitar Nikolic
Mitar Nikolic

Posted on

How I Refactored My Test Output Architecture for MicroUnit

Disclaimer: every time I use the word TestWriter I am referring to the different implementations of the ITestWriter interface.

When I first released MicroUnit, I rushed the beta and left some design baggage in the code. Specifically, the output formatting logic was scattered across multiple classes like assertions, the Diff helper, and even a few other places.

This approach did get the job done and allowed me to show the kind of test output I wanted. However, it was clear from the start that I wasn’t happy with it. So even then it was clear → it had to go at some point. Now I can finally say I found the time to refactor this part of the code.

In this article, I'm going to try to explain exactly how this part of the code works, what the problem was and my idea + implementation to fix it.

Enjoy !


The Core Architecture

Let me give you a little insight on how the core architecture of MicroUnit is designed so you understand what I'm talking about.

So simplifying it to only the things relevant for this post, the flow is as follows:

Case: Test failing on Assertion

Tester runs a test

  1. Test calls for example Assert::equals($expected, $actual)
  2. Assert::equals(...) throws TestFailedException containing a formatted error message (possibly with a diff view)
  3. Tester catches the Exception and creates a TestResult with the same message
  4. TestResult gets passed to one or more implementation(s) of ITestWriter (depending on your custom configuration)
  5. TestWriters use the formatted error message and re-format it accordingly to be able to output to their according source (console, file, html, etc.)

The Problem

While it works, having an architecture like this introduces many problems to the code:

  1. Essentially, the errors are being formatted twice. Once by the assertion methods and once by the TestWriters
  2. Since the diff view gets concatenated to the error message, the TestWriters get it as one string, having to either work with everything as a whole or having to pick apart the string in order to be able to get the diff separately.
  3. With all the formatting being applied to the string, it's nearly impossible for people that might want to create their own ITestWriter implementation to predict how the string is going to act in the environment they paste it in.
  4. It violates the "separation of concerns" principle, since the formatting should only be the concern of the TestWriters.

To give you an idea as to how this bad implementation looked in practice, here is the old Assert::equals(...) method.

public static function equals(mixed $expected, mixed $actual): void
    {
        if ($expected != $actual) {
            $diff = Diff::generate(ValueExporter::export($expected), ValueExporter::export($actual));
            throw new TestFailedException(
                'The provided two values are not equal' . PHP_EOL
                    . ValueExporter::export($diff)
            );
        }
    }

Enter fullscreen mode Exit fullscreen mode

The Idea To Solve This

Now, the basic idea to solving all of this was to have the Assert class and all its methods work with a data only representation of a failed test. Just passing on information from the assertion without worrying about any presentation details.

The Diff view should be a separate optional thing also not formatted and would be generated by calling Diff::generate(...) by the assertion methods that need it.

This way all the formatting is left to the TestWriters, and they get the unformatted Diff and error message separately and can assemble them freely to match their respective output format.

In this post, I want to walk through what I changed, why I changed it, and the benefits of the refactor.


The Implementation

In order to implement this, I introduced the AssertionFailure class. Which would contain all the necessary information about why the assertion failed.

final class AssertionFailure
{
    public function __construct(
        public readonly string $message = 'Unknown Assertion Failure',
        public readonly mixed $expected = null,
        public readonly mixed $actual = null,
        public readonly ?Diff $diff = null,
        public readonly array $metadata = []
    ) {}
}

Enter fullscreen mode Exit fullscreen mode

To the TestFailedException I added a AssertionFailure $failure property to hold all relevant information.

Now the assertion methods just had to throw a TestFailedException and pass the correct information to an instance of AssertionFailure.

I also removed all formatting logic from the Diff class and moved it into a DiffFormatter

class DiffFormatter
{
    public static function toString(Diff $diff): string
    {
        $converted =  '--- Expected' . PHP_EOL;
        $converted .= '+++ Actual' . PHP_EOL;
        $converted .= '@@ @@' . PHP_EOL;
        foreach ($diff->diffLines as $line) {
            $prefix = match ($line->type) {
                DiffLineType::Same => '  ',
                DiffLineType::ExpectedDifferent => '- ',
                DiffLineType::ActualDifferent => '+ ',
            };
            $converted .= $prefix . rtrim($line->line, "\r\n") . PHP_EOL;
        }

        return $converted . PHP_EOL;
    }
}

Enter fullscreen mode Exit fullscreen mode

I did this because the logic to generate a string representation of a Diff is consistent across all TestWriters.

Then all there was left to do was to change the ITestWriter instances to consume the raw data from the AssertionFailure and assemble/format it correctly, depending on the output type (console, file, html, etc.).

For comparison here is the Assert::equals(...) method after the refactoring:

public static function equals(mixed $expected, mixed $actual): void
    {
        if ($expected != $actual) {
            throw new TestFailedException(
                new AssertionFailure(
                    'Expected values to be equal',
                    $expected,
                    $actual,
                    Diff::generate($expected, $actual)
                )
            );
        }
    }

Enter fullscreen mode Exit fullscreen mode

Key Takeaway

When designing an open-source project, there are always going to be parts of your code that you are going to be unhappy about.

Especially in the early stages of development where you are mostly working on your own, you sometimes make sacrifices to quality in order to make more progress in the limited time you have.

While there is nothing wrong about this, it's very important to take the time, revisit these "flaws" and refactor them one by one every once in a while.

That way, you can maintain a high quality code base while still making steady progress towards finishing the project or implementing new features.

It's the mix of both refactoring and adding new things that makes the magic ;)

If you want to see MicroUnit in action, check it out on GitHub

Or install it via Packagist: microunit/microunit

Top comments (0)