[11.x] Skip the number of connections transacting while testing to ru…

…n callbacks (#53377) * Skip the number of connections transacting while testing to run callbacks The testing DatabaseTransactionsManager was hardcoding the number of connections to skip as 1. In a multi-db context, it must skip the same number of connections transacting defined on the tests. * Pass the connection names array instead of the number of connections * Fix tests for the testing DatabaseTransactionsManager * Fix DatabaseTransactions trait * [11.x] Test Improvements Signed-off-by: Mior Muhammad Zaki <[email protected]> * wip Signed-off-by: Mior Muhammad Zaki <[email protected]> * wip Signed-off-by: Mior Muhammad Zaki <[email protected]> * wip Signed-off-by: Mior Muhammad Zaki <[email protected]> * Adds tests for afterCommit callbacks when using multi-dbs * Update DatabaseTransactionsManager.php --------- Signed-off-by: Mior Muhammad Zaki <[email protected]> Co-authored-by: Mior Muhammad Zaki <[email protected]> Co-authored-by: Taylor Otwell <[email protected]>
laravel · Nov 11, 2024 · 4cd413b · 4cd413b
1 parent 4500be3
commit 4cd413b
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 13 deletions.
diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php
@@ -13,9 +13,11 @@ public function beginDatabaseTransaction()
     {
         $database = $this->app->make('db');
 
-        $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager);
+        $connections = $this->connectionsToTransact();
 
-        foreach ($this->connectionsToTransact() as $name) {
+        $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager($connections));
+
+        foreach ($connections as $name) {
             $connection = $database->connection($name);
             $connection->setTransactionManager($transactionsManager);
             $dispatcher = $connection->getEventDispatcher();

diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php
@@ -6,6 +6,24 @@
 
 class DatabaseTransactionsManager extends BaseManager
 {
+    /**
+     * The names of the connections transacting during tests.
+     */
+    protected array $connectionsTransacting;
+
+    /**
+     * Create a new database transaction manager instance.
+     *
+     * @param  array  $connectionsTransacting
+     * @return void
+     */
+    public function __construct(array $connectionsTransacting)
+    {
+        parent::__construct();
+
+        $this->connectionsTransacting = $connectionsTransacting;
+    }
+
     /**
      * Register a transaction callback.
      *
@@ -31,7 +49,7 @@ public function addCallback($callback)
      */
     public function callbackApplicableTransactions()
     {
-        return $this->pendingTransactions->skip(1)->values();
+        return $this->pendingTransactions->skip(count($this->connectionsTransacting))->values();
     }
 
     /**

diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php
@@ -82,9 +82,11 @@ public function beginDatabaseTransaction()
     {
         $database = $this->app->make('db');
 
-        $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager);
+        $connections = $this->connectionsToTransact();
 
-        foreach ($this->connectionsToTransact() as $name) {
+        $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager($connections));
+
+        foreach ($connections as $name) {
             $connection = $database->connection($name);
 
             $connection->setTransactionManager($transactionsManager);

diff --git a/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php b/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php
@@ -9,8 +9,8 @@ class DatabaseTransactionsManagerTest extends TestCase
 {
     public function testItExecutesCallbacksImmediatelyIfThereIsOnlyOneTransaction()
     {
-        $testObject = new TestingDatabaseTransactionsManagerTestObject();
-        $manager = new DatabaseTransactionsManager;
+        $testObject = new TestingDatabaseTransactionsManagerTestObject;
+        $manager = new DatabaseTransactionsManager([null]);
 
         $manager->begin('foo', 1);
 
@@ -22,7 +22,7 @@ public function testItExecutesCallbacksImmediatelyIfThereIsOnlyOneTransaction()
 
     public function testItIgnoresTheBaseTransactionForCallbackApplicableTransactions()
     {
-        $manager = new DatabaseTransactionsManager;
+        $manager = new DatabaseTransactionsManager([null]);
 
         $manager->begin('foo', 1);
         $manager->begin('foo', 2);
@@ -33,7 +33,7 @@ public function testItIgnoresTheBaseTransactionForCallbackApplicableTransactions
 
     public function testCommittingDoesNotRemoveTheBasePendingTransaction()
     {
-        $manager = new DatabaseTransactionsManager;
+        $manager = new DatabaseTransactionsManager([null]);
 
         $manager->begin('foo', 1);
 
@@ -50,8 +50,8 @@ public function testCommittingDoesNotRemoveTheBasePendingTransaction()
 
     public function testItExecutesCallbacksForTheSecondTransaction()
     {
-        $testObject = new TestingDatabaseTransactionsManagerTestObject();
-        $manager = new DatabaseTransactionsManager;
+        $testObject = new TestingDatabaseTransactionsManagerTestObject;
+        $manager = new DatabaseTransactionsManager([null]);
         $manager->begin('foo', 1);
         $manager->begin('foo', 2);
 
@@ -67,17 +67,28 @@ public function testItExecutesCallbacksForTheSecondTransaction()
 
     public function testItExecutesTransactionCallbacksAtLevelOne()
     {
-        $manager = new DatabaseTransactionsManager;
+        $manager = new DatabaseTransactionsManager([null]);
 
         $this->assertFalse($manager->afterCommitCallbacksShouldBeExecuted(0));
         $this->assertTrue($manager->afterCommitCallbacksShouldBeExecuted(1));
         $this->assertFalse($manager->afterCommitCallbacksShouldBeExecuted(2));
     }
+
+    public function testSkipsTheNumberOfConnectionsTransacting()
+    {
+        $manager = new DatabaseTransactionsManager([null]);
+
+        $manager->begin('foo', 1);
+        $manager->begin('foo', 2);
+
+        $this->assertCount(1, $manager->callbackApplicableTransactions());
+    }
 }
 
 class TestingDatabaseTransactionsManagerTestObject
 {
     public $ran = false;
+
     public $runs = 0;
 
     public function handle()

diff --git a/...abase/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php b/...abase/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php
@@ -0,0 +1,57 @@
+<?php
+
+namespace Illuminate\Tests\Integration\Database;
+
+use Illuminate\Support\Facades\DB;
+use Orchestra\Testbench\Attributes\WithConfig;
+
+use function Orchestra\Testbench\artisan;
+
+#[WithConfig('database.connections.second', ['driver' => 'sqlite', 'database' => ':memory:', 'foreign_key_constraints' => false])]
+class EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest extends EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest
+{
+    /** {@inheritDoc} */
+    protected function connectionsToTransact()
+    {
+        return [null, 'second'];
+    }
+
+    /** {@inheritDoc} */
+    protected function afterRefreshingDatabase()
+    {
+        artisan($this, 'migrate', ['--database' => 'second']);
+    }
+
+    public function testAfterCommitCallbacksAreCalledCorrectlyWhenNoAppTransaction()
+    {
+        $called = false;
+
+        DB::afterCommit(function () use (&$called) {
+            $called = true;
+        });
+
+        $this->assertTrue($called);
+    }
+
+    public function testAfterCommitCallbacksAreCalledWithWrappingTransactionsCorrectly()
+    {
+        $calls = [];
+
+        DB::transaction(function () use (&$calls) {
+            DB::afterCommit(function () use (&$calls) {
+                $calls[] = 'first transaction callback';
+            });
+
+            DB::connection('second')->transaction(function () use (&$calls) {
+                DB::connection('second')->afterCommit(function () use (&$calls) {
+                    $calls[] = 'second transaction callback';
+                });
+            });
+        });
+
+        $this->assertEquals([
+            'second transaction callback',
+            'first transaction callback',
+        ], $calls);
+    }
+}
diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php
@@ -17,6 +17,7 @@ class EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest extends TestCas
      */
     protected $driver;
 
+    /** {@inheritDoc} */
     protected function setUp(): void
     {
         $this->beforeApplicationDestroyed(function () {
@@ -28,7 +29,8 @@ protected function setUp(): void
         parent::setUp();
     }
 
-    protected function getEnvironmentSetUp($app)
+    /** {@inheritDoc} */
+    protected function defineEnvironment($app)
     {
         $connection = $app['config']->get('database.default');