From c2211946f7dd4e59faf6d759e9a78bf140699c3e Mon Sep 17 00:00:00 2001 From: Thiemo Kreuz Date: Tue, 17 Sep 2019 16:03:28 +0200 Subject: [PATCH] tests: Replace PHPUnit's loose assertEquals(null) with assertNull() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit assertEquals( null, … ) still succeeds when the actual value is 0, false, an empty string, even an empty array. All these should be reported as a failure, I would argue. Note this patch previously also touched assertSame( null ). I reverted these. The only benefit would have been consistency within this codebase, but there is no strict reason to prefer one over the other. assertNull() and assertSame( null ) are functionally identical. Change-Id: I92102e833a8bc6af90b9516826abf111e2b79aac --- .../changes/CategoryMembershipChangeTest.php | 12 ++++++------ .../includes/content/WikitextContentTest.php | 5 ++--- tests/phpunit/includes/db/LBFactoryTest.php | 2 +- tests/phpunit/includes/db/LoadBalancerTest.php | 4 ++-- .../includes/filebackend/FileBackendTest.php | 6 +++--- .../libs/objectcache/WANObjectCacheTest.php | 14 +++++++------- .../includes/pager/RangeChronologicalPagerTest.php | 2 +- .../resourceloader/ResourceLoaderContextTest.php | 10 +++++----- .../includes/session/CookieSessionProviderTest.php | 2 +- tests/phpunit/unit/includes/FauxResponseTest.php | 4 ++-- .../changes/ChangesListFilterGroupTest.php | 3 +-- tests/phpunit/unit/includes/diff/DiffOpTest.php | 2 +- .../unit/includes/language/LanguageCodeTest.php | 2 +- .../unit/includes/session/SessionUnitTest.php | 4 ++-- 14 files changed, 35 insertions(+), 37 deletions(-) diff --git a/tests/phpunit/includes/changes/CategoryMembershipChangeTest.php b/tests/phpunit/includes/changes/CategoryMembershipChangeTest.php index 31929d386a..4e062ed769 100644 --- a/tests/phpunit/includes/changes/CategoryMembershipChangeTest.php +++ b/tests/phpunit/includes/changes/CategoryMembershipChangeTest.php @@ -85,9 +85,9 @@ class CategoryMembershipChangeTest extends MediaWikiLangTestCase { $this->assertEquals( self::$pageName, self::$lastNotifyArgs[4]->getPrefixedText() ); $this->assertSame( 0, self::$lastNotifyArgs[5] ); $this->assertSame( 0, self::$lastNotifyArgs[6] ); - $this->assertEquals( null, self::$lastNotifyArgs[7] ); + $this->assertNull( self::$lastNotifyArgs[7] ); $this->assertEquals( 1, self::$lastNotifyArgs[8] ); - $this->assertEquals( null, self::$lastNotifyArgs[9] ); + $this->assertSame( '', self::$lastNotifyArgs[9] ); $this->assertSame( 0, self::$lastNotifyArgs[10] ); } @@ -105,9 +105,9 @@ class CategoryMembershipChangeTest extends MediaWikiLangTestCase { $this->assertEquals( self::$pageName, self::$lastNotifyArgs[4]->getPrefixedText() ); $this->assertSame( 0, self::$lastNotifyArgs[5] ); $this->assertSame( 0, self::$lastNotifyArgs[6] ); - $this->assertEquals( null, self::$lastNotifyArgs[7] ); + $this->assertNull( self::$lastNotifyArgs[7] ); $this->assertEquals( 1, self::$lastNotifyArgs[8] ); - $this->assertEquals( null, self::$lastNotifyArgs[9] ); + $this->assertSame( '', self::$lastNotifyArgs[9] ); $this->assertSame( 0, self::$lastNotifyArgs[10] ); } @@ -126,7 +126,7 @@ class CategoryMembershipChangeTest extends MediaWikiLangTestCase { $this->assertEquals( self::$pageName, self::$lastNotifyArgs[4]->getPrefixedText() ); $this->assertEquals( self::$pageRev->getParentId(), self::$lastNotifyArgs[5] ); $this->assertEquals( $revision->getId(), self::$lastNotifyArgs[6] ); - $this->assertEquals( null, self::$lastNotifyArgs[7] ); + $this->assertNull( self::$lastNotifyArgs[7] ); $this->assertSame( 0, self::$lastNotifyArgs[8] ); $this->assertEquals( '127.0.0.1', self::$lastNotifyArgs[9] ); $this->assertSame( 0, self::$lastNotifyArgs[10] ); @@ -147,7 +147,7 @@ class CategoryMembershipChangeTest extends MediaWikiLangTestCase { $this->assertEquals( self::$pageName, self::$lastNotifyArgs[4]->getPrefixedText() ); $this->assertEquals( self::$pageRev->getParentId(), self::$lastNotifyArgs[5] ); $this->assertEquals( $revision->getId(), self::$lastNotifyArgs[6] ); - $this->assertEquals( null, self::$lastNotifyArgs[7] ); + $this->assertNull( self::$lastNotifyArgs[7] ); $this->assertSame( 0, self::$lastNotifyArgs[8] ); $this->assertEquals( '127.0.0.1', self::$lastNotifyArgs[9] ); $this->assertSame( 0, self::$lastNotifyArgs[10] ); diff --git a/tests/phpunit/includes/content/WikitextContentTest.php b/tests/phpunit/includes/content/WikitextContentTest.php index cd7cc1010d..fc1c42d779 100644 --- a/tests/phpunit/includes/content/WikitextContentTest.php +++ b/tests/phpunit/includes/content/WikitextContentTest.php @@ -390,7 +390,7 @@ just a test" $this->assertEquals( 'hello world.', $wikitext, 'Wikitext passed to hook was not as expected' ); - $this->assertEquals( null, $redirectTarget, 'Redirect seen in hook was not null' ); + $this->assertNull( $redirectTarget, 'Redirect seen in hook was not null' ); $this->assertEquals( $title, $options->getRedirectTarget(), 'ParserOptions\' redirectTarget was changed' ); @@ -417,8 +417,7 @@ just a test" $redirectTarget->getFullText(), 'Redirect seen in hook was not the expected title' ); - $this->assertEquals( - null, + $this->assertNull( $options->getRedirectTarget(), 'ParserOptions\' redirectTarget was changed' ); diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php index f00499f35d..c0187444c8 100644 --- a/tests/phpunit/includes/db/LBFactoryTest.php +++ b/tests/phpunit/includes/db/LBFactoryTest.php @@ -412,7 +412,7 @@ class LBFactoryTest extends MediaWikiTestCase { $cpIndex = null; $cp->shutdown( null, 'sync', $cpIndex ); - $this->assertEquals( null, $cpIndex, "CP write index retained" ); + $this->assertNull( $cpIndex, "CP write index retained" ); $this->assertEquals( '45e93a9c215c031d38b7c42d8e4700ca', $cp->getClientId() ); } diff --git a/tests/phpunit/includes/db/LoadBalancerTest.php b/tests/phpunit/includes/db/LoadBalancerTest.php index 981b4ad717..a542936140 100644 --- a/tests/phpunit/includes/db/LoadBalancerTest.php +++ b/tests/phpunit/includes/db/LoadBalancerTest.php @@ -434,7 +434,7 @@ class LoadBalancerTest extends MediaWikiTestCase { $lb = $this->newSingleServerLocalLoadBalancer(); $i = $lb->getWriterIndex(); - $this->assertEquals( null, $lb->getAnyOpenConnection( $i ) ); + $this->assertFalse( $lb->getAnyOpenConnection( $i ) ); $conn1 = $lb->getConnection( $i ); $this->assertNotEquals( null, $conn1 ); @@ -446,7 +446,7 @@ class LoadBalancerTest extends MediaWikiTestCase { $this->assertFalse( $conn2->getFlag( DBO_TRX ) ); if ( $lb->getServerAttributes( $i )[Database::ATTR_DB_LEVEL_LOCKING] ) { - $this->assertEquals( null, + $this->assertFalse( $lb->getAnyOpenConnection( $i, $lb::CONN_TRX_AUTOCOMMIT ) ); $this->assertEquals( $conn1, $lb->getConnection( diff --git a/tests/phpunit/includes/filebackend/FileBackendTest.php b/tests/phpunit/includes/filebackend/FileBackendTest.php index 7bc7918776..afa82834b2 100644 --- a/tests/phpunit/includes/filebackend/FileBackendTest.php +++ b/tests/phpunit/includes/filebackend/FileBackendTest.php @@ -1567,11 +1567,11 @@ class FileBackendTest extends MediaWikiTestCase { $tmpFile = $this->backend->getLocalCopy( [ 'src' => "$base/unittest-cont1/not-there" ] ); - $this->assertEquals( null, $tmpFile, "Local copy of not existing file is null ($backendName)." ); + $this->assertNull( $tmpFile, "Local copy of not existing file is null ($backendName)." ); $tmpFile = $this->backend->getLocalReference( [ 'src' => "$base/unittest-cont1/not-there" ] ); - $this->assertEquals( null, $tmpFile, "Local ref of not existing file is null ($backendName)." ); + $this->assertNull( $tmpFile, "Local ref of not existing file is null ($backendName)." ); } /** @@ -2484,7 +2484,7 @@ class FileBackendTest extends MediaWikiTestCase { "Scoped locking of files succeeded with OK status ($backendName)." ); ScopedLock::release( $sl ); - $this->assertEquals( null, $sl, + $this->assertNull( $sl, "Scoped unlocking of files succeeded ($backendName)." ); $this->assertEquals( [], $status->getErrors(), "Scoped unlocking of files succeeded ($backendName)." ); diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php index 7c4c9bf944..61d3785bc0 100644 --- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php +++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php @@ -381,8 +381,8 @@ class WANObjectCacheTest extends PHPUnit\Framework\TestCase { $v = $cache->getWithSetCallback( $key, 30, $checkFunc, [ 'staleTTL' => 50 ] + $extOpts ); $this->assertEquals( 'xxx1', $v, "Value returned" ); - $this->assertEquals( false, $oldValReceived, "Callback got no stale value" ); - $this->assertEquals( null, $oldAsOfReceived, "Callback got no stale value" ); + $this->assertFalse( $oldValReceived, "Callback got no stale value" ); + $this->assertNull( $oldAsOfReceived, "Callback got no stale value" ); $mockWallClock += 40; $v = $cache->getWithSetCallback( @@ -397,8 +397,8 @@ class WANObjectCacheTest extends PHPUnit\Framework\TestCase { $key, 30, $checkFunc, [ 'staleTTL' => 50 ] + $extOpts ); $this->assertEquals( 'xxx3', $v, "Value still returned after expired" ); $this->assertEquals( 3, $wasSet, "Value recalculated while expired" ); - $this->assertEquals( false, $oldValReceived, "Callback got no stale value" ); - $this->assertEquals( null, $oldAsOfReceived, "Callback got no stale value" ); + $this->assertFalse( $oldValReceived, "Callback got no stale value" ); + $this->assertNull( $oldAsOfReceived, "Callback got no stale value" ); $mockWallClock = ( $priorTime - $cache::HOLDOFF_TTL - 1 ); $wasSet = 0; @@ -414,8 +414,8 @@ class WANObjectCacheTest extends PHPUnit\Framework\TestCase { ); $this->assertEquals( 'xxx1', $v, "Value returned" ); $this->assertEquals( 1, $wasSet, "Value computed" ); - $this->assertEquals( false, $oldValReceived, "Callback got no stale value" ); - $this->assertEquals( null, $oldAsOfReceived, "Callback got no stale value" ); + $this->assertFalse( $oldValReceived, "Callback got no stale value" ); + $this->assertNull( $oldAsOfReceived, "Callback got no stale value" ); $mockWallClock += $cache::TTL_HOUR; // some time passes $v = $cache->getWithSetCallback( @@ -1473,7 +1473,7 @@ class WANObjectCacheTest extends PHPUnit\Framework\TestCase { $this->assertEquals( $valueV2, $v, "Value returned" ); $this->assertEquals( 1, $wasSet, "Value regenerated" ); $this->assertEquals( false, $priorValue, "Old value not given due to old format" ); - $this->assertEquals( null, $priorAsOf, "Old value not given due to old format" ); + $this->assertNull( $priorAsOf, "Old value not given due to old format" ); $wasSet = 0; $v = $cache->getWithSetCallback( $key, 30, $funcV2, $verOpts + $extOpts ); diff --git a/tests/phpunit/includes/pager/RangeChronologicalPagerTest.php b/tests/phpunit/includes/pager/RangeChronologicalPagerTest.php index 72390ac856..f11a2a257f 100644 --- a/tests/phpunit/includes/pager/RangeChronologicalPagerTest.php +++ b/tests/phpunit/includes/pager/RangeChronologicalPagerTest.php @@ -85,7 +85,7 @@ class RangeChronologicalPagerTest extends MediaWikiLangTestCase { */ public function testGetDateRangeCondInvalid( $start, $end ) { $pager = $this->getMockForAbstractClass( RangeChronologicalPager::class ); - $this->assertEquals( null, $pager->getDateRangeCond( $start, $end ) ); + $this->assertNull( $pager->getDateRangeCond( $start, $end ) ); } public function getDateRangeCondInvalidProvider() { diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderContextTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderContextTest.php index c748e2cf6e..6039bd220d 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderContextTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderContextTest.php @@ -26,10 +26,10 @@ class ResourceLoaderContextTest extends PHPUnit\Framework\TestCase { // Request parameters $this->assertEquals( [], $ctx->getModules() ); $this->assertEquals( 'qqx', $ctx->getLanguage() ); - $this->assertEquals( false, $ctx->getDebug() ); - $this->assertEquals( null, $ctx->getOnly() ); + $this->assertFalse( $ctx->getDebug() ); + $this->assertNull( $ctx->getOnly() ); $this->assertEquals( 'fallback', $ctx->getSkin() ); - $this->assertEquals( null, $ctx->getUser() ); + $this->assertNull( $ctx->getUser() ); $this->assertNull( $ctx->getContentOverrideCallback() ); // Misc @@ -67,11 +67,11 @@ class ResourceLoaderContextTest extends PHPUnit\Framework\TestCase { $ctx->getModules(), [ 'foo', 'foo.quux', 'foo.baz', 'foo.bar', 'baz.quux' ] ); - $this->assertEquals( false, $ctx->getDebug() ); + $this->assertFalse( $ctx->getDebug() ); $this->assertEquals( 'zh', $ctx->getLanguage() ); $this->assertEquals( 'styles', $ctx->getOnly() ); $this->assertEquals( 'fallback', $ctx->getSkin() ); - $this->assertEquals( null, $ctx->getUser() ); + $this->assertNull( $ctx->getUser() ); // Misc $this->assertEquals( 'ltr', $ctx->getDirection() ); diff --git a/tests/phpunit/includes/session/CookieSessionProviderTest.php b/tests/phpunit/includes/session/CookieSessionProviderTest.php index 33bc29f1ce..3eb7498c87 100644 --- a/tests/phpunit/includes/session/CookieSessionProviderTest.php +++ b/tests/phpunit/includes/session/CookieSessionProviderTest.php @@ -376,7 +376,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase { ] ); $request = new \FauxRequest(); - $this->assertEquals( null, $provider->suggestLoginUsername( $request ) ); + $this->assertNull( $provider->suggestLoginUsername( $request ) ); $request->setCookies( [ 'xUserName' => 'Example', diff --git a/tests/phpunit/unit/includes/FauxResponseTest.php b/tests/phpunit/unit/includes/FauxResponseTest.php index 5e208aceda..0cca53b3cc 100644 --- a/tests/phpunit/unit/includes/FauxResponseTest.php +++ b/tests/phpunit/unit/includes/FauxResponseTest.php @@ -47,7 +47,7 @@ class FauxResponseTest extends \MediaWikiUnitTestCase { 'expire' => $expire, ]; - $this->assertEquals( null, $this->response->getCookie( 'xkey' ), 'Non-existing cookie' ); + $this->assertNull( $this->response->getCookie( 'xkey' ), 'Non-existing cookie' ); $this->response->setCookie( 'key', 'val', $expire, [ 'prefix' => 'x', 'path' => '/path', @@ -67,7 +67,7 @@ class FauxResponseTest extends \MediaWikiUnitTestCase { * @covers FauxResponse::header */ public function testHeader() { - $this->assertEquals( null, $this->response->getHeader( 'Location' ), 'Non-existing header' ); + $this->assertNull( $this->response->getHeader( 'Location' ), 'Non-existing header' ); $this->response->header( 'Location: http://localhost/' ); $this->assertEquals( diff --git a/tests/phpunit/unit/includes/changes/ChangesListFilterGroupTest.php b/tests/phpunit/unit/includes/changes/ChangesListFilterGroupTest.php index bd54d508ba..efa056498e 100644 --- a/tests/phpunit/unit/includes/changes/ChangesListFilterGroupTest.php +++ b/tests/phpunit/unit/includes/changes/ChangesListFilterGroupTest.php @@ -71,8 +71,7 @@ class ChangesListFilterGroupTest extends \MediaWikiUnitTestCase { $group->getFilter( 'foo' )->getName() ); - $this->assertEquals( - null, + $this->assertNull( $group->getFilter( 'bar' ) ); } diff --git a/tests/phpunit/unit/includes/diff/DiffOpTest.php b/tests/phpunit/unit/includes/diff/DiffOpTest.php index 17487acbca..6566e142be 100644 --- a/tests/phpunit/unit/includes/diff/DiffOpTest.php +++ b/tests/phpunit/unit/includes/diff/DiffOpTest.php @@ -42,7 +42,7 @@ class DiffOpTest extends \MediaWikiUnitTestCase { $this->assertEquals( 'foo', $obj->getClosing( 0 ) ); $this->assertEquals( 'bar', $obj->getClosing( 1 ) ); $this->assertEquals( 'baz', $obj->getClosing( 2 ) ); - $this->assertEquals( null, $obj->getClosing( 3 ) ); + $this->assertNull( $obj->getClosing( 3 ) ); } /** diff --git a/tests/phpunit/unit/includes/language/LanguageCodeTest.php b/tests/phpunit/unit/includes/language/LanguageCodeTest.php index f3a7ae4d7d..e8562cf653 100644 --- a/tests/phpunit/unit/includes/language/LanguageCodeTest.php +++ b/tests/phpunit/unit/includes/language/LanguageCodeTest.php @@ -38,7 +38,7 @@ class LanguageCodeTest extends MediaWikiUnitTestCase { public function testReplaceDeprecatedCodes() { $this->assertEquals( 'gsw', LanguageCode::replaceDeprecatedCodes( 'als' ) ); $this->assertEquals( 'gsw', LanguageCode::replaceDeprecatedCodes( 'gsw' ) ); - $this->assertEquals( null, LanguageCode::replaceDeprecatedCodes( null ) ); + $this->assertNull( LanguageCode::replaceDeprecatedCodes( null ) ); } /** diff --git a/tests/phpunit/unit/includes/session/SessionUnitTest.php b/tests/phpunit/unit/includes/session/SessionUnitTest.php index b6e1d3a7c6..7b2b29932f 100644 --- a/tests/phpunit/unit/includes/session/SessionUnitTest.php +++ b/tests/phpunit/unit/includes/session/SessionUnitTest.php @@ -104,7 +104,7 @@ class SessionUnitTest extends MediaWikiUnitTestCase { $this->assertEquals( 'zero', $session->get( 0 ) ); $this->assertFalse( $backend->dirty ); - $this->assertEquals( null, $session->get( 'null' ) ); + $this->assertNull( $session->get( 'null' ) ); $this->assertEquals( 'default', $session->get( 'null', 'default' ) ); $this->assertFalse( $backend->dirty ); @@ -165,7 +165,7 @@ class SessionUnitTest extends MediaWikiUnitTestCase { $this->assertFalse( $backend->dirty ); $logger->setCollect( true ); - $this->assertEquals( null, $session['null'] ); + $this->assertNull( $session['null'] ); $logger->setCollect( false ); $this->assertFalse( $backend->dirty ); $this->assertSame( [ -- 2.20.1