Skip to content

Commit

Permalink
Use self::assert*() instead of $this->assert*()
Browse files Browse the repository at this point in the history
  • Loading branch information
@lcobucci
lcobucci committed Sep 9, 2017
1 parent 48fa395 commit 62eb475
Show file tree
Hide file tree
Showing 154 changed files with 2,572 additions and 2,572 deletions.
10 changes: 5 additions & 5 deletions tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public function testShouldUseTheGivenCacheKeyIfPresent()
$connectionParams
);

$this->assertEquals(self::CACHE_KEY, $cacheKey, 'The returned cache key should match the given one');
self::assertEquals(self::CACHE_KEY, $cacheKey, 'The returned cache key should match the given one');

This comment has been minimized.

Sign in to view

Copy link
@PowerKiKi

PowerKiKi Sep 20, 2017 •
edited
Loading

Contributor

Would you mind explaining the rationale behind this change ?

The methods are indeed static and can be called statically, but PHPUnit author recommends $this->assert, the official documentation use $this->assert everywhere and a quick search on GitHub shows that there is huge majority using $this->assert with ~45 millions occurences against ~5 millions for self::assert.

So what made you decide that it is no longer appropriate to follow recommendations and the majority ?

Also do you plan on changing other projects, such as doctrine/doctrine2, for consistency ?

This comment has been minimized.

Sign in to view

Copy link
@Majkl578

Majkl578 Sep 20, 2017

Contributor

Also see https://phpunit.de/manual/current/en/appendixes.assertions.html#appendixes.assertions.static-vs-non-static-usage-of-assertion-methods.
There is no good/bad or recommended form, either is correct.

Simply put, since the methods are declared as static, it doesn't feel correct to call them in object context. (This is something that should have been banned altogether in my opinion, ages ago.)

Also do you plan on changing other projects, such as doctrine/doctrine2, for consistency ?

Some of the projects already use it, i.e. Common, ORM should eventually too.

This comment has been minimized.

Sign in to view

Copy link
@PowerKiKi

PowerKiKi Sep 20, 2017

Contributor

Thank you for the quick reply. I tend to agree with you, but it would have been even easier to push for that change, against the majority, if we had some technical reasons in our favor.

This comment has been minimized.

Sign in to view

Copy link
@Ocramius

Ocramius via email Sep 20, 2017

Member
}

public function testShouldGenerateAnAutomaticKeyIfNoKeyHasBeenGiven()
Expand All @@ -66,13 +66,13 @@ public function testShouldGenerateAnAutomaticKeyIfNoKeyHasBeenGiven()
$connectionParams
);

$this->assertNotEquals(
self::assertNotEquals(
self::CACHE_KEY,
$cacheKey,
'The returned cache key should be generated automatically'
);

$this->assertNotEmpty($cacheKey, 'The generated cache key should not be empty');
self::assertNotEmpty($cacheKey, 'The generated cache key should not be empty');
}

public function testShouldGenerateDifferentKeysForSameQueryAndParamsAndDifferentConnections()
Expand Down Expand Up @@ -107,7 +107,7 @@ public function testShouldGenerateDifferentKeysForSameQueryAndParamsAndDifferent
$connectionParams
);

$this->assertNotEquals($firstCacheKey, $secondCacheKey, 'Cache keys should be different');
self::assertNotEquals($firstCacheKey, $secondCacheKey, 'Cache keys should be different');
}

public function testShouldGenerateSameKeysIfNoneOfTheParamsChanges()
Expand Down Expand Up @@ -140,6 +140,6 @@ public function testShouldGenerateSameKeysIfNoneOfTheParamsChanges()
$connectionParams
);

$this->assertEquals($firstCacheKey, $secondCacheKey, 'Cache keys should be the same');
self::assertEquals($firstCacheKey, $secondCacheKey, 'Cache keys should be the same');
}
}
6 changes: 3 additions & 3 deletions tests/Doctrine/Tests/DBAL/ConfigurationTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ protected function setUp()
*/
public function testReturnsDefaultConnectionAutoCommitMode()
{
$this->assertTrue($this->config->getAutoCommit());
self::assertTrue($this->config->getAutoCommit());
}

