Skip to content

Speedup share tests #60709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
icewind1991 wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Speedup share tests #60709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e757101
test: don't trigger file cache gc during tests
icewind1991 May 23, 2026
7f08204
test: mock ocm discovery in external share test
icewind1991 May 23, 2026
2da5e83
test: use dummy users in SharesReminderJobTest
icewind1991 May 23, 2026
0e02179
feat: allow unregistering group backends
icewind1991 May 23, 2026
dc091c6
test: add test trait for creating temporary groups
icewind1991 May 23, 2026
a55798a
fix: remove trait property conflict
icewind1991 May 23, 2026
ea18ef4
test: more group/user test trait usage and setup/teardown improvements
icewind1991 May 23, 2026
56f7823
test: improve setup in share propagation tests
icewind1991 May 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/files_sharing/lib/External/Storage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function __construct($options) {
$this->manager = $options['manager'];
$this->cloudId = $options['cloudId'];
$this->logger = Server::get(LoggerInterface::class);
$discoveryService = Server::get(IOCMDiscoveryService::class);
$discoveryService = $options['discoveryService'] ?? Server::get(IOCMDiscoveryService::class);
$this->config = Server::get(IConfig::class);
$this->appConfig = Server::get(IAppConfig::class);
$this->shareManager = Server::get(IShareManager::class);
Expand Down
2 changes: 0 additions & 2 deletions apps/files_sharing/tests/CacheTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ class CacheTest extends TestCase {
protected Cache $sharedCache;
protected Storage $ownerStorage;
protected Storage $sharedStorage;
/** @var \OCP\Share\IManager $shareManager */
protected $shareManager;

protected function setUp(): void {
parent::setUp();
Expand Down
4 changes: 3 additions & 1 deletion apps/files_sharing/tests/Controller/ShareControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,14 @@
use OCP\Share\IPublicShareTemplateFactory;
use OCP\Share\IShare;
use PHPUnit\Framework\MockObject\MockObject;
use Test\Traits\UserTrait;

/**
* @package OCA\Files_Sharing\Controllers
*/
#[\PHPUnit\Framework\Attributes\Group(name: 'DB')]
class ShareControllerTest extends \Test\TestCase {
use UserTrait;

private string $user;
private string $oldUser;
Expand Down Expand Up @@ -150,7 +152,7 @@ protected function setUp(): void {
// Create a dummy user
$this->user = Server::get(ISecureRandom::class)->generate(12, ISecureRandom::CHAR_LOWER);

Server::get(IUserManager::class)->createUser($this->user, $this->user);
$this->createUser($this->user, $this->user);
\OC_Util::tearDownFS();
$this->loginAsUser($this->user);
}
Expand Down
8 changes: 5 additions & 3 deletions apps/files_sharing/tests/DeleteOrphanedSharesJobTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use OCP\IUserManager;
use OCP\Server;
use OCP\Share\IShare;
use Test\Traits\UserTrait;

/**
* Class DeleteOrphanedSharesJobTest
Expand All @@ -25,6 +26,8 @@
*/
#[\PHPUnit\Framework\Attributes\Group(name: 'DB')]
class DeleteOrphanedSharesJobTest extends \Test\TestCase {
use UserTrait;

/**
* @var bool
*/
Expand Down Expand Up @@ -75,9 +78,8 @@ protected function setUp(): void {
$this->user1 = $this->getUniqueID('user1_');
$this->user2 = $this->getUniqueID('user2_');

$userManager = Server::get(IUserManager::class);
$userManager->createUser($this->user1, 'pass');
$userManager->createUser($this->user2, 'pass');
$this->createUser($this->user1, 'pass');
$this->createUser($this->user2, 'pass');

\OC::registerShareHooks(Server::get(SystemConfig::class));

Expand Down
44 changes: 17 additions & 27 deletions apps/files_sharing/tests/EtagPropagationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
* SPDX-License-Identifier: AGPL-3.0-only
*/

namespace OCA\Files_Sharing\Tests;

use OC\Files\Filesystem;
Expand All @@ -31,16 +32,15 @@ class EtagPropagationTest extends PropagationTestCase {
* "user4" puts the received "inside" folder into "sub1/sub2/inside" (this is to check if it propagates across multiple subfolders)
*/
protected function setUpShares() {
$this->fileIds[self::TEST_FILES_SHARING_API_USER1] = [];
$this->fileIds[self::TEST_FILES_SHARING_API_USER2] = [];
$this->fileIds[self::TEST_FILES_SHARING_API_USER3] = [];
$this->fileIds[self::TEST_FILES_SHARING_API_USER4] = [];
$this->fileInfos[self::TEST_FILES_SHARING_API_USER1] = [];
$this->fileInfos[self::TEST_FILES_SHARING_API_USER2] = [];
$this->fileInfos[self::TEST_FILES_SHARING_API_USER3] = [];
$this->fileInfos[self::TEST_FILES_SHARING_API_USER4] = [];

$rootFolder = Server::get(IRootFolder::class);
$shareManager = Server::get(\OCP\Share\IManager::class);

$this->rootView = new View('');
$this->loginAsUser(self::TEST_FILES_SHARING_API_USER1);
$view1 = new View('/' . self::TEST_FILES_SHARING_API_USER1 . '/files');
$view1->mkdir('/sub1/sub2/folder/inside');
$view1->mkdir('/directReshare');
Expand Down Expand Up @@ -98,9 +98,9 @@ protected function setUpShares() {
$share = $shareManager->createShare($share);
$this->shareManager->acceptShare($share, self::TEST_FILES_SHARING_API_USER2);

$this->fileIds[self::TEST_FILES_SHARING_API_USER1][''] = $view1->getFileInfo('')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER1]['sub1'] = $view1->getFileInfo('sub1')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER1]['sub1/sub2'] = $view1->getFileInfo('sub1/sub2')->getId();
$this->fileInfos[self::TEST_FILES_SHARING_API_USER1][''] = $view1->getFileInfo('');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER1]['sub1'] = $view1->getFileInfo('sub1');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER1]['sub1/sub2'] = $view1->getFileInfo('sub1/sub2');

/*
* User 2
Expand All @@ -109,7 +109,6 @@ protected function setUpShares() {
$view2 = new View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files');
$view2->mkdir('/sub1/sub2');
$view2->rename('/folder', '/sub1/sub2/folder');
$this->loginAsUser(self::TEST_FILES_SHARING_API_USER2);

$insideInfo = $view2->getFileInfo('/sub1/sub2/folder/inside');
$this->assertInstanceOf('\OC\Files\FileInfo', $insideInfo);
Expand Down Expand Up @@ -139,9 +138,9 @@ protected function setUpShares() {
$share = $shareManager->createShare($share);
$this->shareManager->acceptShare($share, self::TEST_FILES_SHARING_API_USER4);

$this->fileIds[self::TEST_FILES_SHARING_API_USER2][''] = $view2->getFileInfo('')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER2]['sub1'] = $view2->getFileInfo('sub1')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER2]['sub1/sub2'] = $view2->getFileInfo('sub1/sub2')->getId();
$this->fileInfos[self::TEST_FILES_SHARING_API_USER2][''] = $view2->getFileInfo('');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER2]['sub1'] = $view2->getFileInfo('sub1');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER2]['sub1/sub2'] = $view2->getFileInfo('sub1/sub2');

/*
* User 3
Expand All @@ -150,9 +149,9 @@ protected function setUpShares() {
$view3 = new View('/' . self::TEST_FILES_SHARING_API_USER3 . '/files');
$view3->mkdir('/sub1/sub2');
$view3->rename('/folder', '/sub1/sub2/folder');
$this->fileIds[self::TEST_FILES_SHARING_API_USER3][''] = $view3->getFileInfo('')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER3]['sub1'] = $view3->getFileInfo('sub1')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER3]['sub1/sub2'] = $view3->getFileInfo('sub1/sub2')->getId();
$this->fileInfos[self::TEST_FILES_SHARING_API_USER3][''] = $view3->getFileInfo('');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER3]['sub1'] = $view3->getFileInfo('sub1');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER3]['sub1/sub2'] = $view3->getFileInfo('sub1/sub2');

/*
* User 4
Expand All @@ -161,18 +160,9 @@ protected function setUpShares() {
$view4 = new View('/' . self::TEST_FILES_SHARING_API_USER4 . '/files');
$view4->mkdir('/sub1/sub2');
$view4->rename('/inside', '/sub1/sub2/inside');
$this->fileIds[self::TEST_FILES_SHARING_API_USER4][''] = $view4->getFileInfo('')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER4]['sub1'] = $view4->getFileInfo('sub1')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER4]['sub1/sub2'] = $view4->getFileInfo('sub1/sub2')->getId();

foreach ($this->fileIds as $user => $ids) {
$this->loginAsUser($user);
foreach ($ids as $id) {
$path = $this->rootView->getPath($id);
$ls = $this->rootView->getDirectoryContent($path);
$this->fileEtags[$id] = $this->rootView->getFileInfo($path)->getEtag();
}
}
$this->fileInfos[self::TEST_FILES_SHARING_API_USER4][''] = $view4->getFileInfo('');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER4]['sub1'] = $view4->getFileInfo('sub1');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER4]['sub1/sub2'] = $view4->getFileInfo('sub1/sub2');
}

public function testOwnerWritesToShare(): void {
Expand Down
8 changes: 4 additions & 4 deletions apps/files_sharing/tests/ExpireSharesJobTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
use OCP\Files\IRootFolder;
use OCP\IDBConnection;
use OCP\IUser;
use OCP\IUserManager;
use OCP\Server;
use OCP\Share\IManager;
use OCP\Share\IShare;
use PHPUnit\Framework\Attributes\DataProvider;
use Test\Traits\UserTrait;

/**
* Class ExpireSharesJobTest
Expand All @@ -28,6 +28,7 @@
*/
#[\PHPUnit\Framework\Attributes\Group(name: 'DB')]
class ExpireSharesJobTest extends \Test\TestCase {
use UserTrait;

private ExpireSharesJob $job;

Expand All @@ -50,9 +51,8 @@ protected function setUp(): void {
$user1 = $this->getUniqueID('user1_');
$user2 = $this->getUniqueID('user2_');

$userManager = Server::get(IUserManager::class);
$this->user1 = $userManager->createUser($user1, 'longrandompassword');
$this->user2 = $userManager->createUser($user2, 'longrandompassword');
$this->user1 = $this->createUser($user1, 'longrandompassword');
$this->user2 = $this->createUser($user2, 'longrandompassword');

\OC::registerShareHooks(Server::get(SystemConfig::class));

Expand Down
6 changes: 3 additions & 3 deletions apps/files_sharing/tests/External/ManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class ManagerTest extends TestCase {
protected ICloudFederationFactory&MockObject $cloudFederationFactory;
protected IGroupManager&MockObject $groupManager;
protected IUserManager&MockObject $userManager;
protected ISetupManager&MockObject $setupManager;
protected ISetupManager&MockObject $setupManagerEncTrait;
protected ICertificateManager&MockObject $certificateManager;
private ExternalShareMapper $externalShareMapper;
private IConfig $config;
Expand All @@ -87,7 +87,7 @@ protected function setUp(): void {
$this->groupManager = $this->createMock(IGroupManager::class);
$this->userManager = $this->createMock(IUserManager::class);
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->setupManager = $this->createMock(ISetupManager::class);
$this->setupManagerEncTrait = $this->createMock(ISetupManager::class);
$this->rootFolder = $this->createMock(IRootFolder::class);
$this->rootFolder->method('getUserFolder')
->willReturnCallback(function (string $userId): Folder {
Expand Down Expand Up @@ -169,7 +169,7 @@ private function createManagerForUser(IUser $user): Manager&MockObject {
$this->eventDispatcher,
$this->logger,
$this->rootFolder,
$this->setupManager,
$this->setupManagerEncTrait,
$this->certificateManager,
$this->externalShareMapper,
$this->config,
Expand Down
12 changes: 12 additions & 0 deletions apps/files_sharing/tests/ExternalStorageTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
use OCP\Http\Client\IClientService;
use OCP\Http\Client\IResponse;
use OCP\ICertificateManager;
use OCP\OCM\IOCMDiscoveryService;
use OCP\OCM\IOCMProvider;
use OCP\Server;

/**
Expand Down Expand Up @@ -61,6 +63,9 @@ private function getTestStorage($uri) {
$manager = $this->createMock(ExternalShareManager::class);
$client = $this->createMock(IClient::class);
$response = $this->createMock(IResponse::class);
$discoveryService = $this->createMock(IOCMDiscoveryService::class);
$ocmProvider = $this->createMock(IOCMProvider::class);

$client
->expects($this->any())
->method('get')
Expand All @@ -73,6 +78,12 @@ private function getTestStorage($uri) {
->expects($this->any())
->method('newClient')
->willReturn($client);
$discoveryService->method('discover')
->willReturn($ocmProvider);
$ocmProvider->method('extractProtocolEntry')
->willReturn('/public.php/webdav');
$ocmProvider->method('getEndPoint')
->willReturn($uri);

return new TestSharingExternalStorage(
[
Expand All @@ -85,6 +96,7 @@ private function getTestStorage($uri) {
'manager' => $manager,
'certificateManager' => $certificateManager,
'HttpClientService' => $httpClientService,
'discoveryService' => $discoveryService,
]
);
}
Expand Down
38 changes: 15 additions & 23 deletions apps/files_sharing/tests/GroupEtagPropagationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ class GroupEtagPropagationTest extends PropagationTestCase {
* "user4" (in group 3)
*/
protected function setUpShares() {
$this->fileIds[self::TEST_FILES_SHARING_API_USER1] = [];
$this->fileIds[self::TEST_FILES_SHARING_API_USER2] = [];
$this->fileIds[self::TEST_FILES_SHARING_API_USER3] = [];
$this->fileIds[self::TEST_FILES_SHARING_API_USER4] = [];
$this->fileInfos[self::TEST_FILES_SHARING_API_USER1] = [];
$this->fileInfos[self::TEST_FILES_SHARING_API_USER2] = [];
$this->fileInfos[self::TEST_FILES_SHARING_API_USER3] = [];
$this->fileInfos[self::TEST_FILES_SHARING_API_USER4] = [];

$this->rootView = new View('');
$this->loginAsUser(self::TEST_FILES_SHARING_API_USER1);
Expand All @@ -42,9 +42,9 @@ protected function setUpShares() {
Constants::PERMISSION_ALL
);
$this->shareManager->acceptShare($share, self::TEST_FILES_SHARING_API_USER2);
$this->fileIds[self::TEST_FILES_SHARING_API_USER1][''] = $view1->getFileInfo('')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER1]['test'] = $view1->getFileInfo('test')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER1]['test/sub'] = $view1->getFileInfo('test/sub')->getId();
$this->fileInfos[self::TEST_FILES_SHARING_API_USER1][''] = $view1->getFileInfo('');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER1]['test'] = $view1->getFileInfo('test');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER1]['test/sub'] = $view1->getFileInfo('test/sub');

$this->loginAsUser(self::TEST_FILES_SHARING_API_USER2);
$view2 = new View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files');
Expand All @@ -66,28 +66,20 @@ protected function setUpShares() {
);
$this->shareManager->acceptShare($share, self::TEST_FILES_SHARING_API_USER4);

$this->fileIds[self::TEST_FILES_SHARING_API_USER2][''] = $view2->getFileInfo('')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER2]['test'] = $view2->getFileInfo('test')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER2]['test/sub'] = $view2->getFileInfo('test/sub')->getId();
$this->fileInfos[self::TEST_FILES_SHARING_API_USER2][''] = $view2->getFileInfo('');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER2]['test'] = $view2->getFileInfo('test');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER2]['test/sub'] = $view2->getFileInfo('test/sub');

$this->loginAsUser(self::TEST_FILES_SHARING_API_USER3);
$view3 = new View('/' . self::TEST_FILES_SHARING_API_USER3 . '/files');
$this->fileIds[self::TEST_FILES_SHARING_API_USER3][''] = $view3->getFileInfo('')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER3]['test'] = $view3->getFileInfo('test')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER3]['test/sub'] = $view3->getFileInfo('test/sub')->getId();
$this->fileInfos[self::TEST_FILES_SHARING_API_USER3][''] = $view3->getFileInfo('');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER3]['test'] = $view3->getFileInfo('test');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER3]['test/sub'] = $view3->getFileInfo('test/sub');

$this->loginAsUser(self::TEST_FILES_SHARING_API_USER4);
$view4 = new View('/' . self::TEST_FILES_SHARING_API_USER4 . '/files');
$this->fileIds[self::TEST_FILES_SHARING_API_USER4][''] = $view4->getFileInfo('')->getId();
$this->fileIds[self::TEST_FILES_SHARING_API_USER4]['sub'] = $view4->getFileInfo('sub')->getId();

foreach ($this->fileIds as $user => $ids) {
$this->loginAsUser($user);
foreach ($ids as $id) {
$path = $this->rootView->getPath($id);
$this->fileEtags[$id] = $this->rootView->getFileInfo($path)->getEtag();
}
}
$this->fileInfos[self::TEST_FILES_SHARING_API_USER4][''] = $view4->getFileInfo('');
$this->fileInfos[self::TEST_FILES_SHARING_API_USER4]['sub'] = $view4->getFileInfo('sub');
}

