From 6ea1e78e1549d81cb60b921bdf2f430bb74cc8ad Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Tue, 25 Jun 2019 10:42:07 +0200 Subject: [PATCH] Improve tests --- lib/Doctrine/DBAL/Connection.php | 10 +-- tests/Doctrine/Tests/DBAL/ConnectionTest.php | 84 +++---------------- .../Tests/DBAL/Functional/ConnectionTest.php | 6 +- .../Tests/DBAL/Functional/TransactionTest.php | 25 ++---- 4 files changed, 21 insertions(+), 104 deletions(-) diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php index fce265ae6c4..28ce7f02fc3 100644 --- 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(); @@ -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; } /** diff --git a/tests/Doctrine/Tests/DBAL/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/ConnectionTest.php index b0067b4e331..b3bb4cd995a 100644 --- a/tests/Doctrine/Tests/DBAL/ConnectionTest.php +++ b/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]]; } /** diff --git a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php index 5759f019b5f..96d4e4b21a7 100644 --- 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(); } } diff --git a/tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php b/tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php index aca8055d37c..a955d25fd0c 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php +++ b/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 } }