PHPUnit runtime more than doubled v11.36.1 and v11.37

[trovster]

Laravel Version

11.37

PHP Version

8.2.26

Database Driver & Version

No response

Description

My test suite using PHPUnit changed from taking ~10 minutes to over 25 minutes.

I think it could be related to this change; #53997

Steps To Reproduce

Run a large PHPUnit test suite using v11.36.1 and then compare the runtime when running the same test suite using v11.37.

[SjorsO]

How are your tests refreshing the database? Are you just using RefreshDatabase trait, or are you using a custom solution?

#53997 will only affect your test suite runtime if your tests are committing the transaction started by RefreshDatabase (which also means the RefreshDatabase trait won't work correctly). Do you maybe have a terminating middleware that commits all database transactions?

[trovster]

I am just using the RefreshDatabase trait. I don't have any database transactions in my main code.

After the slowdown I investigated solutions for speeding up test suites which recommended adding transactions for setting up data in tests. For example;

public function createMusic(): void
{
    DB::beginTransaction();

    Listen::factory()->count(20)->enabled()->recent()->create();
    Track::factory()->count(5)->withListens()->enabled()->create();
    Artist::factory()->count(5)->enabled()->create();

    DB::commit();
}

[SjorsO]

Are you maybe calling something like Artist::truncate() after each test? Truncating tables would also cause this issue (but it would also cause your RefreshDatabase trait to not work correctly, which is what #53997 tries to solve).

Also, I've the DB::beginTransaction() and DB::commit(); you mentioned, that doesn't cause any issues with the changes made to RefreshDatabase (because DB::commit() commits the most recent transaction, leaving the transaction started by RefreshDatabase in tact).

[trovster]

Yes, similar to createMusic() I also have resetMusic(), which resets the data to keep everything consistent.

public function resetMusic(): void
{
    Schema::disableForeignKeyConstraints();
    DB::beginTransaction();

    User::query()->truncate();
    Listen::query()->truncate();
    Track::query()->truncate();
    Artist::query()->truncate();
    Album::query()->truncate();
    AlbumTrack::query()->truncate();

    DB::commit();
    Schema::enableForeignKeyConstraints();
}

[SjorsO]

I'm not sure what you mean by "to keep everything consistent". It looks like you're manually resetting the database, but that's what RefreshDatabase already does for you after each test, you don't have to reset the data yourself.

I think you can remove your resetMusic() method and your tests will still work. If you somehow still need this method, you can change all calls from ->truncate() to ->delete(), that should resolve this issue.

For more explanation, calling truncate commits all active database transactions, including the one started by RefreshDatabase. Without the transaction the RefreshDatabase trait can't clean up your database, meaning that your database will remember data between tests.

[trovster]

I updated all the ->truncate() calls in my tests to ->delete() and the tests were still slow. I then updated the reset methods to simple return; at the top – so they're not used – and the tests, again, were slow.

Running the tests with the return; also caused lots of errors :|

[SjorsO]

If your tests are still slower than before after changing those ->truncate() calls to ->delete(), then you must still be committing the transaction started by RefreshDatabase somewhere. Do you maybe have other truncate calls in your code base? Or maybe an extra DB::commit() somewhere? Or are you maybe making CREATE TABLE or ALTER TABLE queries manually?

[trovster]

Changing to ->delete() caused some errors in the tests, mainly down to duplicate keys. I only have DB::commit() in my tests, as per the example above. ->truncate() was only called in the tests, which I replaced with ->delete() but cause the errors.

I do have a couple of migrations which use ALTER TABLE, to update ENUM values;

DB::statement("
    ALTER TABLE `thing`
    MODIFY COLUMN `type` ENUM('person', 'pet', 'bike', 'thing', 'place')
    DEFAULT 'thing'
");

[SjorsO]

It sounds like some of your tests depend on the database table's auto-increment starting at 1. This normally doesn't work when you use RefreshDatabase because the auto-increment doesn't reset between tests. However, in your case, auto-increments are being reset to 1 because you manually truncate each table.

Truncating tables causes the RefreshDatabase trait to not work correctly, which is exactly what #53997 fixes. However, the trait not working correctly isn't causing issues for you because you're manually truncating all your tables (as in, you're manually doing the work that RefreshDatabase normally does).

I don't really have a solution for you except for not using truncate, and updating the tests that rely on hard-coded auto-increment values.

[trovster]

I am not using "hard-coded auto-increment" values… I am having issues with duplicate keys. I would need to spend a lot more time researching these specific tests and running them without the ->truncate() and see if I can refactor the tests to make them pass.

From my initial tests, when I use ->delete() and the latest version on Laravel, the tests are still slow.