atoum / visibility-extension

The atoum visibility-extension allows you to override method visibility in your unit tests.
Other
4 stars 6 forks source link

Unable to override mocked private methods #8

Closed agallou closed 8 years ago

agallou commented 8 years ago

Let's take this example :

<?php

namespace titi
{
    class foo
    {
        public function baz()
        {
            return $this->bar();
        }

        private function bar()
        {
            return 'fail';
        }
    }
}

namespace titi\tests\units
{
    use mageekguy\atoum;

    class foo extends atoum\test
    {
        public function testBar()
        {
            $this
                ->given(
                    $this->mockGenerator
                        ->makeVisible('bar')
                        ->generate('\titi\foo')
                )
                ->if($mockedSut = new \mock\titi\foo)
                ->and($this->calling($mockedSut)->bar = 'foo')
                ->then
                    ->string($mockedSut->baz())->isEqualTo('foo')
                    ->string($mockedSut->baz())->isEqualTo('foo')
                    ->mock($mockedSut)
                        ->call('bar')->twice()
            ;
        }
    }
}

(or directy this gist : https://gist.github.com/agallou/5a7fe3b2598449271f5a ) (that's almost the same as the example in the README)

when running this test we have this error :

In file /home/agallou/tests/test-atoum-visibility/test.php on line 36, mageekguy\atoum\asserters\phpString() failed: strings are not equal
-Expected
+Actual
@@ -1 +1 @@
-string(3) "foo"
+string(4) "fail"

When running the same test with bar as a protected method, it pass.

In the README, there is

Doing this will create a child class (the mock) and define the protected/private methods as public

So, private methods cannot be override in a mock, it's the "real method" that is . Is this normal ? (this could be useful, for example if a private method does a request, and we want to check the parameters without doing the request).

jubianchi commented 8 years ago

@agallou in fact, it does not work, the readme is wrong.

Allowing private method mocks is really hard and has side effects. Let me sum-up what happens :

The private method is actually mocked but because the call to the private happens in the parent class (and we cannot override privates in child classes) the original method is always called. If you try something like this on PHP > 5.3, the test will pass :

public function testBar()
{
    $this
        ->given(
            $this->mockGenerator
                ->makeVisible('bar')
                ->generate('\titi\foo')
        )
        ->if($mockedSut = new \mock\titi\foo)
        ->and($this->calling($mockedSut)->bar = 'foo')
        ->and($this->calling($mockedSut)->baz = function() { return $this->bar(); })
        ->then
            ->string($mockedSut->baz())->isEqualTo('foo')
            ->string($mockedSut->baz())->isEqualTo('foo')
            ->mock($mockedSut)
                ->call('bar')->twice()
    ;
}

There is one solution to allow mocking and calling the private "correctly" but it has too many side-effects: we could use the reflection to ake the parent private method accessible but this will lead to unexpected behavior: the method will become callable by everything in the test but also outside of the test (in production code) which is not what we want.

If someone mocks a private (it will become accessible) and later call this same private from a tested class, tests will be green but outside of atoum, everything will be broken.

jubianchi commented 8 years ago

BTW, thanks for your issue, it made me find a bug in atoum :)