Improve tests

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();
@@ -1348,7 +1342,7 @@ public function rollBack()
             }
             $this->transactionNestingLevel = 0;
 
-            $result = $connection->rollBack();
+            $connection->rollBack();
 
             $this->isRollbackOnly = false;
             if ($logger) {
@@ -1371,8 +1365,6 @@ public function rollBack()
             $this->isRollbackOnly = true;
             --$this->transactionNestingLevel;
         }
-
-        return $result;
     }
 
     /**

tests/Doctrine/Tests/DBAL/ConnectionTest.php

@@ -299,95 +299,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]];
     }
 
     /**

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();
         }
     }
 

tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php

@@ -2,10 +2,9 @@
 
 namespace Doctrine\Tests\DBAL\Functional;
 
-use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Platforms\MySqlPlatform;
 use Doctrine\Tests\DbalFunctionalTestCase;
-use Throwable;
-use function in_array;
+use PHPUnit\Framework\Error\Warning;
 use function sleep;
 
 class TransactionTest extends DbalFunctionalTestCase
@@ -14,7 +13,7 @@ protected function setUp() : void
     {
         parent::setUp();
 
-        if (in_array($this->connection->getDatabasePlatform()->getName(), ['mysql'])) {
+        if ($this->connection->getDatabasePlatform() instanceof MySqlPlatform) {
             return;
         }
 
@@ -32,22 +31,10 @@ public function testCommitFalse() : void
     {
         $this->connection->query('SET SESSION wait_timeout=1');
 
-        $table = new Table('foo');
-        $table->addColumn('id', 'integer');
-        $table->setPrimaryKey(['id']);
+        $this->assertTrue($this->connection->beginTransaction());
 
-        $this->connection->getSchemaManager()->createTable($table);
+        sleep(2); // during the sleep mysql will close the connection
 
-        self::assertEquals('foo', $table->getName());
-        try {
-            $this->assertTrue($this->connection->beginTransaction());
-            $this->connection->executeUpdate('INSERT INTO foo (id) VALUES(1)');
-
-            sleep(2); // during the sleep mysql will close the connection
-
-            $this->assertFalse(@$this->connection->commit()); // we will ignore `MySQL server has gone away` warnings
-        } catch (Throwable $exception) {
-            $this->fail('Is the PHP/PDO bug https://bugs.php.net/bug.php?id=66528 fixed? Normally no exception is thrown.');
-        }
+        $this->assertFalse(@$this->connection->commit()); // we will ignore `MySQL server has gone away` warnings
     }
 }