TypeError when runnig test with RefreshDatabase trait

[spont4e]

• Laravel-mongodb Version: 4.0.0-rc1
• Laravel Version: 10.22.0 | 10.23.1
• PHP Version: 8.1.17
• Database Driver & Version: MongoDB driver for PHP

• php8.1-mongodb
• 1.15.0+1.11.1+1.9.2+1.7.5-1+020221209.37+debian111.gbp3b38f2

Description:

Eror when running test with RefreshDatabase trait

Steps to reproduce

1. Setup Laravel 10.23.1
2. Setup mongodb/laravel-mongodb:4.0.0-rc1
3. Create new Feature test and use Illuminate\Foundation\Testing\RefreshDatabase::class trait in Test
4. Run php artisan test

Expected behaviour

Continue without errors

Actual behaviour

TypeError: Illuminate\Database\DatabaseTransactionsManager::callbacksShouldIgnore(): Argument #1 ($transaction) must be of type Illuminate\Database\DatabaseTransactionRecord, null given, called in /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Testing/RefreshDatabase.php on line 102

Logs:

$ php artisan test

   PASS  Tests\Unit\ExampleTest
  ✓ it tests true is true                                                                                                                                                                                                      0.01s  

   FAIL  Tests\Feature\ExampleTest
  ⨯ it home page not found

   FAIL  Tests\Feature\ChatConfigurationControllerTest
  ⨯ it list chat configurations
  ⨯ it create chat configuration
  ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────  
   FAILED  Tests\Feature\ExampleTest > it home page not found                                                                                                                                                             TypeError   
  Illuminate\Database\DatabaseTransactionsManager::callbacksShouldIgnore(): Argument #1 ($transaction) must be of type Illuminate\Database\DatabaseTransactionRecord, null given, called in /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Testing/RefreshDatabase.php on line 102

  at vendor/laravel/framework/src/Illuminate/Database/DatabaseTransactionsManager.php:105
    101▕      *
    102▕      * @param  \Illuminate\Database\DatabaseTransactionRecord  $transaction
    103▕      * @return $this
    104▕      */
  ➜ 105▕     public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction)
    106▕     {
    107▕         $this->callbacksShouldIgnore = $transaction;
    108▕ 
    109▕         return $this;


  ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────  
   FAILED  Tests\Feature\ChatConfigurationControllerTest > it list chat configurations                                                                                                                                 TypeError   
  Illuminate\Database\DatabaseTransactionsManager::callbacksShouldIgnore(): Argument #1 ($transaction) must be of type Illuminate\Database\DatabaseTransactionRecord, null given, called in /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Testing/RefreshDatabase.php on line 102

  at vendor/laravel/framework/src/Illuminate/Database/DatabaseTransactionsManager.php:105
    101▕      *
    102▕      * @param  \Illuminate\Database\DatabaseTransactionRecord  $transaction
    103▕      * @return $this
    104▕      */
  ➜ 105▕     public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction)
    106▕     {
    107▕         $this->callbacksShouldIgnore = $transaction;
    108▕ 
    109▕         return $this;


  ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────  
   FAILED  Tests\Feature\ChatConfigurationControllerTest > it create chat configuration                                                                                                                                TypeError   
  Illuminate\Database\DatabaseTransactionsManager::callbacksShouldIgnore(): Argument #1 ($transaction) must be of type Illuminate\Database\DatabaseTransactionRecord, null given, called in /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Testing/RefreshDatabase.php on line 102

  at vendor/laravel/framework/src/Illuminate/Database/DatabaseTransactionsManager.php:105
    101▕      *
    102▕      * @param  \Illuminate\Database\DatabaseTransactionRecord  $transaction
    103▕      * @return $this
    104▕      */
  ➜ 105▕     public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction)
    106▕     {
    107▕         $this->callbacksShouldIgnore = $transaction;
    108▕ 
    109▕         return $this;



  Tests:    3 failed, 1 passed (1 assertions)
  Duration: 0.36s

[Treggats]

@GromNaN isn't this more like a feature request, to have support for the RefreshDatabase trait?
As the docs currently mention that this and the DatabaseTransactions trait are not supported.

[medeiroz]

I've developed a new trait called RefreshDatabase that extends Illuminate\Foundation\Testing\RefreshDatabase. Within this trait, I've overwritten the beginDatabaseTransaction method without implementing any functionality.

My trait example

<?php

namespace Tests;

use Illuminate\Foundation\Testing\RefreshDatabase as BaseRefreshDatabase;

trait RefreshDatabase
{
    use BaseRefreshDatabase {
        BaseRefreshDatabase::beginDatabaseTransaction as parentBeginDatabaseTransaction;
    }
    
    public function beginDatabaseTransaction()
    {
        return;
    }
}

I acknowledge that this approach might not be optimal, but it served as a quick workaround to resolve the error.

[alcaeus]

@medeiroz what purpose does that extra trait serve? Since the original trait itself is opt-in, wouldn't the better solution just be to not include the trait in the first place?

[medeiroz]

@medeiroz what purpose does that extra trait serve? Since the original trait itself is opt-in, wouldn't the better solution just be to not include the trait in the first place?

Run my tests without errors

[alcaeus]

@medeiroz Isn't the RefreshDatabase trait itself opt-in, i.e. you have to explicitly use it in your tests to enable its functionality?

[Treggats]

@medeiroz what purpose does that extra trait serve? Since the original trait itself is opt-in, wouldn't the better solution just be to not include the trait in the first place?

Run my tests without errors

Question; if you replace your trait with the LazilyRefreshDatabase trait. Do the tests still pass?
We use this trait and our Mongo tests all run fine.

I see this issue more as a feature request than a bug report. The docs clearly state that this trait is not supported. So the failing tests are not something that this package is responsable for.

@alcaeus yes you have to add this trait manually.

[alcaeus]

@Treggats thanks for confirming. @medeiroz since the trait is opt-in, you shouldn't have to create a separate trait to undo the RefreshDatabase functionality; you should just not include the trait as this library is not compatible with the trait.

While I get the idea of making tests "faster" by using an uncommitted database transaction, this isn't compatible with MongoDB. For one, MongoDB does not support nested transactions, so using this trait would prevent you from using transactions in your application. Second, transactions are not tied to the connection but rather to a session, so it is theoretically possible to start a transaction, run a command within the transaction, then run a command outside of the transaction (e.g. with a different read preference). While I'm not entirely sure what use cases might prompt someone to do something like that, it's not something we want to prevent for the sake of testing.