laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.52k stars 11.02k forks source link

laravel parallel testing with multiple databases - getConnection() #47833

Closed davidcorreia closed 1 year ago

davidcorreia commented 1 year ago

Laravel Version

9.52.6

PHP Version

8.1.7

Database Driver & Version

mysql/mysql-server:8.0.28

Description

By using the parallel testing with a multiple database setup, after getting the main_db_test_N & second_db_test_N (where the N is the number os the process) every assertDatabaseHas fails

$this->assertDatabaseHas('second_database_table_name', [ 'key' => 'value' ], 'second_database_connection_name');

I believe the problem is with the getConnection()

// "mysql" is the default connection name in config/database.php
dd(
    'mysql : ' . $this->getConnection('mysql')->getDatabaseName(),  
    'second_db : ' . $this->getConnection('second_database_connection_name')->getDatabaseName(),
);

Output: 
================
"mysql : main_db_test_1"
"second_db : second_database".           <-------------- there is no "_test_N"

also, if we access this information using the models

dd(
    (new MainDbTableModel)->getConnectionName(),
    (new SecondDbTableModel)->getConnectionName()
);

Output:
================ 
null
"second_database_connection_name" 

Steps To Reproduce

step 1) have more than 1 database configured in config/database.php

'connections' => [
    'mysql' => [ ... ]
    'second_database_connection_name' => [ ... ]

step 2) override ParallelTestingServiceProvider to setup and seed each database, explained here https://sarahjting.com/blog/laravel-paratest-multiple-db-connections or copy-paste code the app/Providers/DatabasesServiceProvider.php

  use Illuminate\Support\Facades\Artisan;
  use Illuminate\Support\Facades\ParallelTesting;
  use Illuminate\Support\Facades\Schema;
  use Illuminate\Support\ServiceProvider;
  use Illuminate\Testing\Concerns\TestDatabases;

  class DatabasesServiceProvider extends ServiceProvider
  {
      use TestDatabases;

      protected array $parallelConnections = ['second_database_connection_name']; // Connection "mysql" is alredy done by default

      public function boot()
      {
          if ($this->app->runningInConsole()) {
              $this->bootTestDatabase();
          }
      }

      protected function bootTestDatabase()
      {
          ParallelTesting::setUpProcess(function () {
              if (ParallelTesting::option('recreate_databases')) {
                  foreach ($this->parallelConnections as $connection) {
                      Schema::connection($connection)
                          ->dropDatabaseIfExists(
                              $this->testDatabaseOnConnection($connection)
                          );
                  }
              }
          });

          ParallelTesting::tearDownProcess(function () {
              if (ParallelTesting::option('drop_databases')) {
                  foreach ($this->parallelConnections as $connection) {
                      Schema::connection($connection)
                          ->dropDatabaseIfExists(
                              $this->testDatabaseOnConnection($connection)
                          );
                  }
              }
          });

          ParallelTesting::setUpTestDatabase(function () {
              Artisan::call('db:seed');
              $allCreated = [];
              foreach ($this->parallelConnections as $k => $connection) {
                  $this->usingConnection($connection, function ($connection) use (&$allCreated) {
                      $database = config("database.connections.{$connection}.database");
                      [$testDatabase, $created] = $this->ensureTestDatabaseExists($database);
                      $this->switchToDatabase($testDatabase);

                      if ($created) {
                          $allCreated[] = [$connection, $testDatabase];
                          //Artisan::call('command to seed database'); // this is commented because it's not necessary for now
                      }
                  });
              }
              $this->switchToDatabase(config("database.connections." . config('database.default') . ".database"));
          });
      }

      protected function usingConnection(string $connection, \Closure $callable): void
      {
          $originalConnection = config("database.default");

          try {
              config()->set("database.default", $connection);
              $callable($connection);
          } finally {
              config()->set("database.default", $originalConnection);
          }
      }

      protected function testDatabaseOnConnection(string $connection): string
      {
          return $this->testDatabase(config("database.connections.{$connection}.database"));
      }
  }

add import in config/app.php

  'providers' => [
        ...
        App\Providers\DatabasesServiceProvider::class,

step 3) run a single test with parallel with the following code

dd(
    'mysql : ' . $this->getConnection('mysql')->getDatabaseName(),  
    'second_db : ' . $this->getConnection('second_database_connection_name')->getDatabaseName(),
);

command: php artisan test --parallel --processes=2

driesvints commented 1 year ago

override ParallelTestingServiceProvider

I'm sorry but we can't provide support for things you've modified in the framework. If you can provide us with a repo that reproduces this in a fresh Laravel app we can have a look.

laravel new bug-report --github="--public"
davidcorreia commented 1 year ago

@driesvints thank you for the reply.

