Give `Mocker` the ability to stub global function calls. #814

Merged
merged 1 commit into from Feb 6, 2013

Conversation

Projects
None yet
4 participants
@blainesch
Member

blainesch commented Feb 6, 2013

This is more of a trick, since calling a funciton like get_called_class() in a namespace is first searched for in the current namespace, then globally. Here, we simply create it in the same namespace it's called allowing users to overwrite the return value of it. The user defined function can be overwritten, and if deleted will result in the global function being called normally.

Often times we make methods that call php-defined functions such as define, compact, eval, json_decode, token_get_all, and the list goes on. Often it's best to change the php-defined functions to manipulate the behavior of the method, but putting a giant list of php-defined functions in a config variable is not very elegant and would start making your code harder to read, and doing so for testing seems a bit over the top. This allows you to code normally and still have testable code.

This pull request provides a simple and reset-able api to allow stubbing of functions.

For example, nobody wants to create a new file for each test (starting to sound like a factory vs fixture debate...) so you could stub some functions which put your data a lot closer to your test. Here is an example I threw together:

<?php

namespace app\extensions;

class AwesomeFileEditor {

    public static function updateJson($file) {
        if (file_exists($file)) {
            $time = microtime(true);
            $packages = json_decode(file_get_contents($file), true);
            foreach ($packages['users'] as &$package) {
                $package['updated'] = $time;
            }
            return $packages;
        }
        return false;
    }

}

?>
<?php

namespace app\tests\cases\extensions;

use lithium\test\Mocker;
use app\extensions\AwesomeFileEditor;

class AwesomeFileEditorTest extends \lithium\test\Unit {
    public function setUp() {
        Mocker::overwriteFunction(false);
    }

    public function testUpdateJson() {
        Mocker::overwriteFunction('app\extensions\file_exists', function() {
            return true;
        });
        Mocker::overwriteFunction('app\extensions\file_get_contents', function() {
            return <<<EOD
{
    "users": [
        {
            "name": "BlaineSch",
            "updated": 0
        }
    ]
}
EOD;
        });

        $results = AwesomeFileEditor::updateJson('idontexist.json');
        $this->assertNotEqual(0, $results['users'][0]['updated']);
    }
}

?>
@blainesch

View changes