public function testGroupReShareRecipientWrites(): void {
Expand Down
20 changes: 4 additions & 16 deletions apps/files_sharing/tests/LockingTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@
use OC\Files\Filesystem;
use OC\Files\View;
use OCP\Constants;
use OCP\IUserManager;
use OCP\Lock\ILockingProvider;
use OCP\Lock\LockedException;
use OCP\Server;
use OCP\Share\IShare;
use Test\Traits\UserTrait;

/**
* Class LockingTest
Expand All @@ -24,24 +23,18 @@
*/
#[\PHPUnit\Framework\Attributes\Group(name: 'DB')]
class LockingTest extends TestCase {
/**
* @var \Test\Util\User\Dummy
*/
private $userBackend;
use UserTrait;

private $ownerUid;
private $recipientUid;

protected function setUp(): void {
parent::setUp();

$this->userBackend = new \Test\Util\User\Dummy();
Server::get(IUserManager::class)->registerBackend($this->userBackend);

$this->ownerUid = $this->getUniqueID('owner_');
$this->recipientUid = $this->getUniqueID('recipient_');
$this->userBackend->createUser($this->ownerUid, '');
$this->userBackend->createUser($this->recipientUid, '');
$this->createUser($this->ownerUid, '');
$this->createUser($this->recipientUid, '');

$this->loginAsUser($this->ownerUid);
Filesystem::mkdir('/foo');
Expand All @@ -60,11 +53,6 @@ protected function setUp(): void {
$this->assertTrue(Filesystem::file_exists('bar.txt'));
}

protected function tearDown(): void {
Server::get(IUserManager::class)->removeBackend($this->userBackend);
parent::tearDown();
}


public function testLockAsRecipient(): void {
$this->expectException(LockedException::class);
Expand Down
Loading
Loading