i have just tested only with step 1 & 3 (without the override ParallelTestingServiceProvider) and got the same result. can you please take a look?

driesvints commented 1 year ago

Can you please provide a repo?

davidcorreia commented 1 year ago

Hi @driesvints i have created a fresh project installation repo https://github.com/davidcorreia/lara-test

please run:

  1. docker-compose up -d
  2. docker-compose exec app composer install
  3. docker-compose exec app php artisan test --parallel --processes=2

somehow i was expecting to see something like db1_test_1

Output:
================
"mysql : db1" // tests/Feature/ExampleTest.php:19
"second_db : db2" // tests/Feature/ExampleTest.php:19

driesvints commented 1 year ago

Can you please recreate the repo with the command I provided? All of your changes are squashed into the same commit making it impossible for me to see what you changed.

davidcorreia commented 1 year ago

of course, my bad. here https://github.com/davidcorreia/lara-9-test

NOTE: the command "laravel" is not available. am i missing something?

Mdhesari commented 1 year ago

@davidcorreia

why do you expect to see db1_test_1 when your db name in your configuration is set to db2?!

davidcorreia commented 1 year ago

@Mdhesari because when using parallel with databases is expected (and needed) for each process to work with a dedicated instance of the database. https://laravel.com/docs/10.x/testing#parallel-testing-and-databases

EDIT: the expected value db1_test_1 is expected on the $this->getConnection('mysql')->getDatabaseName() not on the $this->getConnection('second_database_connection_name')->getDatabaseName()

Mdhesari commented 1 year ago

@davidcorreia

by default if we run : php artisan test --parallel --recreates-database

it checks your default db config, for example my default config name is laravel so it is going to create some databases if not exists based on process count I have, in this case I have 8 processes :

laravel_test_1 laravel_test_2 laravel_test_3 ...

So it's using multiple database at the same time, I'm not sure what are you trying to achieve, is it possible to let me know?

Mdhesari commented 1 year ago

Also I've tried DatabaseTransactions with the default parallel configuration and it works smoothly

davidcorreia commented 1 year ago

@Mdhesari it did not work on the fresh public repo (https://github.com/davidcorreia/lara-9-test) with the command docker-compose run app php artisan test --parallel --processes=4 --recreate-databases

$ docker-compose run app php artisan test --parallel --processes=4 --recreate-databases ParaTest v6.10.0 upon PHPUnit 9.6.10 by Sebastian Bergmann and contributors.

. In WorkerCrashedException.php line 41:

The command "PARATEST='1' TEST_TOKEN='2' UNIQUE_TEST_TOKEN='2_64c663e1af3a4' '/var/www/html/vendor/phpunit/phpunit/phpunit' '--configuration' '/var/www/html/phpunit.xml' '--no-logging' '--no-coverage' '--printer' 'ParaTest\Runners
\PHPUnit\Worker\NullPhpunitPrinter' '--log-junit' '/tmp/PT_sO7k6P' '/var/www/html/tests/Feature/ExampleTest.php'" failed.

Exit Code: 1(General error)

Working directory: /var/www/html

Output:
================
"mysql : db1" // tests/Feature/ExampleTest.php:19
"second_db2 : db2" // tests/Feature/ExampleTest.php:19

davidcorreia commented 1 year ago

for more details on why/how i think the problem is with getConnection()

the application i'm working on has 2 databases. The main database works just fine (aka: it creates a data by each process with the posfix _test_N in the sql server). With the override of ParallelTestingServiceProvider the second database also shows the expected behavior of creating the posfix _test_N in the sql server.

HOWEVER every assertDatabaseHas with the third argument of the connection name from the second database fails.

https://github.com/laravel/framework/blob/10.x/src/Illuminate/Foundation/Testing/Concerns/InteractsWithDatabase.php#L27

$this->assertDatabaseHas('table_name', [ 'key' => 'value' ], 'second_database_connection_name');

in my test i can see that the second database, by accessing it by getConnection() it does not show the postfix _test_N. (Somehow it does not show on the fresh repo as well. No idea why....)

nunomaduro commented 1 year ago

The parallel testing setup provided by Laravel supports only one database connection (same goes for cache). If you wish to have parallel testing with two database connections, then it is something you will have to manage yourself - as we simply don't support that feature yet.

Of course, you can always try one of the following channels, as you may find some other people who already have found a solution for this:

tiagoraposeiraff commented 1 year ago

@nunomaduro I've been watching this thread because I'm also having issue with getConnection/getConnectionName. regardless of being able to support one or more db connection, shouldn't those methods always return the correct connection name in use for that test? Meaning, if it is a parallel test, it should append the process id, right? This might not be necessarily a problem with the testing bit, but rather those 2 methods and how they get the actual name.

Let me know your thoughts, ty!