/**
Expand All @@ -63,10 +63,10 @@ public function testSetsDefaultConnectionAutoCommitMode()
{
$this->config->setAutoCommit(false);

$this->assertFalse($this->config->getAutoCommit());
self::assertFalse($this->config->getAutoCommit());

$this->config->setAutoCommit(0);

$this->assertFalse($this->config->getAutoCommit());
self::assertFalse($this->config->getAutoCommit());
}
}
62 changes: 31 additions & 31 deletions tests/Doctrine/Tests/DBAL/ConnectionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ public function getExecuteUpdateMockConnection()

public function testIsConnected()
{
$this->assertFalse($this->_conn->isConnected());
self::assertFalse($this->_conn->isConnected());
}

public function testNoTransactionActiveByDefault()
{
$this->assertFalse($this->_conn->isTransactionActive());
self::assertFalse($this->_conn->isTransactionActive());
}

public function testCommitWithNoActiveTransaction_ThrowsException()
Expand Down Expand Up @@ -92,37 +92,37 @@ public function testGetConfiguration()
{
$config = $this->_conn->getConfiguration();

$this->assertInstanceOf('Doctrine\DBAL\Configuration', $config);
self::assertInstanceOf('Doctrine\DBAL\Configuration', $config);
}

public function testGetHost()
{
$this->assertEquals('localhost', $this->_conn->getHost());
self::assertEquals('localhost', $this->_conn->getHost());
}

public function testGetPort()
{
$this->assertEquals('1234', $this->_conn->getPort());
self::assertEquals('1234', $this->_conn->getPort());
}

public function testGetUsername()
{
$this->assertEquals('root', $this->_conn->getUsername());
self::assertEquals('root', $this->_conn->getUsername());
}

public function testGetPassword()
{
$this->assertEquals('password', $this->_conn->getPassword());
self::assertEquals('password', $this->_conn->getPassword());
}

public function testGetDriver()
{
$this->assertInstanceOf('Doctrine\DBAL\Driver\PDOMySql\Driver', $this->_conn->getDriver());
self::assertInstanceOf('Doctrine\DBAL\Driver\PDOMySql\Driver', $this->_conn->getDriver());
}

public function testGetEventManager()
{
$this->assertInstanceOf('Doctrine\Common\EventManager', $this->_conn->getEventManager());
self::assertInstanceOf('Doctrine\Common\EventManager', $this->_conn->getEventManager());
}

public function testConnectDispatchEvent()
Expand All @@ -148,8 +148,8 @@ public function testEventManagerPassedToPlatform()
{
$driverMock = new DriverMock();
$connection = new Connection($this->params, $driverMock);
$this->assertInstanceOf('Doctrine\Common\EventManager', $connection->getDatabasePlatform()->getEventManager());
$this->assertSame($connection->getEventManager(), $connection->getDatabasePlatform()->getEventManager());
self::assertInstanceOf('Doctrine\Common\EventManager', $connection->getDatabasePlatform()->getEventManager());
self::assertSame($connection->getEventManager(), $connection->getDatabasePlatform()->getEventManager());
}

/**
Expand Down Expand Up @@ -189,7 +189,7 @@ public function testEchoSQLLogger()
{
$logger = new \Doctrine\DBAL\Logging\EchoSQLLogger();
$this->_conn->getConfiguration()->setSQLLogger($logger);
$this->assertSame($logger, $this->_conn->getConfiguration()->getSQLLogger());
self::assertSame($logger, $this->_conn->getConfiguration()->getSQLLogger());
}

/**
Expand All @@ -201,15 +201,15 @@ public function testDebugSQLStack()
{
$logger = new \Doctrine\DBAL\Logging\DebugStack();
$this->_conn->getConfiguration()->setSQLLogger($logger);
$this->assertSame($logger, $this->_conn->getConfiguration()->getSQLLogger());
self::assertSame($logger, $this->_conn->getConfiguration()->getSQLLogger());
}

/**
* @group DBAL-81
*/
public function testIsAutoCommit()
{
$this->assertTrue($this->_conn->isAutoCommit());
self::assertTrue($this->_conn->isAutoCommit());
}

