Filina Consulting, Inc.

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!

Comments

tiso January 31st, 2018 Interesting article, but I have that bad feeling that there is nothing to test here. Maybe I just don't understand what you can do with this and when it can be useful.
soup January 31st, 2018 I can imagine performing some very complex testing with this framework.
Anna Filina February 2nd, 2018 @tiso I couldn't put the whole code due to legal reasons, so I had to show a simplified version of that script. In any case:

1. Useful in scenarios where zero-length arrays would cause the code to have errors.
2. As you add new feature of fix bugs in the script, things that were working might break. This will spot them.
3. The production server for legacy apps is often configured to silence notices. PHPUnit would catch those.
4. Running the tests on a higher version of PHP would spot all the potential errors, so you can upgrade with confidence (or know what needs fixing).
tiso February 5th, 2018 I see now, thanks for explanation.
slk500 February 6th, 2018 ...so you prefer using Mockery than PHPUnit?
Anna Filina February 9th, 2018 @slk500 Mockery gives an ability to mock certain things that PHPUnit doesn't. I generally start with PHPUnit, then add Mockery mocks depending on the situation. The two are not mutually exclusive.

Phone: +1 514-918-7866 | E-mail: me@afilina.com