[8.x] Fix error PDOException: There is no active transaction

[hakobyansen]

Got broken integration tests after an upgrade to Laravel 8.24.0 and PHP 8.0.1.

Tests are throwing error PDOException: There is no active transaction on DB refresh.

Tests are throwing error when the Illuminate\Foundation\Testing\RefreshDatabase trait used.

A condition added to the code in this pull request fixes the error described above.

[taylorotwell]

Share your entire test.

[bakerkretzmar]

I ran into this last week too, it happened if I tried to drop or create tables manually with the Schema facade.

This test passes on PHP 7.4.13 but causes that error on PHP 8.0.0 (fresh install of latest Laravel, running tests on a new empty MySQL 8 database, nothing else touched):

<?php

namespace Tests\Feature;

use Illuminate\Database\Schema\Blueprint;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;
use Tests\TestCase;

class ExampleTest extends TestCase
{
    use RefreshDatabase;

    /** @test */
    public function can_use_schema()
    {
        Schema::dropIfExists('test');
        Schema::create('test', function (Blueprint $table) {
            $table->string('name');
        });

        DB::table('users')->insert(['name' => 'Jacob', 'email' => 'jacob@example.com', 'password' => bcrypt('password')]);

        $this->assertSame('Jacob', DB::table('users')->first()->name);
    }
}

The complete error output:

Example (Tests\Feature\Example)
 ✘ Can use schema
   ┐
   ├ PDOException: There is no active transaction
   │
   ╵ /Users/jacob/Code/no-active-transaction/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php:275
   ╵ /Users/jacob/Code/no-active-transaction/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php:250
   ╵ /Users/jacob/Code/no-active-transaction/vendor/laravel/framework/src/Illuminate/Foundation/Testing/RefreshDatabase.php:117
   ╵ /Users/jacob/Code/no-active-transaction/vendor/laravel/framework/src/Illuminate/Foundation/Testing/TestCase.php:237
   ╵ /Users/jacob/Code/no-active-transaction/vendor/laravel/framework/src/Illuminate/Foundation/Testing/TestCase.php:153
   ┴

[crynobone]

This related to implicit commit and has been described in https://laravel.com/docs/8.x/database#implicit-commits-in-transactions

Before PHP 8, while there's no active transaction PDO doesn't throw any error but this not the same in PHP 8.

The example provided by @bakerkretzmar will only solved via this PR in some scenario, imagine the flow the running the test:

1. ExampleTest::setUp()
2. Illuminate\Foundation\Testing\TestCase::setUpTraits()
3. Illuminate\Foundation\Testing\RefreshDatabase::refreshDatabase()
4. Illuminate\Foundation\Testing\RefreshDatabase::refreshTestDatabase()
5. Illuminate\Foundation\Testing\RefreshDatabase::beginDatabaseTransaction() <-- PR changes
6. Example::can_use_schema() <-- Run schema migration which execute implicit commit and cause PDO to not have an active transaction.

See #35380, #35161

[hakobyansen]

@taylorotwell here is an entire test, actually, one of them. All tests that use the RefreshDatabase trait are failing with PHP 8, but passing with PHP 7.4.

<?php

namespace Tests\Integration\Repositories;

use App\Models\Merchant;
use App\Repositories\MerchantRepository;
use Illuminate\Foundation\Testing\DatabaseMigrations;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Tests\TestCase;

class MerchantRepositoryTest extends TestCase
{
	use DatabaseMigrations, RefreshDatabase;

	private MerchantRepository $_MerchantRepo;
	private Merchant $_MockMerchant;

	protected function setUp(): void
	{
		parent::setUp();

		$this->_MerchantRepo = new MerchantRepository(new Merchant());
		$this->_MockMerchant = Merchant::factory()->make();
	}

	public function testStore()
	{
		$Merchant = $this->_MerchantRepo->store($this->_MockMerchant->toArray());

		$this->assertEquals($this->_MockMerchant['name'], $Merchant->getName());
		$this->assertEquals($this->_MockMerchant['password'], $Merchant->getPassword());
		$this->assertEquals($this->_MockMerchant['description'], $Merchant->getDescription());
		$this->assertEquals($this->_MockMerchant['currency'], $Merchant->getCurrency());
	}

	public function testGetByName()
	{
		$this->_MerchantRepo->store($this->_MockMerchant->toArray());

		$Merchant = $this->_MerchantRepo->getByName($this->_MockMerchant['name']);

		$this->assertEquals($this->_MockMerchant['name'], $Merchant->getName());
		$this->assertEquals($this->_MockMerchant['password'], $Merchant->getPassword());
		$this->assertEquals($this->_MockMerchant['description'], $Merchant->getDescription());
		$this->assertEquals($this->_MockMerchant['currency'], $Merchant->getCurrency());

		$Merchant = $this->_MerchantRepo->getByName('there_is_no_store_with_this_name!~!');

		$this->assertNull($Merchant);
	}
}

[bakerkretzmar]

@crynobone thanks, that's really interesting. It sounds like this error is the expected behaviour, and what we're trying to do in these tests actually isn't really supported by MySQL at all? Good to know 😅

@C0d3B0t do those tests you shared run with just DatabaseMigrations or RefreshDatabase, not both? RefreshDatabase also runs your migrations so you shouldn't need to use both.

[hakobyansen]

@bakerkretzmar you're awesome, thank you! The error is due to redundant trait usage.

[hakobyansen]

Maybe there can be enhancements made for better exception handling, anyway, this PR doesn't solve anything, so it should be closed probably. Thank you all!

[juanparati]

I can face the same issue and I resolved it replacing the "RefreshDatabase" trait by the "DatabaseMigrations" trait.

I observed that by the default the "artisan make:test" command creates feature test that uses "use Illuminate\Foundation\Testing\RefreshDatabase" instead of "use Illuminate\Foundation\Testing\DatabaseMigrations;"

I am using Laravel 8.27.0 (artisan --version).

Steps to reproduce:

• Execute "artisan make:test FooBar"

[cAstraea]

Your fix seems to have worked for me ? I don't understand what's wrong though.
Should we not use RefreshDatabase anymore in our tests ?
I'm using it to get a clean slate and in the setUp function I'm running migrate:fresh, passport:install and db:seed

For now I have replaced the RefreshDatabase with a \Artisan::call('db:wipe'); call in the setUp function which seems to achieve my goals but not sure how correct this is.