From fdbd1e0e5a5d6a3495a41d23547b7d56ddb32dfa Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Tue, 19 May 2026 14:59:16 +0200 Subject: [PATCH 1/2] fix(test): remove order-dependent assertions in testExtended getFolderContentsById has no ORDER BY, so getFolderContents returns rows in an arbitrary order that varies across DB engines and parallel runs. Index the result by name instead of relying on array position. AI-Assisted-By: Claude Sonnet 4.6 Signed-off-by: Anna Larch --- tests/lib/Files/Cache/CacheTest.php | 45 ++++++++++++++++------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/tests/lib/Files/Cache/CacheTest.php b/tests/lib/Files/Cache/CacheTest.php index 8d62e449306c1..4ff57b6529d56 100644 --- a/tests/lib/Files/Cache/CacheTest.php +++ b/tests/lib/Files/Cache/CacheTest.php @@ -801,26 +801,31 @@ public function testExtended(): void { $entries = $this->cache->getFolderContents(''); $this->assertCount(4, $entries); - $this->assertEquals('foo1', $entries[0]->getName()); - $this->assertEquals('foo2', $entries[1]->getName()); - $this->assertEquals('foo3', $entries[2]->getName()); - $this->assertEquals('foo4', $entries[3]->getName()); - - $this->assertEquals(20, $entries[0]->getCreationTime()); - $this->assertEquals(0, $entries[0]->getUploadTime()); - $this->assertEquals(null, $entries[0]->getMetadataEtag()); - - $this->assertEquals(0, $entries[1]->getCreationTime()); - $this->assertEquals(30, $entries[1]->getUploadTime()); - $this->assertEquals(null, $entries[1]->getMetadataEtag()); - - $this->assertEquals(0, $entries[2]->getCreationTime()); - $this->assertEquals(0, $entries[2]->getUploadTime()); - $this->assertEquals('foo', $entries[2]->getMetadataEtag()); - - $this->assertEquals(0, $entries[3]->getCreationTime()); - $this->assertEquals(0, $entries[3]->getUploadTime()); - $this->assertEquals(null, $entries[3]->getMetadataEtag()); + // getFolderContentsById has no ORDER BY — index by name to avoid order-dependent assertions + $byName = []; + foreach ($entries as $entry) { + $byName[$entry->getName()] = $entry; + } + $this->assertArrayHasKey('foo1', $byName); + $this->assertArrayHasKey('foo2', $byName); + $this->assertArrayHasKey('foo3', $byName); + $this->assertArrayHasKey('foo4', $byName); + + $this->assertEquals(20, $byName['foo1']->getCreationTime()); + $this->assertEquals(0, $byName['foo1']->getUploadTime()); + $this->assertEquals(null, $byName['foo1']->getMetadataEtag()); + + $this->assertEquals(0, $byName['foo2']->getCreationTime()); + $this->assertEquals(30, $byName['foo2']->getUploadTime()); + $this->assertEquals(null, $byName['foo2']->getMetadataEtag()); + + $this->assertEquals(0, $byName['foo3']->getCreationTime()); + $this->assertEquals(0, $byName['foo3']->getUploadTime()); + $this->assertEquals('foo', $byName['foo3']->getMetadataEtag()); + + $this->assertEquals(0, $byName['foo4']->getCreationTime()); + $this->assertEquals(0, $byName['foo4']->getUploadTime()); + $this->assertEquals(null, $byName['foo4']->getMetadataEtag()); $this->cache->update($id1, ['upload_time' => 25]); From 32fda1600a25f04f3f4635f0f49c64c19d9dface Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Tue, 19 May 2026 19:16:02 +0200 Subject: [PATCH 2/2] fix(test): stabilise flaky DB and quota tests - Quota::fopen: when free_space() returns SPACE_NOT_COMPUTED the previous code skipped wrapping the stream entirely, letting unlimited writes through. Now wraps with the full quota as a conservative ceiling, which makes testStreamCopyNotEnoughSpace deterministic regardless of whether the cache has a valid root-size entry. - CleanupShareTargetTest::setUp: call getUserFolder()->getDirectoryListing() for every test user so their home folders are in the filecache before any share operations. tearDown wipes filecache; without this getShareById (which JOINs on filecache) throws ShareNotFound intermittently on the second createUserShare call. - DBConfigServiceTest::setUp: remove any mounts left by a previously failed test run so testSetMountPoint and testGetAllMounts do not see unexpected extra rows. - StoragesServiceTestCase / CleaningDBConfig: add cleanAll() that removes every mount from the DB (not just those tracked by the current instance) and call it at the top of setUp so testGetStoragesBackendNotVisible and testGetStoragesAuthMechanismNotVisible start with an empty external mount table. Signed-off-by: Anna Larch AI-Assisted-By: Claude Sonnet 4.6 --- apps/files_external/tests/Service/DBConfigServiceTest.php | 4 ++++ .../tests/Service/StoragesServiceTestCase.php | 8 ++++++++ .../files_sharing/tests/Repair/CleanupShareTargetTest.php | 5 +++++ lib/private/Files/Storage/Wrapper/Quota.php | 8 +++++--- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/apps/files_external/tests/Service/DBConfigServiceTest.php b/apps/files_external/tests/Service/DBConfigServiceTest.php index d41c2ee31813c..dbc30bd367e98 100644 --- a/apps/files_external/tests/Service/DBConfigServiceTest.php +++ b/apps/files_external/tests/Service/DBConfigServiceTest.php @@ -26,6 +26,10 @@ protected function setUp(): void { parent::setUp(); $this->connection = Server::get(IDBConnection::class); $this->dbConfig = new DBConfigService($this->connection, Server::get(ICrypto::class)); + // Remove any mounts left by a previously failed test run + foreach ($this->dbConfig->getAllMounts() as $mount) { + $this->dbConfig->removeMount($mount['mount_id']); + } } protected function tearDown(): void { diff --git a/apps/files_external/tests/Service/StoragesServiceTestCase.php b/apps/files_external/tests/Service/StoragesServiceTestCase.php index 5eed8294b8f8c..594684e8236f6 100644 --- a/apps/files_external/tests/Service/StoragesServiceTestCase.php +++ b/apps/files_external/tests/Service/StoragesServiceTestCase.php @@ -50,6 +50,13 @@ public function clean(): void { $this->removeMount($id); } } + + public function cleanAll(): void { + foreach ($this->getAllMounts() as $mount) { + $this->removeMount($mount['mount_id']); + } + $this->mountIds = []; + } } #[\PHPUnit\Framework\Attributes\Group(name: 'DB')] @@ -65,6 +72,7 @@ abstract class StoragesServiceTestCase extends \Test\TestCase { protected function setUp(): void { parent::setUp(); $this->dbConfig = new CleaningDBConfig(Server::get(IDBConnection::class), Server::get(ICrypto::class)); + $this->dbConfig->cleanAll(); // Remove any mounts left by previous test runs self::$hookCalls = []; $config = Server::get(IConfig::class); $this->dataDir = $config->getSystemValue( diff --git a/apps/files_sharing/tests/Repair/CleanupShareTargetTest.php b/apps/files_sharing/tests/Repair/CleanupShareTargetTest.php index aaa653135ce4b..1b86a1336bca8 100644 --- a/apps/files_sharing/tests/Repair/CleanupShareTargetTest.php +++ b/apps/files_sharing/tests/Repair/CleanupShareTargetTest.php @@ -27,6 +27,11 @@ class CleanupShareTargetTest extends TestCase { protected function setUp(): void { parent::setUp(); $this->cleanupShareTarget = Server::get(CleanupShareTarget::class); + // Ensure all test users' home folders are in the filecache before any share + // operations, because tearDown wipes filecache and getShareById JOINs on it. + foreach ([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER3] as $user) { + $this->rootFolder->getUserFolder($user)->getDirectoryListing(); + } } private function createUserShare(string $by, string $target = self::TEST_FOLDER_NAME): IShare { diff --git a/lib/private/Files/Storage/Wrapper/Quota.php b/lib/private/Files/Storage/Wrapper/Quota.php index 11b992dbf9727..a5b4c6ddc5196 100644 --- a/lib/private/Files/Storage/Wrapper/Quota.php +++ b/lib/private/Files/Storage/Wrapper/Quota.php @@ -132,10 +132,12 @@ public function fopen(string $path, string $mode) { } $source = $this->getWrapperStorage()->fopen($path, $mode); - if ($source && (is_int($free) || is_float($free)) && $free >= 0 && $mode !== 'r' && $mode !== 'rb') { - // only apply quota for files, not metadata, trash or others - if ($this->shouldApplyQuota($path)) { + if ($source && $mode !== 'r' && $mode !== 'rb' && $this->shouldApplyQuota($path)) { + if ((is_int($free) || is_float($free)) && $free >= 0) { return \OC\Files\Stream\Quota::wrap($source, $free); + } elseif ($free === FileInfo::SPACE_NOT_COMPUTED) { + // Used space is unknown; apply full quota as a conservative ceiling + return \OC\Files\Stream\Quota::wrap($source, $this->getQuota()); } }