From b9650aea749441d5daafa1a1dd530068d40d9768 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 8 Jun 2022 14:04:50 +0200 Subject: [PATCH 1/8] Fix risky test without assertion Signed-off-by: Joas Schilling --- tests/lib/Files/Node/FolderTest.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 4cda92b6e838b..1419b4eb0370c 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -105,11 +105,14 @@ public function testGet() { ->method('getUser') ->willReturn($this->user); + $expected = $this->createMock(Node::class); $root->method('get') - ->with('/bar/foo/asd'); + ->with('/bar/foo/asd') + ->willReturn($expected); $node = new Folder($root, $view, '/bar/foo'); - $node->get('asd'); + $return = $node->get('asd'); + self::assertEquals($expected, $return); } public function testNodeExists() { From c5c7ba578817a560e09eaafd243c58447c3f481c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 8 Jun 2022 14:09:26 +0200 Subject: [PATCH 2/8] Fail on risky tests Signed-off-by: Joas Schilling --- tests/phpunit-autotest.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/phpunit-autotest.xml b/tests/phpunit-autotest.xml index d2d45b50db7d7..b5489601451a7 100644 --- a/tests/phpunit-autotest.xml +++ b/tests/phpunit-autotest.xml @@ -2,6 +2,7 @@ Date: Wed, 8 Jun 2022 15:34:20 +0200 Subject: [PATCH 3/8] Fix missing imports Signed-off-by: Joas Schilling --- .../tests/ResetTokenBackgroundJobTest.php | 10 ++--- .../tests/Settings/AdminTest.php | 38 +++++++++++-------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/apps/updatenotification/tests/ResetTokenBackgroundJobTest.php b/apps/updatenotification/tests/ResetTokenBackgroundJobTest.php index 129ba37098063..e1fce2ad831dc 100644 --- a/apps/updatenotification/tests/ResetTokenBackgroundJobTest.php +++ b/apps/updatenotification/tests/ResetTokenBackgroundJobTest.php @@ -65,13 +65,11 @@ public function testRunWithNotExpiredToken() { public function testRunWithExpiredToken() { $this->timeFactory - ->expects($this->at(0)) ->method('getTime') - ->willReturn(1455131633); - $this->timeFactory - ->expects($this->at(1)) - ->method('getTime') - ->willReturn(1455045234); + ->willReturnOnConsecutiveCalls( + 1455131633, + 1455045234 + ); $this->config ->expects($this->once()) ->method('getAppValue') diff --git a/apps/updatenotification/tests/Settings/AdminTest.php b/apps/updatenotification/tests/Settings/AdminTest.php index 852504fb45aa4..bbaa56fa18dcf 100644 --- a/apps/updatenotification/tests/Settings/AdminTest.php +++ b/apps/updatenotification/tests/Settings/AdminTest.php @@ -29,6 +29,7 @@ */ namespace OCA\UpdateNotification\Tests\Settings; +use OC\User\Backend; use OCA\UpdateNotification\Settings\Admin; use OCA\UpdateNotification\UpdateChecker; use OCP\AppFramework\Http\TemplateResponse; @@ -36,13 +37,18 @@ use OCP\IDateTimeFormatter; use OCP\IGroup; use OCP\IGroupManager; +use OCP\IUserManager; use OCP\L10N\IFactory; use OCP\L10N\ILanguageIterator; use OCP\Support\Subscription\IRegistry; +use OCP\User\Backend\ICountUsersBackend; +use OCP\UserInterface; use OCP\Util; -use Test\TestCase; -use OCP\IUserManager; use Psr\Log\LoggerInterface; +use Test\TestCase; + +interface UserInterfaceAwareICountUsersBackend extends UserInterface, ICountUsersBackend { +} class AdminTest extends TestCase { /** @var IFactory|\PHPUnit\Framework\MockObject\MockObject */ @@ -77,11 +83,11 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->admin = new Admin( - $this->config, - $this->updateChecker, - $this->groupManager, - $this->dateTimeFormatter, - $this->l10nFactory, + $this->config, + $this->updateChecker, + $this->groupManager, + $this->dateTimeFormatter, + $this->l10nFactory, $this->subscriptionRegistry, $this->userManager, $this->logger @@ -89,9 +95,9 @@ protected function setUp(): void { } public function testGetFormWithUpdate() { - $backend1 = $this->createMock(UserInterface::class); - $backend2 = $this->createMock(UserInterface::class); - $backend3 = $this->createMock(UserInterface::class); + $backend1 = $this->createMock(UserInterfaceAwareICountUsersBackend::class); + $backend2 = $this->createMock(UserInterfaceAwareICountUsersBackend::class); + $backend3 = $this->createMock(UserInterfaceAwareICountUsersBackend::class); $backend1 ->expects($this->once()) ->method('implementsActions') @@ -213,9 +219,9 @@ public function testGetFormWithUpdate() { } public function testGetFormWithUpdateAndChangedUpdateServer() { - $backend1 = $this->createMock(UserInterface::class); - $backend2 = $this->createMock(UserInterface::class); - $backend3 = $this->createMock(UserInterface::class); + $backend1 = $this->createMock(UserInterfaceAwareICountUsersBackend::class); + $backend2 = $this->createMock(UserInterfaceAwareICountUsersBackend::class); + $backend3 = $this->createMock(UserInterfaceAwareICountUsersBackend::class); $backend1 ->expects($this->once()) ->method('implementsActions') @@ -337,9 +343,9 @@ public function testGetFormWithUpdateAndChangedUpdateServer() { } public function testGetFormWithUpdateAndCustomersUpdateServer() { - $backend1 = $this->createMock(UserInterface::class); - $backend2 = $this->createMock(UserInterface::class); - $backend3 = $this->createMock(UserInterface::class); + $backend1 = $this->createMock(UserInterfaceAwareICountUsersBackend::class); + $backend2 = $this->createMock(UserInterfaceAwareICountUsersBackend::class); + $backend3 = $this->createMock(UserInterfaceAwareICountUsersBackend::class); $backend1 ->expects($this->once()) ->method('implementsActions') From 3f06f1fa682024f78f0a87086f44d54fa56d4ad4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 8 Jun 2022 15:35:11 +0200 Subject: [PATCH 4/8] Import classes instead of using magic strings Signed-off-by: Joas Schilling --- tests/lib/Encryption/UtilTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/Encryption/UtilTest.php b/tests/lib/Encryption/UtilTest.php index 02155be11ddab..7f7fc862aac7f 100644 --- a/tests/lib/Encryption/UtilTest.php +++ b/tests/lib/Encryption/UtilTest.php @@ -40,11 +40,11 @@ protected function setUp(): void { ->disableOriginalConstructor() ->getMock(); - $this->userManager = $this->getMockBuilder('OC\User\Manager') + $this->userManager = $this->getMockBuilder(\OC\User\Manager::class) ->disableOriginalConstructor() ->getMock(); - $this->groupManager = $this->getMockBuilder('OC\Group\Manager') + $this->groupManager = $this->getMockBuilder(\OC\Group\Manager::class) ->disableOriginalConstructor() ->getMock(); From 8e62662ec55d8444a419e1e0657756627eb6313d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 8 Jun 2022 15:35:32 +0200 Subject: [PATCH 5/8] Make warnings fail the tests Signed-off-by: Joas Schilling --- tests/phpunit-autotest.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/phpunit-autotest.xml b/tests/phpunit-autotest.xml index b5489601451a7..95664ddae3bc1 100644 --- a/tests/phpunit-autotest.xml +++ b/tests/phpunit-autotest.xml @@ -3,6 +3,7 @@ bootstrap="bootstrap.php" verbose="true" failOnRisky="true" + failOnWarning="true" backupGlobals="false" timeoutForSmallTests="900" timeoutForMediumTests="900" From 6df0a79e5d335c5d6a23b839818932ec6e365c6e Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 8 Jun 2022 17:16:37 +0200 Subject: [PATCH 6/8] Enable files_external for tests Because we have more tests now that require classes from this app. Signed-off-by: Vincent Petry --- tests/enable_all.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/enable_all.php b/tests/enable_all.php index c494f3e0d553f..e1cf0025e61c9 100644 --- a/tests/enable_all.php +++ b/tests/enable_all.php @@ -18,6 +18,7 @@ function enableApp($app) { enableApp('files_sharing'); enableApp('files_trashbin'); +enableApp('files_external'); enableApp('encryption'); enableApp('user_ldap'); enableApp('files_versions'); From 32cab8102f24fdd2b552a9e7f6f514c6944d12dc Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 8 Jun 2022 22:46:32 +0200 Subject: [PATCH 7/8] Revert "Enable files_external for tests" This reverts commit 6df0a79e5d335c5d6a23b839818932ec6e365c6e. --- tests/enable_all.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/enable_all.php b/tests/enable_all.php index e1cf0025e61c9..c494f3e0d553f 100644 --- a/tests/enable_all.php +++ b/tests/enable_all.php @@ -18,7 +18,6 @@ function enableApp($app) { enableApp('files_sharing'); enableApp('files_trashbin'); -enableApp('files_external'); enableApp('encryption'); enableApp('user_ldap'); enableApp('files_versions'); From de93d1722b6e5c17c3699246204a761aa16b0ebd Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 8 Jun 2022 23:01:38 +0200 Subject: [PATCH 8/8] Fix encryption util tests for files_external Don't rely on files_external being enabled and mock all the classes from it. Signed-off-by: Vincent Petry --- lib/private/Encryption/Util.php | 4 +++- tests/lib/Encryption/UtilTest.php | 29 ++++++++++++++++++++++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/lib/private/Encryption/Util.php b/lib/private/Encryption/Util.php index 174af2e8b8995..541a4ed24f532 100644 --- a/lib/private/Encryption/Util.php +++ b/lib/private/Encryption/Util.php @@ -37,6 +37,7 @@ use OCP\Encryption\IEncryptionModule; use OCP\IConfig; use OCP\IUser; +use OCP\App\IAppManager; class Util { public const HEADER_START = 'HBEGIN'; @@ -299,7 +300,8 @@ public function getUserWithAccessToMountPoint($users, $groups) { * @return boolean */ public function isSystemWideMountPoint($path, $uid) { - if (\OCP\App::isEnabled("files_external")) { + $appManager = \OC::$server->get(IAppManager::class); + if ($appManager->isEnabledForUser('files_external', null)) { /** @var GlobalStoragesService $storageService */ $storageService = \OC::$server->get(GlobalStoragesService::class); $storages = $storageService->getAllStorages(); diff --git a/tests/lib/Encryption/UtilTest.php b/tests/lib/Encryption/UtilTest.php index 7f7fc862aac7f..ab6101b7027a9 100644 --- a/tests/lib/Encryption/UtilTest.php +++ b/tests/lib/Encryption/UtilTest.php @@ -4,8 +4,7 @@ use OC\Encryption\Util; use OC\Files\View; -use OCA\Files_External\Lib\StorageConfig; -use OCA\Files_External\Service\GlobalStoragesService; +use OCP\App\IAppManager; use OCP\Encryption\IEncryptionModule; use OCP\IConfig; use Test\TestCase; @@ -207,6 +206,15 @@ public function dataTestIsSystemWideMountPoint() { * @dataProvider dataTestIsSystemWideMountPoint */ public function testIsSystemWideMountPoint($expectedResult, $expectationText, $applicableUsers, $applicableGroups, $mountPointName = '/mp') { + $appManager = $this->createMock(IAppManager::class); + $appManager + ->expects($this->once()) + ->method('isEnabledForUser') + ->with('files_external') + ->willReturn(true); + + $this->overwriteService(IAppManager::class, $appManager); + $this->groupManager->method('isInGroup') ->will($this->returnValueMap([ ['user1', 'group1', true], // user is only in group1 @@ -215,17 +223,28 @@ public function testIsSystemWideMountPoint($expectedResult, $expectationText, $a $storages = []; - $storageConfig = $this->createMock(StorageConfig::class); + // StorageConfig + $storageConfig = $this->getMockBuilder('OCA\\Files_External\\Lib\\StorageConfig') + ->setMethods([ + 'getMountPoint', + 'getApplicableUsers', + 'getApplicableGroups', + ]) + ->getMock(); $storageConfig->method('getMountPoint')->willReturn($mountPointName); $storageConfig->method('getApplicableUsers')->willReturn($applicableUsers); $storageConfig->method('getApplicableGroups')->willReturn($applicableGroups); $storages[] = $storageConfig; - $storagesServiceMock = $this->createMock(GlobalStoragesService::class); + $storagesServiceMock = $this->getMockBuilder('OCA\\Files_External\\Service\\GlobalStoragesService') + ->setMethods([ + 'getAllStorages', + ]) + ->getMock(); $storagesServiceMock->expects($this->atLeastOnce())->method('getAllStorages') ->willReturn($storages); - $this->overwriteService(GlobalStoragesService::class, $storagesServiceMock); + $this->overwriteService('OCA\\Files_External\\Service\\GlobalStoragesService', $storagesServiceMock); $this->assertEquals($expectedResult, $this->util->isSystemWideMountPoint('/files/mp', 'user1'), 'Test case: ' . $expectationText); }