Add functional test for comit() returning false

doctrine · Jun 26, 2019 · 38e20f9 · 38e20f9
1 parent f2a0003
commit 38e20f9
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 89 deletions.
diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php
@@ -1254,8 +1254,6 @@ public function beginTransaction()
     /**
      * {@inheritDoc}
      *
-     * @return bool
-     *
      * @throws ConnectionException If the commit failed due to no active transaction or
      *                                            because the transaction was marked for rollback only.
      */
@@ -1326,8 +1324,6 @@ private function commitAll()
     /**
      * Cancels any database changes done during the current transaction.
      *
-     * @return bool
-     *
      * @throws ConnectionException If the rollback operation failed.
      */
     public function rollBack()
@@ -1336,8 +1332,6 @@ public function rollBack()
             throw ConnectionException::noActiveTransaction();
         }
 
-        $result = true;
-
         $connection = $this->getWrappedConnection();
 
         $logger = $this->_config->getSQLLogger();
@@ -1347,9 +1341,7 @@ public function rollBack()
                 $logger->startQuery('"ROLLBACK"');
             }
             $this->transactionNestingLevel = 0;
-
-            $result = $connection->rollBack();
-
+            $connection->rollBack();
             $this->isRollbackOnly = false;
             if ($logger) {
                 $logger->stopQuery();
@@ -1371,8 +1363,6 @@ public function rollBack()
             $this->isRollbackOnly = true;
             --$this->transactionNestingLevel;
         }
-
-        return $result;
     }
 
     /**

diff --git a/tests/Doctrine/Tests/DBAL/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/ConnectionTest.php
@@ -25,7 +25,6 @@
 use Doctrine\DBAL\VersionAwarePlatformDriver;
 use Doctrine\Tests\DbalTestCase;
 use Exception;
-use PDO;
 use PHPUnit\Framework\MockObject\MockObject;
 use stdClass;
 use function call_user_func_array;
@@ -299,95 +298,33 @@ public function testCommitStartsTransactionInNoAutoCommitMode() : void
     }
 
     /**
-     * @group DBAL-81
+     * @dataProvider resultProvider
      */
-    public function testCommitReturnTrue() : void
+    public function testCommitReturn(bool $expectedResult) : void
     {
-        $driverMock = $this->createMock(Driver::class);
-        $driverMock->expects($this->any())
-            ->method('connect')
-            ->will($this->returnValue(
-                $this->createMock(DriverConnection::class)
-            ));
-        $pdo = $this->createMock(PDO::class);
-        $pdo->expects($this->once())
-            ->method('commit')->willReturn(true);
-
-        $conn = new Connection(['pdo' => $pdo], $driverMock);
-
-        $conn->connect();
-        $conn->beginTransaction();
-
-        self::assertTrue($conn->commit());
-    }
-
-    /**
-     * @group DBAL-81
-     */
-    public function testCommitReturnFalse() : void
-    {
-        $driverMock = $this->createMock(Driver::class);
-        $driverMock->expects($this->any())
-            ->method('connect')
-            ->will($this->returnValue(
-                $this->createMock(DriverConnection::class)
-            ));
-        $pdo = $this->createMock(PDO::class);
-        $pdo->expects($this->once())
-            ->method('commit')->willReturn(false);
-
-        $conn = new Connection(['pdo' => $pdo], $driverMock);
+        $driverConnection = $this->createMock(DriverConnection::class);
+        $driverConnection->expects($this->once())
+            ->method('commit')->willReturn($expectedResult);
 
-        $conn->connect();
-        $conn->beginTransaction();
-
-        self::assertFalse($conn->commit());
-    }
-
-    /**
-     * @group DBAL-81
-     */
-    public function testRollackReturnTrue() : void
-    {
         $driverMock = $this->createMock(Driver::class);
         $driverMock->expects($this->any())
             ->method('connect')
-            ->will($this->returnValue(
-                $this->createMock(DriverConnection::class)
-            ));
-        $pdo = $this->createMock(PDO::class);
-        $pdo->expects($this->once())
-            ->method('rollback')->willReturn(true);
+            ->will($this->returnValue($driverConnection));
 
-        $conn = new Connection(['pdo' => $pdo], $driverMock);
+        $conn = new Connection([], $driverMock);
 
         $conn->connect();
         $conn->beginTransaction();
 
-        self::assertTrue($conn->rollback());
+        self::assertSame($expectedResult, $conn->commit());
     }
 
     /**
-     * @group DBAL-81
+     * @return bool[][]
      */
-    public function testRollackReturnFalse() : void
+    public function resultProvider() : array
     {
-        $driverMock = $this->createMock(Driver::class);
-        $driverMock->expects($this->any())
-            ->method('connect')
-            ->will($this->returnValue(
-                $this->createMock(DriverConnection::class)
-            ));
-        $pdo = $this->createMock(PDO::class);
-        $pdo->expects($this->once())
-            ->method('rollback')->willReturn(false);
-
-        $conn = new Connection(['pdo' => $pdo], $driverMock);
-
-        $conn->connect();
-        $conn->beginTransaction();
-
-        self::assertFalse($conn->rollback());
+        return [[true], [false]];
     }
 
     /**

diff --git a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php
@@ -63,7 +63,7 @@ public function testTransactionNestingBehavior() : void
             }
             self::assertTrue($this->connection->isRollbackOnly());
 
-            self::assertTrue($this->connection->commit()); // should throw exception
+            $this->connection->commit(); // should throw exception
             $this->fail('Transaction commit after failed nested transaction should fail.');
         } catch (ConnectionException $e) {
             self::assertEquals(1, $this->connection->getTransactionNestingLevel());
@@ -93,7 +93,7 @@ public function testTransactionNestingBehaviorWithSavepoints() : void
                 throw new Exception();
                 $this->connection->commit(); // never reached
             } catch (Throwable $e) {
-                self::assertTrue($this->connection->rollBack());
+                $this->connection->rollBack();
                 self::assertEquals(1, $this->connection->getTransactionNestingLevel());
                 //no rethrow
             }
@@ -107,7 +107,7 @@ public function testTransactionNestingBehaviorWithSavepoints() : void
             $this->connection->commit(); // should not throw exception
         } catch (ConnectionException $e) {
             $this->fail('Transaction commit after failed nested transaction should not fail when using savepoints.');
-            self::assertTrue($this->connection->rollBack());
+            $this->connection->rollBack();
         }
     }
 
@@ -191,8 +191,9 @@ public function testTransactionBehaviour() : void
         try {
             $this->connection->beginTransaction();
             self::assertEquals(1, $this->connection->getTransactionNestingLevel());
-            self::assertTrue($this->connection->commit());
+            $this->connection->commit();
         } catch (Throwable $e) {
+            $this->connection->rollBack();
             self::assertEquals(0, $this->connection->getTransactionNestingLevel());
         }
 

diff --git a/tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php b/tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php
@@ -0,0 +1,39 @@
+<?php
+
+namespace Doctrine\Tests\DBAL\Functional;
+
+use Doctrine\DBAL\Platforms\MySqlPlatform;
+use Doctrine\Tests\DbalFunctionalTestCase;
+use function sleep;
+
+class TransactionTest extends DbalFunctionalTestCase
+{
+    protected function setUp() : void
+    {
+        parent::setUp();
+
+        if ($this->connection->getDatabasePlatform() instanceof MySqlPlatform) {
+            return;
+        }
+
+        $this->markTestSkipped('Restricted to MySQL.');
+    }
+
+    protected function tearDown() : void
+    {
+        $this->resetSharedConn();
+
+        parent::tearDown();
+    }
+
+    public function testCommitFalse() : void
+    {
+        $this->connection->query('SET SESSION wait_timeout=1');
+
+        $this->assertTrue($this->connection->beginTransaction());
+
+        sleep(2); // during the sleep mysql will close the connection
+
+        $this->assertFalse(@$this->connection->commit()); // we will ignore `MySQL server has gone away` warnings
+    }
+}