[11.x] Re-refresh the database if the RefreshDatabase transaction was committed

[SjorsO]

The RefreshDatabase trait:

• Runs migrate:fresh before the first test
• Opens a database transaction before each test
• Rolls back that transaction after each test

The issue is that it is possible to accidentally commit the transaction started by RefreshDatabase. For example, truncating a table automatically commits any active transactions. If a test commits the transaction, all database records created by that test won't be rolled back, causing each subsequent test to have a dirty database.

This is especially frustrating to debug because the test that commits the transaction passes, but tests after that fail unpredictably depending on the type of assertions they do. The logical place to start debugging the issue is with the failing tests, but those have nothing to do with the problem.

This PR solves this issue by refreshing the database if you (accidentally) commit the database transaction started by the RefreshDatabase trait.

To reproduce this issue, use MySQL, then run the following tests:

class ExampleTest extends TestCase
{
    use RefreshDatabase; // or LazilyRefreshDatabase

    public function test_the_first_test()
    {
        User::truncate();

        User::factory()->create();

        $this->expectNotToPerformAssertions();
    }

    public function test_the_second_test()
    {
        // This test passes if you run it in isolation, but it fails if you run all tests
        $this->assertSame(0, User::count());
    }
}

[taylorotwell]

Is it possible to add a test for this in some way?

[chrisnetonline]

I think there might be an issue introduced here. Post upgrade I am seeing the following error:

Error: Call to a member function inTransaction() on null
/var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Testing/RefreshDatabase.php:112
/var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/InteractsWithTestCaseLifecycle.php:285
/var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/InteractsWithTestCaseLifecycle.php:113
/var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Testing/TestCase.php:66

[SjorsO]

I think there might be an issue introduced here. Post upgrade I am seeing the following error:

Error: Call to a member function inTransaction() on null
/var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Testing/RefreshDatabase.php:112
/var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/InteractsWithTestCaseLifecycle.php:285
/var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/InteractsWithTestCaseLifecycle.php:113
/var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Testing/TestCase.php:66

Which database are you using for your tests?

[chrisnetonline]

@SjorsO MySQL Docker Image 8.0.32-1.2.11-server

[SjorsO]

@SjorsO MySQL Docker Image 8.0.32-1.2.11-server

Could you manually edit the RefreshDatabase trait in your project to see if the code below solves the issue?

if ($connection->getPdo() && ! $connection->getPdo()->inTransaction()) {
    RefreshDatabaseState::$migrated = false;
}

If it does, I'll open up a PR to get this breaking change fixed (although I have no clue why ->getPdo() would be null in your case).

[chrisnetonline]

@SjorsO MySQL Docker Image 8.0.32-1.2.11-server

Could you manually edit the RefreshDatabase trait in your project to see if the code below solves the issue?

if ($connection->getPdo() && ! $connection->getPdo()->inTransaction()) {
    RefreshDatabaseState::$migrated = false;
}

If it does, I'll open up a PR to get this breaking change fixed (although I have no clue why ->getPdo() would be null in your case).

That does indeed fix the issue.

[chrisnetonline]

I can't share a ton of code, but our base "Feature" test case uses the DatabaseTransactions trait and then we supply a list of database connections that we have transactions on. The app has multiple connections so maybe that is somehow related:

    protected array $connectionsToTransact = [
        DatabaseConnection::CONNECTION1,
        DatabaseConnection::CONNECTION2,
        DatabaseConnection::CONNECTION3,
    ];

[SjorsO]

I've opened a PR to fix the breaking change: #54075

You mentioned that you're using DatabaseTransactions, but that trait is separate from RefreshDatabase, so does that mean you're using both?

[chrisnetonline]

@SjorsO All of our feature tests use a base class that includes DatabaseTransactions. This one test class happened to also include RefreshDatabase, so yes, in this case, both traits were in use. The RefreshDatabase trait didn't actually need to be used so we have since removed it.

[27pchrisl]

@chrisnetonline I had the same issue, our system uses https://github.com/archtechx/tenancy. It might be because connectionsToTransact() returns multiple database connections that are defined and could support transactions, but one was never actually used and so pdo was never initialized for that connection. The change in #54075 fixed the issue for me too.

[27pchrisl]

Woops I actually meant @SjorsO ! ;)

[oranges13]

We are using sqlite in our test cases and have the same Error: Call to a member function inTransaction() on null glad to see it fixed above, but adding just in case someone else finds themselves here