test/Mocker.php
@@ -122,6 +131,16 @@ class Mocker {
),
);
+ protected static $_mockFunctionIngredients = array(

This comment has been minimized.

Show comment Hide comment
@blainesch

blainesch Feb 6, 2013

Member

forgot docblocks

@blainesch

blainesch Feb 6, 2013

Member

forgot docblocks

@blainesch

This comment has been minimized.

Show comment Hide comment
@blainesch

blainesch Feb 6, 2013

Member

Added docblocks to the undoc'd variable and added the example to the class docblocks.

Member

blainesch commented Feb 6, 2013

Added docblocks to the undoc'd variable and added the example to the class docblocks.

@ericcholis

This comment has been minimized.

Show comment Hide comment
@ericcholis

ericcholis Feb 6, 2013

Contributor

I like it, specifically the file_get_contents example. Could be very useful for mocking 3rd Party APIs without Integration Testing.

Contributor

ericcholis commented Feb 6, 2013

I like it, specifically the file_get_contents example. Could be very useful for mocking 3rd Party APIs without Integration Testing.

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Feb 6, 2013

Owner

That is... pretty fuckin' clever.

Owner

nateabele commented Feb 6, 2013

That is... pretty fuckin' clever.

nateabele added a commit that referenced this pull request Feb 6, 2013

Merge pull request #814 from BlaineSch/feature/mockFunctions
Give `Mocker` the ability to stub global function calls.

@nateabele nateabele merged commit cd6d383 into UnionOfRAD:dev Feb 6, 2013

1 check passed

default The Travis build passed
Details

@blainesch blainesch deleted the blainesch:feature/mockFunctions branch Feb 6, 2013

@jails

This comment has been minimized.

Show comment Hide comment
@jails

jails Mar 28, 2013

Contributor

I see the benefit of such feature but due to PHP limitations, this will not make test isolation possible.
If a test class TestA instanciate A which run some functions, you will not be able in a class TestB to overwrite any function in the A namespace since function has already been loaded during TestA. And there's no way in PHP to unload a class.
This will conflicts almost all the time if integration tests are runned before unit tests for example.
Imo this should be reverted or we should find a way to make test isolation possible.

Contributor

jails commented Mar 28, 2013

I see the benefit of such feature but due to PHP limitations, this will not make test isolation possible.
If a test class TestA instanciate A which run some functions, you will not be able in a class TestB to overwrite any function in the A namespace since function has already been loaded during TestA. And there's no way in PHP to unload a class.
This will conflicts almost all the time if integration tests are runned before unit tests for example.
Imo this should be reverted or we should find a way to make test isolation possible.

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Mar 28, 2013

Owner

Maybe Mocker needs an unregister() method that you can call in tearDown(), which resets all the function references. So for example, if you have this:

Mocker::overwriteFunction('app\extensions\file_get_contents', function() {
    return "stuff";
});

Then unregister() would basically unhook that closure from the file_get_contents() reference for that namespace, and for any Mocker-defined functions that aren't bound to implementations, the behavior would just be this:

function() {
    return call_user_func_array("file_get_contents", func_get_args());
}

Make sense? I think that would provide pretty effective test isolation.

Owner

nateabele commented Mar 28, 2013

Maybe Mocker needs an unregister() method that you can call in tearDown(), which resets all the function references. So for example, if you have this:

Mocker::overwriteFunction('app\extensions\file_get_contents', function() {
    return "stuff";
});

Then unregister() would basically unhook that closure from the file_get_contents() reference for that namespace, and for any Mocker-defined functions that aren't bound to implementations, the behavior would just be this:

function() {
    return call_user_func_array("file_get_contents", func_get_args());
}

Make sense? I think that would provide pretty effective test isolation.

@jails

This comment has been minimized.

Show comment Hide comment
@jails

jails Mar 28, 2013

Contributor

What I mean is more when you want to overwrite a function once it has already been called.

For exampe if I consider the following class:

namespace app\libs;
class A {
    public function hello($args) {
        var_export($args);
    }   
}

And the following test function:

public function testMock() {
    Mocker::overwriteFunction('app\controllers\var_export', function() {
        echo 'hello';
    });
    $a = new A();
    $a->hello('args'); // It's ok the 'hello' string is correclty echoed
}

But the following make the overwrite fail:

public function testMock() {
    $b = new A();
    $b->hello('args'); // the 'args' string is echoed (it's ok)
    Mocker::overwriteFunction('app\controllers\var_export', function() {
        echo 'hello';
    });
    $a = new A();
    $a->hello('args'); // the 'args' string is still echoed !!! overwriting don't work anymore
}

So to sum it up. If a previous test use the 'A::hello()' the Mocker won't be able to overwrite the function and all test done with overwrited function for 'A::hello()' will fail.

Contributor

jails commented Mar 28, 2013

What I mean is more when you want to overwrite a function once it has already been called.

For exampe if I consider the following class:

namespace app\libs;
class A {
    public function hello($args) {
        var_export($args);
    }   
}

And the following test function:

public function testMock() {
    Mocker::overwriteFunction('app\controllers\var_export', function() {
        echo 'hello';
    });
    $a = new A();
    $a->hello('args'); // It's ok the 'hello' string is correclty echoed
}

But the following make the overwrite fail:

public function testMock() {
    $b = new A();
    $b->hello('args'); // the 'args' string is echoed (it's ok)
    Mocker::overwriteFunction('app\controllers\var_export', function() {
        echo 'hello';
    });
    $a = new A();
    $a->hello('args'); // the 'args' string is still echoed !!! overwriting don't work anymore
}

So to sum it up. If a previous test use the 'A::hello()' the Mocker won't be able to overwrite the function and all test done with overwrited function for 'A::hello()' will fail.

@blainesch

This comment has been minimized.

Show comment Hide comment
@blainesch

blainesch Mar 28, 2013

Member

@jails I suppose PHP is caching the namespace of the function after the first call? I can't seem to reproduce:

public function testMetaTags() {
    var_export('hello'); // 'hello'
    Mocker::overwriteFunction('app\tests\cases\models\var_export', function() {
        echo 'FOOBARBAZ';
    });
    var_export('hello'); // FOOBARBAZ
}

We could potentially have a "setupFunctions" methods that accepts an array of methods to "setup" in the setUp() and create the methods if we have not already.

Mocker::setupFunctions('app\tests\cases\models\var_export', 'app\tests\cases\models\print_r');

@nateabele You can unset functions.

Mocker::overwriteFunction('app\controllers\var_export', function() { echo 'hello'; });
Mocker::overwriteFunction('app\controllers\var_export', false); // reset var_export
Mocker::overwriteFunction(false); // reset all

You can also overwrite the current one.

Mocker::overwriteFunction('app\controllers\var_export', function() { echo 'hello'; });
app\controllers\var_export(); // hello

Mocker::overwriteFunction('app\controllers\var_export', function() { echo 'world'; });
app\controllers\var_export(); // world

As far as static classes and instance objects go you can reset methods as well.

Member

blainesch commented Mar 28, 2013

@jails I suppose PHP is caching the namespace of the function after the first call? I can't seem to reproduce:

public function testMetaTags() {
    var_export('hello'); // 'hello'
    Mocker::overwriteFunction('app\tests\cases\models\var_export', function() {
        echo 'FOOBARBAZ';
    });
    var_export('hello'); // FOOBARBAZ
}

We could potentially have a "setupFunctions" methods that accepts an array of methods to "setup" in the setUp() and create the methods if we have not already.

Mocker::setupFunctions('app\tests\cases\models\var_export', 'app\tests\cases\models\print_r');

@nateabele You can unset functions.

Mocker::overwriteFunction('app\controllers\var_export', function() { echo 'hello'; });
Mocker::overwriteFunction('app\controllers\var_export', false); // reset var_export
Mocker::overwriteFunction(false); // reset all

You can also overwrite the current one.

Mocker::overwriteFunction('app\controllers\var_export', function() { echo 'hello'; });
app\controllers\var_export(); // hello

Mocker::overwriteFunction('app\controllers\var_export', function() { echo 'world'; });
app\controllers\var_export(); // world

As far as static classes and instance objects go you can reset methods as well.

@jails

This comment has been minimized.

Show comment Hide comment
@jails

jails Mar 28, 2013

Contributor

Yeap you're right, I also guess it's due to PHP caching. Indeed your use case works but it's unfortunately not the most frequent one. For a setup step you think about a kind of bootstrap file exectuted before launching tests ? The other option is to run tests as an independant process but it's... well you see what I mean...;-)

