PHP automated refactoring with PHP-Parser: convert SimpleTest mocks to Mockery

One of 2018 challenge for Tuleap is the migration to PHP7. I know, we are late. But it's not that easy to migrate 800k LoC with some them written when PHP3 was still a thing.

As part of our transformation process, having our test suite running with PHP7 is something that should come first but it's not that simple (uh oh). The very first tests were introduced back in 2005 and back in time (no iphone, no facebook, etc) phpunit was not that dominant plus SimpleTest had very good support for mock and partial mock objects. Fast forward 13 years later: simpletest if not dead is not in a very good shape. While the latest versions are PHP7 compatible the mock system is quite broken and the activity is low.

In between, alternative mock systems were made available and Mockery is doing a really great job. We started to use it as part of our test suite and it was big gain as the mocking system is far more advanced and less tolerant to errors so tests are more robust. The big deal however is: what to do with the legacy test suite and the 7300 tests?

PHP-Parser

PHP-Parser is parser for php language written in PHP by the awesome Nikita Popov. It can load any 5.2+ php file, generate an Abstract Syntax Tree (AST), allows you to visit and modify it and dump the result into a valid php file. Better, since a couple of month on master and since a couple of days with the stable 4.0.0 release, PHP-Parser allows to do formatting-preserving pretty printing, that is to say you can load you file, do AST transformation within, serialize the result and keep difference minimal between the source and destination (comments, spaces, etc) are kept. It's the perfect tool to do our kind of transformations.

So we started to develop a tool, based on PHP-Parser to assist us in the transformation.

SimpleTest to Mockery

  • Convert class level Mock::generate('XYZ') + new MockXYZ into \Mockery::spy(XYZ::class) in each test
  • Convert class level Mock::generatePartial('XYZ') + new MockXYZ into \Mockery::mock(XYZ::class)->makePartial()->shouldAllowMockingProtectedMethods() in each test
  • Remove top level Mock::generate*
  • Convert setReturnValue, setReturnReference, expectOnce, expectNever, expectCallCount
  • Inject parent::setUp() and parent::tearDown() when they are missing
  • Convert dirname(__FILE__) to __DIR__ (because why not...)
  • (specific to Tuleap test suite) Convert Tuleap mock() and partial_mock() wrappers

That's pretty neat as it's only a couple of hours from time to time on the last 2 weeks.

Concretly speaking here an example of the transformations. Given the following SimpleTest TestCase:

After having run the transformation:

As you can see the generated code is very close to what you had in input. However it's not as concise as idiomatic Mockery would have been because we do a "line by line" transformation. You can notice however than despite having introduced new statements the original formatting (line breaks to logically separate the blocks) remains.

What would have been expected is that the following code:

Mock::generate('AdminDelegation_UserServiceLogDao');

...

$usDao  = new MockAdminDelegation_UserServiceDao($this);
$usDao->expectOnce('addUserService', array(112, AdminDelegation_Service::SHOW_PROJECT_ADMINS));
$usDao->setReturnValue('addUserService', true);

should have been converted to:

$usDao = \Mockery::spy(AdminDelegation_UserServiceLogDao::class)
    ->shouldReceive('addUserService')
    ->with(112, AdminDelegation_Service::SHOW_PROJECT_ADMINS)
    ->andReturns(true);

However we cannot predict the order in which the statement were written with simple test, if the expectation is defined after the return or if there are 20 lines between the former and the later, it's hard to collect all operation and to aggregate them. Not to say that some relevant comments might have been set on one of the simpletest assertion or expectation and we would loose the main interest of having a Formatting-preserving pretty printing.

So the design choice was to introduce an intermediate variable to hold Mockery expecation/returns, so we can have as many lines as needed:

$usDao  = \Mockery::spy(AdminDelegation_UserServiceDao::class);
$usDao_addUserService = $usDao->shouldReceive('addUserService');
$usDao_addUserService->with(112, AdminDelegation_Service::SHOW_PROJECT_ADMINS);
$usDao_addUserService->once();
$usDao_addUserService->andReturns(true);

Internally, we keep track of methods declaration so regardless if return, expectation, etc comes first we will have only one call to shouldReceive for one method.

This also works when the mock is defined in setUp.

Under the hood

As already stated, most of the work is really done by PHP-Parser itself. Our logic is enclosed in SimpleTest2MockeryVisitor that was added to the boilerplate needed for load, parse and save with formatting-preserving pretty printing.

A simple example to start: we want to replace all occurences of dirname(__FILE__) by __DIR__ in include statements:

if ($node instanceof Node\Expr\Include_ &&
    $node->expr instanceof Node\Expr\BinaryOp\Concat &&
    $node->expr->left instanceof Node\Expr\FuncCall &&
    (string)$node->expr->left->name->parts[0] === 'dirname' &&
    $node->expr->left->args[0]->value instanceof Node\Scalar\MagicConst\File
) {
    $node->expr->left = new Node\Scalar\MagicConst\Dir();
}

As we work on the AST, its a bit verbose but its rather straightfoward (a couple of var_dump will help for the very first attempts).

What was more complex to achieve was the transformations needed to transform the 2 calls ->expectOnce() and ->setReturnValue() into 4 mockery calls: ->shouldReceive, with(), once() and andReturns.

The main issue we face was to work at the wrong level in the AST. Most the simple examples you can find on PHP-Parser (including the one I gave just before) works at Node\Expr level. However when you want add or remove lines (statements) you need to work at Node\Stmt\Expression level, at this level the node you want to work with will be in $node->expr.

As an example, here is the simplified pseudo-code for removing static calls Mock::generate(...):

if ($node->expr instanceof Node\Expr\StaticCall &&
    (string)$node->expr->class === 'Mock' && 
    (string)$node->expr->name === 'generated') {
        return NodeTraverser::REMOVE_NODE;
}

You also now have a very good example provided in the documentation, look at the last example about spliting return statement in 2 calls.

Current state and future work

The current state already works for tests that only rely on setReturnValue, setReturnReference, expectOnce, expectNever, expectCallCount. The other expectations will be added in the incoming weeks. However full automation is probably not reasonable to target, this tool is meant to be an assistant for conversion. It tries to does as much of the heavy lifting but it cannot handle all the cases. Typically, Mockery mocks (even if spies here) are much more coercive than SimpleTest ones, you will often have to rework the generated code to fine tune mocks.

If you are interested in this kind of conversion and want to help, feel free to create a PR, contributions are welcomed.

Again a big thank you to Nikita Popov whose work is invaluable.

As a more generic conclusion, I would highlight that working with PHP in 2018 is really great given the quality of the ecosystem tooling: PHP7.X, phan, PHP-Parser, composer, mockery, phpunit and blackfire to name a few.

Share this post

Leave a comment

To prevent automated submissions please leave this field empty.

Plain text

  • No HTML tags allowed.
  • Web page addresses and e-mail addresses turn into links automatically.
  • Lines and paragraphs break automatically.
Websites located at tuleap.org and other tuleap.org subdomains need to store and access cookies on your device. We need your acceptance. Get more information. Yes, I agree No, I disagree