/**
Expand All @@ -218,9 +218,9 @@ public function testIsAutoCommit()
public function testSetAutoCommit()
{
$this->_conn->setAutoCommit(false);
$this->assertFalse($this->_conn->isAutoCommit());
self::assertFalse($this->_conn->isAutoCommit());
$this->_conn->setAutoCommit(0);
$this->assertFalse($this->_conn->isAutoCommit());
self::assertFalse($this->_conn->isAutoCommit());
}

/**
Expand All @@ -236,11 +236,11 @@ public function testConnectStartsTransactionInNoAutoCommitMode()

$conn->setAutoCommit(false);

$this->assertFalse($conn->isTransactionActive());
self::assertFalse($conn->isTransactionActive());

$conn->connect();

$this->assertTrue($conn->isTransactionActive());
self::assertTrue($conn->isTransactionActive());
}

/**
Expand All @@ -258,7 +258,7 @@ public function testCommitStartsTransactionInNoAutoCommitMode()
$conn->connect();
$conn->commit();

$this->assertTrue($conn->isTransactionActive());
self::assertTrue($conn->isTransactionActive());
}

/**
Expand All @@ -276,7 +276,7 @@ public function testRollBackStartsTransactionInNoAutoCommitMode()
$conn->connect();
$conn->rollBack();

$this->assertTrue($conn->isTransactionActive());
self::assertTrue($conn->isTransactionActive());
}

/**
Expand All @@ -295,13 +295,13 @@ public function testSwitchingAutoCommitModeCommitsAllCurrentTransactions()
$conn->beginTransaction();
$conn->setAutoCommit(false);

$this->assertSame(1, $conn->getTransactionNestingLevel());
self::assertSame(1, $conn->getTransactionNestingLevel());

$conn->beginTransaction();
$conn->beginTransaction();
$conn->setAutoCommit(true);

$this->assertFalse($conn->isTransactionActive());
self::assertFalse($conn->isTransactionActive());
}

public function testEmptyInsert()
Expand Down Expand Up @@ -507,7 +507,7 @@ public function testFetchAssoc()
->with($statement, $params, $types)
->will($this->returnValue($driverStatementMock));

$this->assertSame($result, $conn->fetchAssoc($statement, $params, $types));
self::assertSame($result, $conn->fetchAssoc($statement, $params, $types));
}

public function testFetchArray()
Expand Down Expand Up @@ -541,7 +541,7 @@ public function testFetchArray()
->with($statement, $params, $types)
->will($this->returnValue($driverStatementMock));

$this->assertSame($result, $conn->fetchArray($statement, $params, $types));
self::assertSame($result, $conn->fetchArray($statement, $params, $types));
}

public function testFetchColumn()
Expand Down Expand Up @@ -576,7 +576,7 @@ public function testFetchColumn()
->with($statement, $params, $types)
->will($this->returnValue($driverStatementMock));

$this->assertSame($result, $conn->fetchColumn($statement, $params, $column, $types));
self::assertSame($result, $conn->fetchColumn($statement, $params, $column, $types));
}

public function testConnectionIsClosedButNotUnset()
Expand All @@ -599,7 +599,7 @@ public function testConnectionIsClosedButNotUnset()
// the wrapped connection should be null
// (and since connect() does nothing, this will not reconnect)
// this will also fail if this _conn property was unset instead of set to null
$this->assertNull($connection->getWrappedConnection());
self::assertNull($connection->getWrappedConnection());
}

public function testFetchAll()
Expand Down Expand Up @@ -632,7 +632,7 @@ public function testFetchAll()
->with($statement, $params, $types)
->will($this->returnValue($driverStatementMock));

$this->assertSame($result, $conn->fetchAll($statement, $params, $types));
self::assertSame($result, $conn->fetchAll($statement, $params, $types));
}

public function testConnectionDoesNotMaintainTwoReferencesToExternalPDO()
Expand All @@ -643,7 +643,7 @@ public function testConnectionDoesNotMaintainTwoReferencesToExternalPDO()

$conn = new Connection($params, $driverMock);

$this->assertArrayNotHasKey('pdo', $conn->getParams(), "Connection is maintaining additional reference to the PDO connection");
self::assertArrayNotHasKey('pdo', $conn->getParams(), "Connection is maintaining additional reference to the PDO connection");
}

public function testPassingExternalPDOMeansConnectionIsConnected()
Expand All @@ -654,7 +654,7 @@ public function testPassingExternalPDOMeansConnectionIsConnected()

$conn = new Connection($params, $driverMock);

$this->assertTrue($conn->isConnected(), "Connection is not connected after passing external PDO");
self::assertTrue($conn->isConnected(), "Connection is not connected after passing external PDO");
}

public function testCallingDeleteWithNoDeletionCriteriaResultsInInvalidArgumentException()
Expand Down Expand Up @@ -741,7 +741,7 @@ public function testPlatformDetectionIsTriggerOnlyOnceOnRetrievingPlatform()
->with('6.6.6')
->will($this->returnValue($platformMock));

$this->assertSame($platformMock, $connection->getDatabasePlatform());
self::assertSame($platformMock, $connection->getDatabasePlatform());
}

public function testConnectionParamsArePassedToTheQueryCacheProfileInExecuteCacheQuery()
Expand Down Expand Up @@ -776,7 +776,7 @@ public function testConnectionParamsArePassedToTheQueryCacheProfileInExecuteCach
/* @var $driver Driver */
$driver = $this->createMock(Driver::class);

$this->assertInstanceOf(
self::assertInstanceOf(
ArrayStatement::class,
(new Connection($this->params, $driver))->executeCacheQuery($query, $params, $types, $queryCacheProfileMock)
);
Expand Down
8 changes: 4 additions & 4 deletions tests/Doctrine/Tests/DBAL/DBALExceptionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public function testDriverExceptionDuringQueryAcceptsBinaryData()
/* @var $driver Driver */
$driver = $this->createMock(Driver::class);
$e = DBALException::driverExceptionDuringQuery($driver, new \Exception, '', array('ABC', chr(128)));
$this->assertContains('with params ["ABC", "\x80"]', $e->getMessage());
self::assertContains('with params ["ABC", "\x80"]', $e->getMessage());
}

public function testAvoidOverWrappingOnDriverException()
Expand All @@ -40,16 +40,16 @@ public function getSQLState()
};
$ex = new DriverException('', $inner);
$e = DBALException::driverExceptionDuringQuery($driver, $ex, '');
$this->assertSame($ex, $e);
self::assertSame($ex, $e);
}

public function testDriverRequiredWithUrl()
{
$url = 'mysql://localhost';
$exception = DBALException::driverRequired($url);

$this->assertInstanceOf(DBALException::class, $exception);
$this->assertSame(
self::assertInstanceOf(DBALException::class, $exception);
self::assertSame(
sprintf(
"The options 'driver' or 'driverClass' are mandatory if a connection URL without scheme " .
'is given to DriverManager::getConnection(). Given URL: %s',
Expand Down
18 changes: 9 additions & 9 deletions tests/Doctrine/Tests/DBAL/Driver/AbstractDriverTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ public function getSQLState()

$convertedException = $this->driver->convertException($message, $driverException);

$this->assertSame($convertedExceptionClassName, get_class($convertedException));
self::assertSame($convertedExceptionClassName, get_class($convertedException));

$this->assertSame($driverException->getErrorCode(), $convertedException->getErrorCode());
$this->assertSame($driverException->getSQLState(), $convertedException->getSQLState());
$this->assertSame($message, $convertedException->getMessage());
self::assertSame($driverException->getErrorCode(), $convertedException->getErrorCode());
self::assertSame($driverException->getSQLState(), $convertedException->getSQLState());
self::assertSame($message, $convertedException->getMessage());
}
}

Expand All @@ -122,7 +122,7 @@ public function testCreatesDatabasePlatformForVersion()
}

foreach ($data as $item) {
$this->assertSame($item[1], get_class($this->driver->createDatabasePlatformForVersion($item[0])));
self::assertSame($item[1], get_class($this->driver->createDatabasePlatformForVersion($item[0])));
}
}

Expand Down Expand Up @@ -152,21 +152,21 @@ public function testReturnsDatabaseName()
->method('getParams')
->will($this->returnValue($params));

$this->assertSame($params['dbname'], $this->driver->getDatabase($connection));
self::assertSame($params['dbname'], $this->driver->getDatabase($connection));
}

public function testReturnsDatabasePlatform()
{
$this->assertEquals($this->createPlatform(), $this->driver->getDatabasePlatform());
self::assertEquals($this->createPlatform(), $this->driver->getDatabasePlatform());
}

public function testReturnsSchemaManager()
{
$connection = $this->getConnectionMock();
$schemaManager = $this->driver->getSchemaManager($connection);

$this->assertEquals($this->createSchemaManager($connection), $schemaManager);
$this->assertAttributeSame($connection, '_conn', $schemaManager);
self::assertEquals($this->createSchemaManager($connection), $schemaManager);
self::assertAttributeSame($connection, '_conn', $schemaManager);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/Doctrine/Tests/DBAL/Driver/AbstractMySQLDriverTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function testReturnsDatabaseName()
->method('query')
->will($this->returnValue($statement));

$this->assertSame($database, $this->driver->getDatabase($connection));
self::assertSame($database, $this->driver->getDatabase($connection));
}

protected function createDriver()
Expand Down
Loading

0 comments on commit 62eb475

Please sign in to comment.