pest icon indicating copy to clipboard operation
pest copied to clipboard

[Bug]: If the test using Mockery fails, the RefreshDatabase rollBack operation is not executed.

Open naotake51 opened this issue 1 year ago • 4 comments

What Happened

RefreshDatabase registers rollBack processing in TestCase::beforeApplicationDestroyed. https://github.com/laravel/framework/blob/10.x/src/Illuminate/Foundation/Testing/RefreshDatabase.php#L110

However, before TestCase::tearDown is called, Mockery's verify is performed and an exception is thrown, so TestCase::callBeforeApplicationDestroyedCallbacks is not called. https://github.com/pestphp/pest/blob/2.x/src/Concerns/Testable.php#L236-L238

        $this->__callClosure($afterEach, func_get_args()); // throw Mockery\Exception\InvalidCountException

        parent::tearDown(); // call TestCase::callBeforeApplicationDestroyedCallbacks

How to Reproduce

test sample

uses(RefreshDatabase::class)->in('Feature');

test('testing', function () {
    $this->partialMock(
        Anything::class,
        fn (MockInterface $mock) => $mock->shouldReceive('execute')->once()
    );

    // rollback is not called
});

Sample Repository

No response

Pest Version

2.23.2

PHP Version

8.2.10

Operation System

macOS

Notes

Operation System: using laravel sail.

naotake51 avatar Oct 20 '23 06:10 naotake51

How about doing the following so that parent::tearDown is always executed?

https://github.com/pestphp/pest/blob/2.x/src/Concerns/Testable.php#L236-L238

        try {
            $this->__callClosure($afterEach, func_get_args());
        } finally {
            parent::tearDown();

            TestSuite::getInstance()->test = null;
        }

naotake51 avatar Oct 20 '23 10:10 naotake51

I found #533 at first because that is what I ran into. Tests failing with database lock issues. Since that was closed by @nunomaduro as "not a Pest issue" I didn't look further into this project and spent time looking for issues in PHPUnit. I even tried to make a repro using PHPUnit and everything works fine there. So I was about to open a new issue here after I've created a reproduction project that can reproduce the bug with pest and found this issue.

Reproduction: https://github.com/floppy012-repros/pest-mockery-bug (note: it has two branches phpunit and pest)

PHPUnit Log: https://github.com/floppy012-repros/pest-mockery-bug/actions/runs/7224529701/job/19686003604#step:5:15 (setUp() and tearDown() are called with a failing mockery expectation)

pest Log: https://github.com/floppy012-repros/pest-mockery-bug/actions/runs/7224537897/job/19686030758#step:5:10 (only beforeEach() is called while afterEach() is not)

Floppy012 avatar Dec 15 '23 16:12 Floppy012

Just chipping in to say i'm running into the same issue on a Laravel project. Mock expectation fails, and it seems that the failing tests transaction isn't rollbacked, causing issues on subsequent tests. In my case the later test fails to get a lock on a table as the previous transaction appears to still be holding it.

I've added the snippet from https://github.com/pestphp/pest/issues/988#issuecomment-1772492837 and it seems to do the trick. My subsequent test passes rather than hanging on waiting for the table lock.

@nunomaduro Do you have an opinion on this? I don't have a good knowledge of the internals of pest, but it seems like a safe bet to always call Parent::tearDown regardless of whether __callClosure throws an exception or not?

tomb1n0 avatar Apr 04 '24 15:04 tomb1n0