eloquent / phony

Mocks, stubs, and spies for PHP.
http://eloquent-software.com/phony
MIT License
194 stars 7 forks source link

procedural function stubbing unclear #249

Closed gegnew closed 3 years ago

gegnew commented 4 years ago

Hello,

I'm trying to use Phony to write tests for a very procedural, and largely badly-written, legacy codebase. Almost nothing is OO, and I really need to be able to stub some functions. Unfortunately, there are also almost no namespaces, but primarily file-relative imports.

I am trying to use https://eloquent-software.com/phony/latest/#stubs to figure out a way to test this, but I'm having trouble. More specifically:

I have a class auth/ApiKey/ApiKeyValidation.php that imports a bunch of database functions. Specifically, here it uses db_esc.

require_once( __DIR__ . '/../../libraries/db.php' );
class ApiKeyValidation extends ApiKey
{
    ...

    public static function getKeyProperties($key)
    {
        ...
        $esc_key = db_esc($key)
    }
}

I have a test file in tests/auth/ApiKey/ApiKeyTest.php that tests this getKeyProperties method. Because of the absolute mess that is the db.php file, I really have to mock the db_esc function itself.

From my reading of the docs, this should be possible in the test file with:

final class ApiKeyTest extends \PHPUnit\Framework\TestCase 
{
    ...
    public function test_whatever(): void
    {
        $key = 'some key';
        //.... etc, set up here
        $stub = stub('db_esc')->with($key)->returns('some sanitized key');
        $apiKeyValidation->getKeyProperties($key);
    }
}

Unfortunately, this stubs nothing, and just calls db_esc.

I tried also without quotation marks: $stub = stub(db_esc)->with($key)->returns('some sanitized key'); making sure that db.php was added in the composer autoload/files directive. This results in Use of undefined constant db_esc.

I also tried adding namespacing to:

In db.php, namespace Whatever\DB; In ApiKeyValidation.php, use Whatever\DB as DB; and DB\db_esc($key) In ApiKeyTest.php, use Whatever\DB as DB; and stub(DB\db_esc)

But to no avail, as this gives me Undefined constant 'Services\Libraries\DB\db_esc'

I'm totally lost, and new to PHP to boot. Any advice?

ezzatron commented 4 years ago

Have a read of the section on Stubbing global functions, as it's probably your best bet. Just be aware that it only works under certain conditions (detailed in the docs I just linked to). For a TL;DR:

The stub() function you're using is for when you can pass the function into the system under test (i.e. dependency injection). If you're dealing code that calls a function that's already declared, your options are far more limited.

A couple of notes about PHP that might help you:

In your particular case, the following kind of setup is needed in order for stubGlobal() to work (written from memory, might not be 100% correct):

// auth/ApiKey/ApiKeyValidation.php
namespace Auth\ApiKey; // ApiKeyValidation MUST be in a namespace

class ApiKeyValidation extends ApiKey
{
    public static function getKeyProperties($key)
    {
        $esc_key = db_esc($key); // db_esc MUST NOT be in a namespace
    }
}

// tests/auth/ApiKey/ApiKeyTest.php
namespace Auth\ApiKey;  // note the namespace here too

use PHPUnit\Framework\TestCase;
use function Eloquent\Phony\restoreGlobalFunctions;
use function Eloquent\Phony\stubGlobal;

final class ApiKeyTest extends TestCase 
{
    protected function setUp() : void
    {
        // __NAMESPACE__ is just a string containing the current namespace (i.e. 'Auth\ApiKey')
        $this->db_esc = stubGlobal('db_esc', __NAMESPACE__); 
    }

    protected function tearDown() : void
    {
        restoreGlobalFunctions();
    }

    public function testWhatever(): void
    {
        $this->db_esc->with( 'some key')->returns('some sanitized key');

        // in your example test, you were calling getKeyProperties wrong
        // static methods are called with the :: operator:
        $this->assertSame($expected, ApiKeyValidation::getKeyProperties($key));
    }
}

There can be some other tricky problems surrounding stubGlobal(). From memory, I think PHP will only try to resolve db_esc once, meaning if db_esc is called before stubGlobal(), this whole approach won't work, so keep that in mind too.

If you're willing to do a bit more refactoring, you could also use an approach like this, by converting the class to be something you instantiate, rather than something you call statically. This is a more common approach in modern PHP:

// auth/ApiKey/ApiKeyValidation.php

namespace Auth\ApiKey;

class ApiKeyValidation extends ApiKey
{
    public function __construct($dbEsc = 'db_esc')
    {
        $this->dbEsc = $dbEsc;
    }

    public function getKeyProperties($key)
    {
        // you can either call it like this:
        $dbEsc = $this->dbEsc;
        $esc_key = $dbEsc($key);

        // or like this:
        $esc_key = call_user_func($this->dbEsc, $key);
    }

    private $dbEsc;
}

// tests/auth/ApiKey/ApiKeyTest.php
namespace Auth\ApiKey;

use PHPUnit\Framework\TestCase;
use function Eloquent\Phony\stub; // regular stub works with this approach - much cleaner

final class ApiKeyTest extends TestCase 
{
    public function testWhatever(): void
    {
        $dbEsc = stub()->with( 'some key')->returns('some sanitized key');
        $apiKeyValidation = new ApiKeyValidation($dbEsc);

        $this->assertSame($expected, $apiKeyValidation->getKeyProperties($key));
    }
}

Hopefully that's enough to get you started.

ezzatron commented 4 years ago

The section on Alternatives for stubbing global functions might also be useful, but it's mostly another example of the type of refactoring I suggested.

gegnew commented 4 years ago

Thank you! This is incredibly helpful.