Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Created mocks + call count expectations + dataProvider = 💥 #1193

Open
MiguelAlcaino opened this issue Oct 13, 2022 · 5 comments
Open

Created mocks + call count expectations + dataProvider = 💥 #1193

MiguelAlcaino opened this issue Oct 13, 2022 · 5 comments

Comments

@MiguelAlcaino
Copy link

MiguelAlcaino commented Oct 13, 2022

HI there.
I found what sounds to me it's a bug, or at least a not desirable behaviour when using dataProviders + Mockery's mocks + calls counts expectations.

Let have this example:

<?php

use Mockery\Adapter\Phpunit\MockeryTestCase;

class A
{
    public function getFirstNumber(): int
    {
        return 2;
    }
}

class B
{
    public function getSecondNumber(): int
    {
        return 3;
    }
}

class MyService
{
    private A $classA;
    private B $classB;

    public function __construct(A $classA, B $classB)
    {
        $this->classA = $classA;
        $this->classB = $classB;
    }

    public function sumValues(): int
    {
        return $this->classA->getFirstNumber() + $this->classB->getSecondNumber();
    }
}

class ResourceServiceTest extends MockeryTestCase
{
    /**
     * @dataProvider myDataProvider
     */
    public function testMySuperFunctionality(A $aInstance, B $bInstance, int $expectedResult): void
    {
        $service = new MyService($aInstance, $bInstance);
        $result  = $service->sumValues();

        self::assertEquals($expectedResult, $result);
    }

    public function myDataProvider(): array
    {
        return [
            '1st case' => [
                $this->createAMock(2),
                $this->createBMock(3),
                5,
            ],
            '2nd case' => [
                $this->createAMock(3),
                $this->createBMock(4),
                7,
            ],
        ];
    }

    private function createAMock(int $number): A
    {
        $mock = Mockery::mock(A::class);
        $mock->allows('getFirstNumber')->andReturn($number)->once();

        return $mock;
    }

    private function createBMock(int $number): B
    {
        $mock = Mockery::mock(B::class);
        $mock->allows('getSecondNumber')->andReturn($number)->once();

        return $mock;
    }
}

That fails. But if you remove any of the dataprovider's cases and leave only one (regardless of which) it works.

I investigated the problem a bit and I realized that every time Mockery::mock() is invoked it internally saves the mock on a container (an array of mocks) and all those are evaluated at the end of each test. This means that for 1st case the container has 4 mocks to evaluate when it should be only 2. As there are 4 mocks, the real expectation for the 1st case is that the 4 mocks call their methods once, but only two of them are meeting the expectations, the other two are not, hence it fails. That happens because the dataprovider returns the whole array with its values inside, hence, the whole array's methods are executed in order to obtain the array's values. Wanna have a look by yourself? Add a breakpoint on line vendor/mockery/mockery/library/Mockery/Container.php:298 and see how the container has 4 mocks instead of 2.

The ugly Workaround: I've been using a workaround in the meantime in which I return a anonynous function that returns the mocks for my case and the function is executed within the test method. In this case the 1st case Mocks container has only two mocks.

I would submit a PR, but I feel that I would have to investigate the internals of Mockery and I'm with good pieces of work to do at the moment. Please help :)

@MiguelAlcaino MiguelAlcaino changed the title Created mocks and dataProvider 💥 Created mocks + call count expectations + dataProvider = 💥 Oct 13, 2022
@mfn
Copy link
Contributor

mfn commented Oct 14, 2022

Going out on a limb and saying "this is expected" given the knowledge I am aware of.

I related this to the behaviour also how phpunit (!) handles the data provider: they are also all executed before the test suite is even started, even before your first setUp is executed.
This is necessary for phpunit so it "knows" upfront how many tests it will run.

Your code being run in a data provider creates a "side effect" before any kind of test is even set up.

People run into the same problem, unrelated to Mockery, if they use the Carbon time library and use setTestNow() and setUp() but also use Carbon::now() in the data provider and then realize, by the time the data provider is run, setTestNow() was not yet called.
Same solution: you need to figure out a way to "defer" the execution into the test runtime. Either a closure, as you figured, or you include just some data facts for you to execute the code in the test.

Curious to hear what others know about this scenario!

@MiguelAlcaino
Copy link
Author

MiguelAlcaino commented Oct 14, 2022

Hi @mfn . I don't think this is an expected behaviour. Coming from mocking with prophecy and directly with phpUnit, this was never the case. Check the following pieces of code, which do exacly the same as my previous one, but with Prophecy and PhpUnit mocks:

With Prophecy:

<?php

