Testing Legacy PHP Scripts

January 26th, 2018

I gave myself a challenge: to test a legacy "controller" in isolation, yet with minimal impact on the original code. Here is the portion of the script that I will test (simplified):

<?php
require __DIR__."/../includes/common.inc.php";
require __DIR__."/includes/common.inc.php";

if ($_GET['a'] == 'preview') {
    require __DIR__."/classes/billing.php";
    $invoices = \Billing::prepareInvoices(new \DateTime('first day of last month'), new \DateTime('last day of last month'));
    ?>
    <h1>Billing Cycle Preview</h1>

    <ul>
        <?php foreach ($invoices as $invoice):?>
            <li><?= $invoice['total'] ?></li>
        <?php endforeach ?>
    </ul>
    <?php
}
//...

Isolation

I use my standard procedure regardless of the type of code I'm testing: I list all the collaborators of the primary method. Here, there are no methods to speak of. The code that I want to test is between the curly braces of the if statement. The collaborators are:

• Everything that is loaded via require
• Billing::prepareInvoices

These collaborators can be tested separately or as part of integration/system tests.

The biggest problem with require is that these files in turn load other files. When executed in its entirety, this script will complain about missing $_SESSION keys, $_SERVER keys, session already started and other garbage.

Billing::prepareInvoices

This was the simplest one to isolate. It's just a static call that we can stub. I used Mockery for this. In my test case:

$billingStaticMock = \Mockery::mock('alias:Billing');
$billingStaticMock->shouldReceive('prepareInvoices')->andReturn([[
    'customer_id' => 1,
    'subtotal' => 30.00,
    'total' => 34.50,
]]);

This will intercept calls to Billing::prepareInvoices and return the canned response, so that $invoices in my script would contain whatever I need it to contain for that particular test case.

If we had something like <?= $invoice['something_else'] ?> in the script, the test would fail. This is useful to check whether our "views" would crash under certain circumstances.

require

This one was trickier and dirtier. Although there are mocking frameworks that help us with methods, require is a language construct. You can tell because it can be called without parenthesis. There is currently no way to mock constructs.

The solution? Proxy the construct through a method. I created a class:

<?php
namespace App\Util;

class Php
{
    public function require(string $filename):void
    {
        require $filename;
    }
}

I know, it looks cringy, but bear with me. Now I can do something like this:

<?php
require_once __DIR__."/../vendor/autoload.php";
use App\Util\Php;
$php = new Php();

$php->require(__DIR__."/../includes/common.inc.php");
$php->require(__DIR__."/includes/common.inc.php");
$php->require(__DIR__."/classes/billing.php");

Proxying the construct adds a tiny bit of overhead, but that is the least of this legacy's problems. You can even put the first 3 lines into a little bootstrap.php file and prepend it for all server requests using the auto_prepend_file setting. Now we can stub it too.

$functionMock = \Mockery::mock('overload:\App\Util\Php');
$functionMock->shouldReceive('require');

There, these files won't get loaded anymore. You get the desired isolation.

Executing the Controller

Once you stubbed all the collaborators, you just need to set the globals used in the main script, then load the script.

$_GET['a'] = 'preview';
ob_start();
require __DIR__.'/../../../admin/billing.php';
ob_end_clean();

I'm using output buffer to avoid polluting my CLI output.

Complete Test

Here is what my PHPUnit test class contains:

public static function setUpBeforeClass()
{
    $functionMock = \Mockery::mock('overload:\App\Util\Php');
    $functionMock->shouldReceive('require');
}

public function testPreview_WithOneInvoice()
{
    $functionMock = \Mockery::mock('overload:\App\Util\Php');
    $functionMock->shouldReceive('require');

    $billingStaticMock = \Mockery::mock('alias:Billing');
    $billingStaticMock->shouldReceive('prepareInvoices')->andReturn([[
        'customer_id' => 1,
        'subtotal' => 30.00,
        'total' => 30.00,
    ]]);

    $_GET['a'] = 'preview';

    ob_start();
    require __DIR__.'/../../../admin/billing.php';
    ob_end_clean();

    $this->assertCount(1, $invoices);
}

Refactor Instead?

Of course, most people's reaction is "Why bother? Just refactor!"

Refactoring a big chunk of legacy at once is not always feasible or profitable for the company. See some phploc stats, excluding vendor:

Logical Lines of Code (LLOC)                  178860
Functions                                     3028
Maximum Method Length                         177
Not in classes or functions                   22965
Maximum Method Complexity                     158.00
Global Accesses                               3237
Static Method Calls                           5888

It's not the worst I've had to work with, but still, it's a challenge.

I would first refactor places that contain known bugs, parts that can be isolated without too much risk or parts that would bring the most value to the business. Gutting an application or starting over is everyone's dream, but it's not always that simple. Someone already treid to rewrite this application and failed, introducing new bugs in the process.

The approach I showed here requires minimal changes, introduces no risk and the test setup is rather short and clean, given the constraints.

Happy testing!

Previous: How I Got 100% on Google PageSpeed Insights Next: What Do Speakers Look for in Reviews?