From 99069504840b0e6863d10786d45ee1f6d3023811 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 23 Jul 2020 13:36:32 +0200 Subject: [PATCH 1/5] Use assertStringContainsString instead of assertContains on strings Signed-off-by: Morris Jobke --- apps/dav/tests/unit/Command/ListCalendarsTest.php | 8 ++++---- apps/dav/tests/unit/Command/MoveCalendarTest.php | 4 ++-- tests/Core/Command/TwoFactorAuth/CleanupTest.php | 2 +- tests/Core/Command/TwoFactorAuth/DisableTest.php | 6 +++--- tests/Core/Command/TwoFactorAuth/EnableTest.php | 6 +++--- tests/Core/Command/TwoFactorAuth/EnforceTest.php | 12 ++++++------ tests/Core/Command/TwoFactorAuth/StateTest.php | 6 +++--- tests/lib/AppFramework/Http/DownloadResponseTest.php | 4 ++-- 8 files changed, 24 insertions(+), 24 deletions(-) diff --git a/apps/dav/tests/unit/Command/ListCalendarsTest.php b/apps/dav/tests/unit/Command/ListCalendarsTest.php index 1a0ca8c7954a6..41fbe77f35193 100644 --- a/apps/dav/tests/unit/Command/ListCalendarsTest.php +++ b/apps/dav/tests/unit/Command/ListCalendarsTest.php @@ -73,7 +73,7 @@ public function testWithBadUser() $commandTester->execute([ 'uid' => self::USERNAME, ]); - $this->assertContains("User <" . self::USERNAME . "> in unknown", $commandTester->getDisplay()); + $this->assertStringContainsString("User <" . self::USERNAME . "> in unknown", $commandTester->getDisplay()); } public function testWithCorrectUserWithNoCalendars() @@ -92,7 +92,7 @@ public function testWithCorrectUserWithNoCalendars() $commandTester->execute([ 'uid' => self::USERNAME, ]); - $this->assertContains("User <" . self::USERNAME . "> has no calendars\n", $commandTester->getDisplay()); + $this->assertStringContainsString("User <" . self::USERNAME . "> has no calendars\n", $commandTester->getDisplay()); } public function dataExecute() @@ -133,7 +133,7 @@ public function testWithCorrectUser(bool $readOnly, string $output) $commandTester->execute([ 'uid' => self::USERNAME, ]); - $this->assertContains($output, $commandTester->getDisplay()); - $this->assertNotContains(BirthdayService::BIRTHDAY_CALENDAR_URI, $commandTester->getDisplay()); + $this->assertStringContainsString($output, $commandTester->getDisplay()); + $this->assertStringNotContainsString(BirthdayService::BIRTHDAY_CALENDAR_URI, $commandTester->getDisplay()); } } diff --git a/apps/dav/tests/unit/Command/MoveCalendarTest.php b/apps/dav/tests/unit/Command/MoveCalendarTest.php index 07d789dbfb25a..1d4ba9e196089 100644 --- a/apps/dav/tests/unit/Command/MoveCalendarTest.php +++ b/apps/dav/tests/unit/Command/MoveCalendarTest.php @@ -314,7 +314,7 @@ public function testMoveWithDestinationPartOfGroup() 'destinationuid' => 'user2', ]); - $this->assertContains("[OK] Calendar was moved from user to ", $commandTester->getDisplay()); + $this->assertStringContainsString("[OK] Calendar was moved from user to ", $commandTester->getDisplay()); } public function testMoveWithDestinationNotPartOfGroupAndForce() @@ -362,7 +362,7 @@ public function testMoveWithDestinationNotPartOfGroupAndForce() '--force' => true ]); - $this->assertContains("[OK] Calendar was moved from user to ", $commandTester->getDisplay()); + $this->assertStringContainsString("[OK] Calendar was moved from user to ", $commandTester->getDisplay()); } public function dataTestMoveWithCalendarAlreadySharedToDestination(): array diff --git a/tests/Core/Command/TwoFactorAuth/CleanupTest.php b/tests/Core/Command/TwoFactorAuth/CleanupTest.php index 227283decf6b1..8609dfbed5cf8 100644 --- a/tests/Core/Command/TwoFactorAuth/CleanupTest.php +++ b/tests/Core/Command/TwoFactorAuth/CleanupTest.php @@ -60,7 +60,7 @@ public function testCleanup() { $this->assertEquals(0, $rc); $output = $this->cmd->getDisplay(); - $this->assertContains("All user-provider associations for provider u2f have been removed", $output); + $this->assertStringContainsString("All user-provider associations for provider u2f have been removed", $output); } } diff --git a/tests/Core/Command/TwoFactorAuth/DisableTest.php b/tests/Core/Command/TwoFactorAuth/DisableTest.php index 30ebc007dc150..e075bbf86ae4b 100644 --- a/tests/Core/Command/TwoFactorAuth/DisableTest.php +++ b/tests/Core/Command/TwoFactorAuth/DisableTest.php @@ -67,7 +67,7 @@ public function testInvalidUID() { ]); $this->assertEquals(1, $rc); - $this->assertContains("Invalid UID", $this->command->getDisplay()); + $this->assertStringContainsString("Invalid UID", $this->command->getDisplay()); } public function testEnableNotSupported() { @@ -87,7 +87,7 @@ public function testEnableNotSupported() { ]); $this->assertEquals(2, $rc); - $this->assertContains("The provider does not support this operation", $this->command->getDisplay()); + $this->assertStringContainsString("The provider does not support this operation", $this->command->getDisplay()); } public function testEnabled() { @@ -107,6 +107,6 @@ public function testEnabled() { ]); $this->assertEquals(0, $rc); - $this->assertContains("Two-factor provider totp disabled for user ricky", $this->command->getDisplay()); + $this->assertStringContainsString("Two-factor provider totp disabled for user ricky", $this->command->getDisplay()); } } diff --git a/tests/Core/Command/TwoFactorAuth/EnableTest.php b/tests/Core/Command/TwoFactorAuth/EnableTest.php index f31f92da4a5f8..10a9b735fa1a2 100644 --- a/tests/Core/Command/TwoFactorAuth/EnableTest.php +++ b/tests/Core/Command/TwoFactorAuth/EnableTest.php @@ -67,7 +67,7 @@ public function testInvalidUID() { ]); $this->assertEquals(1, $rc); - $this->assertContains("Invalid UID", $this->command->getDisplay()); + $this->assertStringContainsString("Invalid UID", $this->command->getDisplay()); } public function testEnableNotSupported() { @@ -87,7 +87,7 @@ public function testEnableNotSupported() { ]); $this->assertEquals(2, $rc); - $this->assertContains("The provider does not support this operation", $this->command->getDisplay()); + $this->assertStringContainsString("The provider does not support this operation", $this->command->getDisplay()); } public function testEnabled() { @@ -107,7 +107,7 @@ public function testEnabled() { ]); $this->assertEquals(0, $rc); - $this->assertContains("Two-factor provider totp enabled for user belle", $this->command->getDisplay()); + $this->assertStringContainsString("Two-factor provider totp enabled for user belle", $this->command->getDisplay()); } } diff --git a/tests/Core/Command/TwoFactorAuth/EnforceTest.php b/tests/Core/Command/TwoFactorAuth/EnforceTest.php index 2d9b77e18c23b..ba5423118d779 100644 --- a/tests/Core/Command/TwoFactorAuth/EnforceTest.php +++ b/tests/Core/Command/TwoFactorAuth/EnforceTest.php @@ -64,7 +64,7 @@ public function testEnforce() { $this->assertEquals(0, $rc); $display = $this->command->getDisplay(); - $this->assertContains("Two-factor authentication is enforced for all users", $display); + $this->assertStringContainsString("Two-factor authentication is enforced for all users", $display); } public function testEnforceForOneGroup() { @@ -82,7 +82,7 @@ public function testEnforceForOneGroup() { $this->assertEquals(0, $rc); $display = $this->command->getDisplay(); - $this->assertContains("Two-factor authentication is enforced for members of the group(s) twofactorers", $display); + $this->assertStringContainsString("Two-factor authentication is enforced for members of the group(s) twofactorers", $display); } public function testEnforceForAllExceptOneGroup() { @@ -100,7 +100,7 @@ public function testEnforceForAllExceptOneGroup() { $this->assertEquals(0, $rc); $display = $this->command->getDisplay(); - $this->assertContains("Two-factor authentication is enforced for all users, except members of yoloers", $display); + $this->assertStringContainsString("Two-factor authentication is enforced for all users, except members of yoloers", $display); } public function testDisableEnforced() { @@ -117,7 +117,7 @@ public function testDisableEnforced() { $this->assertEquals(0, $rc); $display = $this->command->getDisplay(); - $this->assertContains("Two-factor authentication is not enforced", $display); + $this->assertStringContainsString("Two-factor authentication is not enforced", $display); } public function testCurrentStateEnabled() { @@ -129,7 +129,7 @@ public function testCurrentStateEnabled() { $this->assertEquals(0, $rc); $display = $this->command->getDisplay(); - $this->assertContains("Two-factor authentication is enforced for all users", $display); + $this->assertStringContainsString("Two-factor authentication is enforced for all users", $display); } public function testCurrentStateDisabled() { @@ -141,7 +141,7 @@ public function testCurrentStateDisabled() { $this->assertEquals(0, $rc); $display = $this->command->getDisplay(); - $this->assertContains("Two-factor authentication is not enforced", $display); + $this->assertStringContainsString("Two-factor authentication is not enforced", $display); } } diff --git a/tests/Core/Command/TwoFactorAuth/StateTest.php b/tests/Core/Command/TwoFactorAuth/StateTest.php index 580e137fe32f1..08665b7125939 100644 --- a/tests/Core/Command/TwoFactorAuth/StateTest.php +++ b/tests/Core/Command/TwoFactorAuth/StateTest.php @@ -61,7 +61,7 @@ public function testWrongUID() { ]); $output = $this->cmd->getDisplay(); - $this->assertContains("Invalid UID", $output); + $this->assertStringContainsString("Invalid UID", $output); } public function testStateNoProvidersActive() { @@ -84,7 +84,7 @@ public function testStateNoProvidersActive() { ]); $output = $this->cmd->getDisplay(); - $this->assertContains("Two-factor authentication is not enabled for user eldora", $output); + $this->assertStringContainsString("Two-factor authentication is not enabled for user eldora", $output); } public function testStateOneProviderActive() { @@ -107,7 +107,7 @@ public function testStateOneProviderActive() { ]); $output = $this->cmd->getDisplay(); - $this->assertContains("Two-factor authentication is enabled for user mohamed", $output); + $this->assertStringContainsString("Two-factor authentication is enabled for user mohamed", $output); } } diff --git a/tests/lib/AppFramework/Http/DownloadResponseTest.php b/tests/lib/AppFramework/Http/DownloadResponseTest.php index 07c932b2725c8..e7fd6cd0dcdae 100644 --- a/tests/lib/AppFramework/Http/DownloadResponseTest.php +++ b/tests/lib/AppFramework/Http/DownloadResponseTest.php @@ -48,8 +48,8 @@ protected function setUp(){ public function testHeaders() { $headers = $this->response->getHeaders(); - $this->assertContains('attachment; filename="file"', $headers['Content-Disposition']); - $this->assertContains('content', $headers['Content-Type']); + $this->assertStringContainsString('attachment; filename="file"', $headers['Content-Disposition']); + $this->assertStringContainsString('content', $headers['Content-Type']); } From 42905727d5e235d48b945a56eddd0cfa146a0e25 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 23 Jul 2020 13:36:49 +0200 Subject: [PATCH 2/5] Remove deprecated test of internal attributes via assertAttributeEquals See https://github.com/sebastianbergmann/phpunit/issues/3339#issuecomment-428843322 It is seen as bad practice to test internal stuff of objects instead of the actual input and output of mathod calls. Signed-off-by: Morris Jobke --- tests/lib/ConfigTest.php | 24 +++++++++--------------- tests/lib/DB/DBSchemaTest.php | 4 ++-- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/tests/lib/ConfigTest.php b/tests/lib/ConfigTest.php index 2a4c962034079..3a5b3c17b6a0f 100644 --- a/tests/lib/ConfigTest.php +++ b/tests/lib/ConfigTest.php @@ -71,9 +71,7 @@ public function testGetValueReturnsEnvironmentValueIfSetToFalse() { public function testSetValue() { $this->config->setValue('foo', 'moo'); - $expectedConfig = $this->initialConfig; - $expectedConfig['foo'] = 'moo'; - $this->assertAttributeEquals($expectedConfig, 'cache', $this->config); + $this->assertSame('moo', $this->config->getValue('foo')); $content = file_get_contents($this->configFile); $expected = " 'moo',\n 'beers' => \n array (\n 0 => 'Appenzeller',\n " . @@ -81,10 +79,9 @@ public function testSetValue() { $this->assertEquals($expected, $content); $this->config->setValue('bar', 'red'); - $this->config->setValue('apps', array('files', 'gallery')); - $expectedConfig['bar'] = 'red'; - $expectedConfig['apps'] = array('files', 'gallery'); - $this->assertAttributeEquals($expectedConfig, 'cache', $this->config); + $this->config->setValue('apps', ['files', 'gallery']); + $this->assertSame('red', $this->config->getValue('bar')); + $this->assertSame(['files', 'gallery'], $this->config->getValue('apps')); $content = file_get_contents($this->configFile); @@ -105,7 +102,8 @@ public function testSetValues() { 'not_exists' => null, ]); - $this->assertAttributeEquals($this->initialConfig, 'cache', $this->config); + $this->assertSame('bar', $this->config->getValue('foo')); + $this->assertSame(null, $this->config->getValue('not_exists')); $content = file_get_contents($this->configFile); $this->assertEquals(self::TESTCONTENT, $content); @@ -113,10 +111,8 @@ public function testSetValues() { 'foo' => 'moo', 'alcohol_free' => null, ]); - $expectedConfig = $this->initialConfig; - $expectedConfig['foo'] = 'moo'; - unset($expectedConfig['alcohol_free']); - $this->assertAttributeEquals($expectedConfig, 'cache', $this->config); + $this->assertSame('moo', $this->config->getValue('foo')); + $this->assertSame(null, $this->config->getValue('not_exists')); $content = file_get_contents($this->configFile); $expected = " 'moo',\n 'beers' => \n array (\n 0 => 'Appenzeller',\n " . @@ -126,9 +122,7 @@ public function testSetValues() { public function testDeleteKey() { $this->config->deleteKey('foo'); - $expectedConfig = $this->initialConfig; - unset($expectedConfig['foo']); - $this->assertAttributeEquals($expectedConfig, 'cache', $this->config); + $this->assertSame('this_was_clearly_not_set_before', $this->config->getValue('foo', 'this_was_clearly_not_set_before')); $content = file_get_contents($this->configFile); $expected = " \n array (\n 0 => 'Appenzeller',\n " . diff --git a/tests/lib/DB/DBSchemaTest.php b/tests/lib/DB/DBSchemaTest.php index cc6059c163f35..13fb04b708ba7 100644 --- a/tests/lib/DB/DBSchemaTest.php +++ b/tests/lib/DB/DBSchemaTest.php @@ -83,8 +83,8 @@ public function doTestSchemaDumping() { $outfile = $this->tempManager->getTemporaryFile(); OC_DB::getDbStructure($outfile); $content = file_get_contents($outfile); - $this->assertContains($this->table1, $content); - $this->assertContains($this->table2, $content); + $this->assertStringContainsString($this->table1, $content); + $this->assertStringContainsString($this->table2, $content); } public function doTestSchemaRemoving() { From 4d54943ae601d15dbd79f000e74de3f4562f47e6 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 23 Jul 2020 13:38:49 +0200 Subject: [PATCH 3/5] Use assertEqualsCanonicalizing instead of deprecated assertEquals parameter Signed-off-by: Morris Jobke --- apps/dav/tests/unit/CalDAV/CalDavBackendTest.php | 2 +- tests/lib/L10N/FactoryTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php index 212b09f31fb49..90cac9d270714 100644 --- a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php +++ b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php @@ -442,7 +442,7 @@ public function testCalendarQuery($expectedEventsInResult, $propFilters, $compFi $expectedEventsInResult = array_map(function($index) use($events) { return $events[$index]; }, $expectedEventsInResult); - $this->assertEquals($expectedEventsInResult, $result, '', 0.0, 10, true); + $this->assertEqualsCanonicalizing($expectedEventsInResult, $result); } public function testGetCalendarObjectByUID() { diff --git a/tests/lib/L10N/FactoryTest.php b/tests/lib/L10N/FactoryTest.php index be842cf12c7bb..55d7acf6cb0a9 100644 --- a/tests/lib/L10N/FactoryTest.php +++ b/tests/lib/L10N/FactoryTest.php @@ -329,7 +329,7 @@ public function testFindAvailableLanguages($app) { ->with($app) ->willReturn(\OC::$SERVERROOT . '/tests/data/l10n/'); - $this->assertEquals(['cs', 'de', 'en', 'ru'], $factory->findAvailableLanguages($app), '', 0.0, 10, true); + $this->assertEqualsCanonicalizing(['cs', 'de', 'en', 'ru'], $factory->findAvailableLanguages($app)); } public function dataLanguageExists() { @@ -360,7 +360,7 @@ public function testFindAvailableLanguagesWithThemes() { ->with('theme') ->willReturn('abc'); - $this->assertEquals(['en', 'zz'], $factory->findAvailableLanguages($app), '', 0.0, 10, true); + $this->assertEqualsCanonicalizing(['en', 'zz'], $factory->findAvailableLanguages($app)); } /** From 2273de183c5085f80906a7c1f37dc8c559f0461d Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 23 Jul 2020 13:39:13 +0200 Subject: [PATCH 4/5] Replace deprecated assertArraySubset with logic that does the same Signed-off-by: Morris Jobke --- .../federatedfilesharing/tests/FederatedShareProviderTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php index 2a060b2d412bb..6f0e25de4fd72 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -195,7 +195,9 @@ public function testCreate() { 'accepted' => 0, 'token' => 'token', ]; - $this->assertArraySubset($expected, $data); + foreach (array_keys($expected) as $key) { + $this->assertEquals($expected[$key], $data[$key], "Assert that value for key '$key' is the same"); + } $this->assertEquals($data['id'], $share->getId()); $this->assertEquals(\OCP\Share::SHARE_TYPE_REMOTE, $share->getShareType()); From ffcb2830637f07ffe6b1ebf624857fc2c51f36e7 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 23 Jul 2020 13:44:18 +0200 Subject: [PATCH 5/5] Remove deprecated test of internal attributes via assertAttributeEquals in constructor tests I removed the tests completely because they just test that the constructor assigns the values to the internal properties. Nothing that should be cared about from the outside. See https://github.com/sebastianbergmann/phpunit/issues/3339#issuecomment-428843322 It is seen as bad practice to test internal stuff of objects instead of the actual input and output of mathod calls. Signed-off-by: Morris Jobke --- tests/lib/Template/CSSResourceLocatorTest.php | 10 ---------- tests/lib/Template/JSResourceLocatorTest.php | 11 ----------- tests/lib/Template/ResourceLocatorTest.php | 11 ----------- 3 files changed, 32 deletions(-) diff --git a/tests/lib/Template/CSSResourceLocatorTest.php b/tests/lib/Template/CSSResourceLocatorTest.php index a928f043ed3fd..39bca10314616 100644 --- a/tests/lib/Template/CSSResourceLocatorTest.php +++ b/tests/lib/Template/CSSResourceLocatorTest.php @@ -107,16 +107,6 @@ private function randomString() { return sha1(uniqid(mt_rand(), true)); } - public function testConstructor() { - $locator = $this->cssResourceLocator(); - $this->assertAttributeEquals('theme', 'theme', $locator); - $this->assertAttributeEquals('core', 'serverroot', $locator); - $this->assertAttributeEquals(array('core'=>'map','3rd'=>'party'), 'mapping', $locator); - $this->assertAttributeEquals('3rd', 'thirdpartyroot', $locator); - $this->assertAttributeEquals('map', 'webroot', $locator); - $this->assertAttributeEquals(array(), 'resources', $locator); - } - public function testFindWithAppPathSymlink() { // First create new apps path, and a symlink to it $apps_dirname = $this->randomString(); diff --git a/tests/lib/Template/JSResourceLocatorTest.php b/tests/lib/Template/JSResourceLocatorTest.php index f5dff62729fa1..eefd17edd3fe4 100644 --- a/tests/lib/Template/JSResourceLocatorTest.php +++ b/tests/lib/Template/JSResourceLocatorTest.php @@ -86,17 +86,6 @@ private function randomString() { return sha1(uniqid(mt_rand(), true)); } - - public function testConstructor() { - $locator = $this->jsResourceLocator(); - $this->assertAttributeEquals('theme', 'theme', $locator); - $this->assertAttributeEquals('core', 'serverroot', $locator); - $this->assertAttributeEquals(array('core'=>'map','3rd'=>'party'), 'mapping', $locator); - $this->assertAttributeEquals('3rd', 'thirdpartyroot', $locator); - $this->assertAttributeEquals('map', 'webroot', $locator); - $this->assertAttributeEquals(array(), 'resources', $locator); - } - public function testFindWithAppPathSymlink() { // First create new apps path, and a symlink to it $apps_dirname = $this->randomString(); diff --git a/tests/lib/Template/ResourceLocatorTest.php b/tests/lib/Template/ResourceLocatorTest.php index 580dfd60d0ecc..797c3b867c600 100644 --- a/tests/lib/Template/ResourceLocatorTest.php +++ b/tests/lib/Template/ResourceLocatorTest.php @@ -33,17 +33,6 @@ public function getResourceLocator($theme, $core_map, $party_map, $appsRoots) { '', true, true, true, array()); } - public function testConstructor() { - $locator = $this->getResourceLocator('theme', - array('core'=>'map'), array('3rd'=>'party'), array('foo'=>'bar')); - $this->assertAttributeEquals('theme', 'theme', $locator); - $this->assertAttributeEquals('core', 'serverroot', $locator); - $this->assertAttributeEquals(array('core'=>'map','3rd'=>'party'), 'mapping', $locator); - $this->assertAttributeEquals('3rd', 'thirdpartyroot', $locator); - $this->assertAttributeEquals('map', 'webroot', $locator); - $this->assertAttributeEquals(array(), 'resources', $locator); - } - public function testFind() { $locator = $this->getResourceLocator('theme', array('core' => 'map'), array('3rd' => 'party'), array('foo' => 'bar'));