declare(strict_types=1);

namespace Malcaino\MockeryBug\Test\Unit;

use PHPUnit\Framework\TestCase;
use Prophecy\PhpUnit\ProphecyTrait;

class OtherA
{
    public function getFirstNumber(): int
    {
        return 2;
    }
}

class OtherB
{
    public function getSecondNumber(): int
    {
        return 3;
    }
}

class MyOtherService
{
    private OtherA $classA;
    private OtherB $classB;

    public function __construct(OtherA $classA, OtherB $classB)
    {
        $this->classA = $classA;
        $this->classB = $classB;
    }

    public function sumValues(): int
    {
        return $this->classA->getFirstNumber() + $this->classB->getSecondNumber();
    }
}

class WithProphecyTest extends TestCase
{
    use ProphecyTrait;

    /**
     * @dataProvider myDataProvider
     */
    public function testMySuperFunctionality(OtherA $aInstance, OtherB $bInstance, int $expectedResult): void
    {
        $service = new MyOtherService($aInstance, $bInstance);
        $result  = $service->sumValues();

        self::assertEquals($expectedResult, $result);
    }

    public function myDataProvider(): array
    {
        return [
            '1st case' => [
                $this->createAMock(2),
                $this->createBMock(3),
                5,
            ],
            '2nd case' => [
                $this->createAMock(3),
                $this->createBMock(4),
                7,
            ],
        ];
    }

    private function createAMock(int $number): OtherA
    {
        $prophecy = $this->prophesize(OtherA::class);
        $prophecy->getFirstNumber()->willReturn($number)->shouldBeCalledOnce();

        return $prophecy->reveal();
    }

    private function createBMock(int $number): OtherB
    {
        $prophecy = $this->prophesize(OtherB::class);
        $prophecy->getSecondNumber()->willReturn($number)->shouldBeCalledOnce();

        return $prophecy->reveal();
    }
}

and with PhpUnit:

<?php

declare(strict_types=1);

namespace Malcaino\MockeryBug\Test\Unit;

use PHPUnit\Framework\TestCase;

class AnotherA
{
    public function getFirstNumber(): int
    {
        return 2;
    }
}

class AnotherB
{
    public function getSecondNumber(): int
    {
        return 3;
    }
}

class MyAnotherService
{
    private AnotherA $classA;
    private AnotherB $classB;

    public function __construct(AnotherA $classA, AnotherB $classB)
    {
        $this->classA = $classA;
        $this->classB = $classB;
    }

    public function sumValues(): int
    {
        return $this->classA->getFirstNumber() + $this->classB->getSecondNumber();
    }
}

class WithPhpUnitTest extends TestCase
{
    /**
     * @dataProvider myDataProvider
     */
    public function testMySuperFunctionality(AnotherA $aInstance, AnotherB $bInstance, int $expectedResult): void
    {
        $service = new MyAnotherService($aInstance, $bInstance);
        $result  = $service->sumValues();

        self::assertEquals($expectedResult, $result);
    }

    public function myDataProvider(): array
    {
        return [
            '1st case' => [
                $this->createAMock(2),
                $this->createBMock(3),
                5,
            ],
            '2nd case' => [
                $this->createAMock(3),
                $this->createBMock(4),
                7,
            ],
        ];
    }

    private function createAMock(int $number): AnotherA
    {
        $mock = $this->createMock(AnotherA::class);
        $mock
            ->expects($this->once())
            ->method('getFirstNumber')
            ->willReturn($number);

        return $mock;
    }

    private function createBMock(int $number): AnotherB
    {
        $prophecy = $this->createMock(AnotherB::class);
        $prophecy
            ->expects($this->once())
            ->method('getSecondNumber')
            ->willReturn($number);

        return $prophecy;
    }
}

Doing exactly the same with these other tools, it just works. I created a repo so you can run the tests easier: https://github.com/MiguelAlcaino/mockery-and-dataprovider-bug

@mfn
Copy link
Contributor

mfn commented Oct 14, 2022

Does prophecy keep internal state, like Mockery?

@MiguelAlcaino
Copy link
Author

It seems that it does. Line: vendor/phpspec/prophecy/src/Prophecy/Prophet.php:81
image

@davedevelopment
Copy link
Collaborator

PHPUnit has it's own internal way of dealing with this, I'm not sure if it's something we want to try and replicate, would probably require tying ourself in knots trying to keep up with PHPUnit internals. See PHPUnit\Framework\TestCase::registerMockObjectsFromTestArguments.

I think Prophecy isn't working at all in your examples, I don't think it is checking expectations and therefore not failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants