fix(test): remove order-dependent assertions in CacheTest::testExtended#60533
Open
miaulalala wants to merge 2 commits into
Open
fix(test): remove order-dependent assertions in CacheTest::testExtended#60533miaulalala wants to merge 2 commits into
miaulalala wants to merge 2 commits into
Conversation
miaulalala
requested review from
Altahrim,
ArtificialOwl,
icewind1991 and
salmart-dev
and removed request for
a team
Contributor
Author
|
/backport to stable34 |
Contributor
Author
|
/backport to stable33 |
Contributor
Author
|
/backport to stable32 |
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 <noreply@anthropic.com> Signed-off-by: Anna Larch <anna@nextcloud.com>
- 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 <anna@nextcloud.com> AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
miaulalala
force-pushed
the
fix/cache-test-order-dependency
branch
from
92809c3 to
32fda16
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes several intermittently failing PHPUnit tests spotted while reviewing #60453.
CacheTest::testExtended
CacheTest::testExtended(and its subclassCachePermissionsMaskTest::testExtended) callsgetFolderContents('')and then asserts against$entries[0]--$entries[3]by position.getFolderContentsByIdhas noORDER BY, so the result order is non-deterministic across DB engines and under parallel test execution:Fix: index the returned entries by name before asserting.
QuotaTest::testStreamCopyNotEnoughSpace
When
free_space()cannot compute used disk size it returnsSPACE_NOT_COMPUTED(-1). The oldQuota::fopencode only wrapped the stream when$free >= 0, so a -1 result caused the raw, unwrapped stream to be returned, allowing unlimited writes.stream_copy_to_streamthen returned the full byte count instead offalse, breaking the assertion.Fix: when space usage is unknown, wrap with the full quota value as a conservative ceiling.
CleanupShareTargetTest::testRepairMultipleConflicting
tearDowntruncates the entirefilecachetable.DefaultShareProvider::getShareByIdJOINssharewithfilecache-- if a test user's home folder entry was wiped and not yet recreated beforeupdateShareis called, the JOIN finds nothing and throwsShareNotFound.Fix: call
getUserFolder()->getDirectoryListing()for all three test users insetUpto ensure their home folders are in the filecache before any share operations.DBConfigServiceTest::testSetMountPoint / testGetAllMounts
Mounts left over from a previously crashed test run remain in the DB and are returned by
getAllMounts(), causing count mismatches and unexpected nulls.Fix: delete all existing mounts in
setUpbefore each test.StoragesServiceTestCase::testGetStoragesBackendNotVisible / testGetStoragesAuthMechanismNotVisible
Same root cause as above: leftover mounts from a previous run inflate the results of
getAllStorages().Fix: added
cleanAll()toCleaningDBConfig(removes all mounts regardless of whether they were created by the current instance) and call it at the top ofsetUp.Checklist
AI (if applicable)
AI-Assisted-By: Claude Sonnet 4.6 noreply@anthropic.com