Contributor

jails commented Mar 28, 2013

Yeap you're right, I also guess it's due to PHP caching. Indeed your use case works but it's unfortunately not the most frequent one. For a setup step you think about a kind of bootstrap file exectuted before launching tests ? The other option is to run tests as an independant process but it's... well you see what I mean...;-)

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Mar 28, 2013

Owner

@jails Maybe you could write a simple unit test for Mocker that would prove the issue? It really seems like, as long as mocked functions are removed at the end, everything works fine.

Owner

nateabele commented Mar 28, 2013

@jails Maybe you could write a simple unit test for Mocker that would prove the issue? It really seems like, as long as mocked functions are removed at the end, everything works fine.

@jails

This comment has been minimized.

Show comment Hide comment
@jails

jails Mar 28, 2013

Contributor

Yeah sure, I must go right now, but I can show you the issue by simply executing all tests at once:
Here the diff:
jails/lithium@UnionOfRAD:dev...jails:bug/mocker-overwrite-function

And the travis result:
https://travis-ci.org/jails/lithium/jobs/5877274

As you can see some unit tests fails. This is due to the way tests are lauched. Here integrations tests are launched first so the SocketTest (or ServiceTest I don't remember) integration test is runned first. It instanciate a socket class which use stream_context_create / stream_context_get_options to connect to google (if I remember well). When tests cases reach the ContextTest unit test, the Mocker fail to overwrite this function since they had already been loaded, so tests fails.

If unit tests had been exectuted first, every tests would passed.

Imo only a kind of Mocker::setupFunctions (as suggested @blainesch) at a boostrap level should be able to keep the control on overwriting.

Contributor

jails commented Mar 28, 2013

Yeah sure, I must go right now, but I can show you the issue by simply executing all tests at once:
Here the diff:
jails/lithium@UnionOfRAD:dev...jails:bug/mocker-overwrite-function

And the travis result:
https://travis-ci.org/jails/lithium/jobs/5877274

As you can see some unit tests fails. This is due to the way tests are lauched. Here integrations tests are launched first so the SocketTest (or ServiceTest I don't remember) integration test is runned first. It instanciate a socket class which use stream_context_create / stream_context_get_options to connect to google (if I remember well). When tests cases reach the ContextTest unit test, the Mocker fail to overwrite this function since they had already been loaded, so tests fails.

If unit tests had been exectuted first, every tests would passed.

Imo only a kind of Mocker::setupFunctions (as suggested @blainesch) at a boostrap level should be able to keep the control on overwriting.

@blainesch

This comment has been minimized.

Show comment Hide comment
@blainesch

blainesch Mar 28, 2013

Member

I assume something like apc (not php) is potentially caching the namespace of the method after it's first call. This is why it would work for me, but potentially not for @jails. I like the idea of having a setupFunctions() to help you declare which methods you are going to use. I'll submit a pull request soon.

Member

blainesch commented Mar 28, 2013

I assume something like apc (not php) is potentially caching the namespace of the method after it's first call. This is why it would work for me, but potentially not for @jails. I like the idea of having a setupFunctions() to help you declare which methods you are going to use. I'll submit a pull request soon.

@jails

This comment has been minimized.

Show comment Hide comment
@jails

jails Mar 29, 2013

Contributor

APC seems to be one of the factors but only for PHP 5.3. Just tried to disabled APC it in travis and the result is the following:
https://travis-ci.org/jails/lithium/builds/5885629

PHP 5.3 pass, but it failed for PHP 5.4 & PHP 5.5

Contributor

jails commented Mar 29, 2013

APC seems to be one of the factors but only for PHP 5.3. Just tried to disabled APC it in travis and the result is the following:
https://travis-ci.org/jails/lithium/builds/5885629

PHP 5.3 pass, but it failed for PHP 5.4 & PHP 5.5

@blainesch

This comment has been minimized.

Show comment Hide comment
@blainesch

blainesch Mar 29, 2013

Member

PHP 5.4+ potentially emulated some features of apc which could explain the 20%+ speed increase I've heard about between 5.3 and 5.4. Does #871 fix the issue?

Member

blainesch commented Mar 29, 2013

PHP 5.4+ potentially emulated some features of apc which could explain the 20%+ speed increase I've heard about between 5.3 and 5.4. Does #871 fix the issue?

@jails

This comment has been minimized.

Show comment Hide comment
@jails

jails Mar 29, 2013

Contributor

Yeap, with such init at a boostrap level, it should work.

Mocker::setupFunctions(array(
    'lithium\util\getmxrr',
    'lithium\console\command\shell_exec',
    'lithium\console\command\is_dir',
    'lithium\net\socket\feof',
    'lithium\net\socket\fopen',
    'lithium\net\socket\stream_socket_client',
    'lithium\net\socket\stream_get_contents',
    'lithium\net\socket\stream_context_create',
    'lithium\net\socket\stream_get_meta_data',
    'lithium\tests\cases\net\socket\stream_context_get_options',
    'lithium\net\socket\curl_init',
    'lithium\net\socket\curl_setopt',
    'lithium\net\socket\curl_close',
    'lithium\net\socket\curl_exec'
);

But I'm not sure where is the best place for it. Adding a boostrap file for the core ?

Contributor

jails commented Mar 29, 2013

Yeap, with such init at a boostrap level, it should work.

Mocker::setupFunctions(array(
    'lithium\util\getmxrr',
    'lithium\console\command\shell_exec',
    'lithium\console\command\is_dir',
    'lithium\net\socket\feof',
    'lithium\net\socket\fopen',
    'lithium\net\socket\stream_socket_client',
    'lithium\net\socket\stream_get_contents',
    'lithium\net\socket\stream_context_create',
    'lithium\net\socket\stream_get_meta_data',
    'lithium\tests\cases\net\socket\stream_context_get_options',
    'lithium\net\socket\curl_init',
    'lithium\net\socket\curl_setopt',
    'lithium\net\socket\curl_close',
    'lithium\net\socket\curl_exec'
);

But I'm not sure where is the best place for it. Adding a boostrap file for the core ?

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Apr 8, 2013

Owner

Sorry, finally following up on this and #871. It wouldn't be possible to call setupFunctions() in each relevant test where the individual method is overwritten?

Owner

nateabele commented Apr 8, 2013

Sorry, finally following up on this and #871. It wouldn't be possible to call setupFunctions() in each relevant test where the individual method is overwritten?

@jails

This comment has been minimized.

Show comment Hide comment
@jails

jails Apr 8, 2013

Contributor

Not really the setup must be done before any call of "overwrited" & "not yet overwrited" function. To make this kind of setup works it must be runned before any test.

Contributor

jails commented Apr 8, 2013

Not really the setup must be done before any call of "overwrited" & "not yet overwrited" function. To make this kind of setup works it must be runned before any test.

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Apr 8, 2013

Owner

So, no method could be overwritten anywhere before being set up? Er, before all of them are set up?

Owner

nateabele commented Apr 8, 2013

So, no method could be overwritten anywhere before being set up? Er, before all of them are set up?

@jails

This comment has been minimized.

Show comment Hide comment
@jails

jails Apr 8, 2013

Contributor

Yeah if I need to overwrite a function, I need to add it in the setup list. Otherwise the overwriting will fail if the fuction had already been loaded previously (due to a cache issue imo). Since this "caching" can occurs in any test the only solution is to set up all functions which are intended to be overwrited at a "boostrap level". This way we can keep the control on the functions all along the tests.

Contributor

jails commented Apr 8, 2013

Yeah if I need to overwrite a function, I need to add it in the setup list. Otherwise the overwriting will fail if the fuction had already been loaded previously (due to a cache issue imo). Since this "caching" can occurs in any test the only solution is to set up all functions which are intended to be overwrited at a "boostrap level". This way we can keep the control on the functions all along the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment