Commit b33fb416 authored by Wouter Wolters's avatar Wouter Wolters Committed by Georg Ringer
Browse files

[TASK] Improve PHPunit used assertions

Main changes are:

* Use assertArrayHasKey/assertArrayNotHasKey instead of array_key_exists.
* Fix misordered assertEquals/assertNotEquals arguments.
* Use assertCount instead of assertEquals and count() method.

Resolves: #94751
Releases: master
Change-Id: Iaa0dbb4744bf41f43f4700714ed47910feaab081
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/70292


Tested-by: core-ci's avatarcore-ci <typo3@b13.com>
Tested-by: Roman Büchler's avatarRoman Büchler <info@buechler.pro>
Tested-by: Georg Ringer's avatarGeorg Ringer <georg.ringer@gmail.com>
Reviewed-by: Roman Büchler's avatarRoman Büchler <info@buechler.pro>
Reviewed-by: Georg Ringer's avatarGeorg Ringer <georg.ringer@gmail.com>
parent 5d257b78
......@@ -41,7 +41,7 @@ class BackendUserTest extends UnitTestCase
*/
public function getUidReturnsInitialValueForInt()
{
self::assertTrue($this->subject->getUid() === null, 'Not uid set after initialization.');
self::assertNull($this->subject->getUid(), 'Not uid set after initialization.');
}
/**
......@@ -49,7 +49,7 @@ class BackendUserTest extends UnitTestCase
*/
public function getUserNameReturnsInitialValueForString()
{
self::assertTrue($this->subject->getUserName() === '', 'Username not empty');
self::assertSame($this->subject->getUserName(), '', 'Username not empty');
}
/**
......@@ -67,7 +67,7 @@ class BackendUserTest extends UnitTestCase
*/
public function getRealNameReturnInitialValueForString()
{
self::assertTrue($this->subject->getRealName() === '', 'Real name not empty');
self::assertSame($this->subject->getRealName(), '', 'Real name not empty');
}
/**
......@@ -85,7 +85,7 @@ class BackendUserTest extends UnitTestCase
*/
public function getAdminReturnInitialValueForBoolean()
{
self::assertTrue($this->subject->getIsAdministrator() === false, 'Admin status is correct.');
self::assertFalse($this->subject->getIsAdministrator(), 'Admin status is correct.');
}
/**
......
......@@ -30,7 +30,7 @@ class LoginCest extends AbstractCest
$I->amGoingTo('assert the install tool is locked in the first place');
$I->waitForElementVisible('.panel-heading');
$I->see('The Install Tool is locked');
$I->assertFileNotExists(self::ENABLE_INSTALL_TOOL_FILEPATH);
$I->assertFileDoesNotExist(self::ENABLE_INSTALL_TOOL_FILEPATH);
$I->amGoingTo('lock the tool without logging in');
$I->writeToFile(self::ENABLE_INSTALL_TOOL_FILEPATH, '');
......
......@@ -49,7 +49,7 @@ class BackendLoginCest
$bsmo = $I->executeInSelenium(function (\Facebook\WebDriver\Remote\RemoteWebDriver $webdriver) {
return $webdriver->findElement(\Facebook\WebDriver\WebDriverBy::cssSelector('#t3-login-submit'))->getCSSValue('background-color');
});
$I->assertFalse($bs === $bsmo);
$I->assertNotSame($bs, $bsmo);
}
/**
......
......@@ -66,7 +66,7 @@ class DefaultValuesTest extends AbstractDataHandlerActionTestCase
$newContentId = reset($map['tt_content']);
$newContentRecord = BackendUtility::getRecord('tt_content', $newContentId);
// Empty header is used, because it was handed in
self::assertEquals($newContentRecord['header'], '');
self::assertEquals('', $newContentRecord['header']);
$map = $this->actionService->createNewRecord('tt_content', $newPageId, [
'bodytext' => 'Random bodytext'
......@@ -89,7 +89,7 @@ TCAdefaults.tt_content.header = global space');
]);
$newPageId = reset($map['pages']);
$newPageRecord = BackendUtility::getRecord('pages', $newPageId);
self::assertEquals($newPageRecord['keywords'], 'from pagets, with love');
self::assertEquals('from pagets, with love', $newPageRecord['keywords']);
$map = $this->actionService->createNewRecord('tt_content', $newPageId, [
'header' => '',
......@@ -98,14 +98,14 @@ TCAdefaults.tt_content.header = global space');
$newContentId = reset($map['tt_content']);
$newContentRecord = BackendUtility::getRecord('tt_content', $newContentId);
// Empty header is used, because it was handed in
self::assertEquals($newContentRecord['header'], '');
self::assertEquals('', $newContentRecord['header']);
$map = $this->actionService->createNewRecord('tt_content', $newPageId, [
'bodytext' => 'Random bodytext'
]);
$newContentId = reset($map['tt_content']);
$newContentRecord = BackendUtility::getRecord('tt_content', $newContentId);
self::assertEquals($newContentRecord['header'], 'global space');
self::assertEquals('global space', $newContentRecord['header']);
}
/**
......@@ -128,7 +128,7 @@ TCAdefaults.tt_content.header = local space
]);
$newPageId = reset($map['pages']);
$newPageRecord = BackendUtility::getRecord('pages', $newPageId);
self::assertEquals($newPageRecord['keywords'], 'I am specific, not generic');
self::assertEquals('I am specific, not generic', $newPageRecord['keywords']);
$map = $this->actionService->createNewRecord('tt_content', $newPageId, [
'header' => '',
......@@ -137,14 +137,14 @@ TCAdefaults.tt_content.header = local space
$newContentId = reset($map['tt_content']);
$newContentRecord = BackendUtility::getRecord('tt_content', $newContentId);
// Empty header is used, because it was handed in
self::assertEquals($newContentRecord['header'], '');
self::assertEquals('', $newContentRecord['header']);
$map = $this->actionService->createNewRecord('tt_content', $newPageId, [
'bodytext' => 'Random bodytext'
]);
$newContentId = reset($map['tt_content']);
$newContentRecord = BackendUtility::getRecord('tt_content', $newContentId);
self::assertEquals($newContentRecord['header'], 'local space');
self::assertEquals('local space', $newContentRecord['header']);
}
/**
......
......@@ -51,6 +51,6 @@ class DeleteTranslatedSubpagesTest extends AbstractDataHandlerActionTestCase
$dataHandler->start([], $cmd);
$dataHandler->process_cmdmap();
self::assertEquals($dataHandler->errorLog, []);
self::assertEquals([], $dataHandler->errorLog);
}
}
......@@ -115,7 +115,10 @@ class PharStreamWrapperInterceptorTest extends FunctionalTestCase
$handle = opendir('phar://' . $path);
closedir($handle);
self::assertFalse(is_resource($handle));
// Testing with a variable here, otherwise the suggested assertion would be assertIsNotResource
// which fails.
$isResource = is_resource($handle);
self::assertFalse($isResource);
}
public function directoryActionDeniesInvocationDataProvider()
......@@ -330,7 +333,10 @@ class PharStreamWrapperInterceptorTest extends FunctionalTestCase
$allowedPath = $this->instancePath . '/typo3conf/ext/test_resources/bundle.phar';
$handle = fopen('phar://' . $allowedPath . '/Resources/content.txt', 'r');
fclose($handle);
self::assertFalse(is_resource($handle));
// Testing with a variable here, otherwise the suggested assertion would be assertIsNotResource
// which fails.
$isResource = is_resource($handle);
self::assertFalse($isResource);
}
/**
......
......@@ -54,7 +54,7 @@ class FlashMessageQueueTest extends FunctionalTestCase
$filteredFlashMessages = $flashMessageQueue->getAllMessages(FlashMessage::NOTICE);
self::assertEquals(count($filteredFlashMessages), 1);
self::assertCount(1, $filteredFlashMessages);
reset($filteredFlashMessages);
$flashMessage = current($filteredFlashMessages);
......@@ -102,7 +102,7 @@ class FlashMessageQueueTest extends FunctionalTestCase
$filteredFlashMessages = $flashMessageQueue->getAllMessagesAndFlush(FlashMessage::NOTICE);
self::assertEquals(count($filteredFlashMessages), 1);
self::assertCount(1, $filteredFlashMessages);
reset($filteredFlashMessages);
$flashMessage = current($filteredFlashMessages);
......
......@@ -239,8 +239,8 @@ class ResourceStorageTest extends FunctionalTestCase
$file = GeneralUtility::makeInstance(ResourceFactory::class)->getFileObjectFromCombinedIdentifier('1:/foo/bar.txt');
$subject->deleteFile($file);
self::assertTrue(file_exists(Environment::getPublicPath() . '/fileadmin/_recycler_/bar.txt'));
self::assertFalse(file_exists(Environment::getPublicPath() . '/fileadmin/foo/bar.txt'));
self::assertFileExists(Environment::getPublicPath() . '/fileadmin/_recycler_/bar.txt');
self::assertFileDoesNotExist(Environment::getPublicPath() . '/fileadmin/foo/bar.txt');
}
/**
......@@ -259,7 +259,7 @@ class ResourceStorageTest extends FunctionalTestCase
$file = GeneralUtility::makeInstance(ResourceFactory::class)->getFileObjectFromCombinedIdentifier('1:/foo/bar.txt');
$subject->deleteFile($file);
self::assertFalse(file_exists(Environment::getPublicPath() . '/fileadmin/foo/bar.txt'));
self::assertFileDoesNotExist(Environment::getPublicPath() . '/fileadmin/foo/bar.txt');
}
public function searchFilesFindsFilesInFolderDataProvider(): array
......
......@@ -191,7 +191,7 @@ class DatabaseSessionBackendTest extends FunctionalTestCase
$this->subject->set('randomSessionId2', $this->testSessionRecord);
// Check if session was really removed
self::assertEquals(2, count($this->subject->getAll()));
self::assertCount(2, $this->subject->getAll());
}
/**
......
......@@ -232,7 +232,7 @@ class RedisSessionBackendTest extends FunctionalTestCase
$this->subject->set('randomSessionId2', $this->testSessionRecord);
// Check if session was really removed
self::assertEquals(2, count($this->subject->getAll()));
self::assertCount(2, $this->subject->getAll());
}
/**
......
......@@ -89,7 +89,7 @@ class Argon2iPasswordHashTest extends UnitTestCase
{
$hash = $this->subject->getHashedPassword('password');
self::assertNotNull($hash);
self::assertTrue(is_string($hash));
self::assertIsString($hash);
}
/**
......
......@@ -96,7 +96,7 @@ class Argon2idPasswordHashTest extends UnitTestCase
{
$hash = $this->subject->getHashedPassword('password');
self::assertNotNull($hash);
self::assertTrue(is_string($hash));
self::assertIsString($hash);
}
/**
......
......@@ -78,7 +78,7 @@ class BcryptPasswordHashTest extends UnitTestCase
{
$hash = $this->subject->getHashedPassword('password');
self::assertNotNull($hash);
self::assertTrue(is_string($hash));
self::assertIsString($hash);
}
/**
......
......@@ -82,7 +82,7 @@ class DataHandlerTest extends UnitTestCase
*/
public function fixtureCanBeCreated()
{
self::assertTrue($this->subject instanceof DataHandler);
self::assertInstanceOf(DataHandler::class, $this->subject);
}
//////////////////////////////////////////
......
......@@ -153,7 +153,7 @@ class FailsafeContainerTest extends UnitTestCase
self::assertNull($container->get('null'));
self::assertTrue($container->has('null'));
self::assertNull($container->get('null'));
self::assertEquals($calledCount, 1);
self::assertEquals(1, $calledCount);
}
public function testHas(): void
......
......@@ -67,12 +67,12 @@ class ListenerProviderTest extends UnitTestCase
$this->listenerProvider->addListener('Event\\Name', 'listener1');
$this->listenerProvider->addListener('Event\\Name', 'listener2', 'methodName');
self::assertEquals($this->listenerProvider->getAllListenerDefinitions(), [
self::assertEquals([
'Event\\Name' => [
[ 'service' => 'listener1', 'method' => null ],
[ 'service' => 'listener2', 'method' => 'methodName' ],
]
]);
], $this->listenerProvider->getAllListenerDefinitions());
}
/**
......
......@@ -102,7 +102,10 @@ class FormProtectionFactoryTest extends UnitTestCase
*/
public function getForTypeInstallToolReturnsInstallToolFormProtection()
{
self::assertTrue(FormProtectionFactory::get(InstallToolFormProtection::class) instanceof InstallToolFormProtection);
self::assertInstanceOf(
InstallToolFormProtection::class,
FormProtectionFactory::get(InstallToolFormProtection::class)
);
}
/**
......
......@@ -184,7 +184,7 @@ class MessageTest extends UnitTestCase
self::assertNotSame($message, $message2);
self::assertFalse($message2->hasHeader('X-Foo'));
$headers = $message2->getHeaders();
self::assertEquals(0, count($headers));
self::assertCount(0, $headers);
}
/**
......
......@@ -36,9 +36,6 @@ class RequestFactoryTest extends UnitTestCase
self::assertInstanceOf(RequestFactoryInterface::class, $factory);
}
/**
* @test
*/
public function testRequestHasMethodSet()
{
$factory = new RequestFactory();
......@@ -46,9 +43,6 @@ class RequestFactoryTest extends UnitTestCase
self::assertSame('POST', $request->getMethod());
}
/**
* @test
*/
public function testRequestFactoryHasAWritableEmptyBody()
{
$factory = new RequestFactory();
......
......@@ -352,7 +352,7 @@ class RequestTest extends UnitTestCase
$request = new Request('http://example.com');
$headers = $request->getHeaders();
self::assertArrayHasKey('host', $headers);
self::assertTrue(in_array('example.com', $headers['host']));
self::assertContains('example.com', $headers['host']);
}
/**
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment