Skip to content

test(files): expand and improve quota wrapper coverage - #63035

Draft
joshtrichards wants to merge 3 commits into
masterfrom
jtr/test-QuotaWrapper-coverage
Draft

test(files): expand and improve quota wrapper coverage#63035
joshtrichards wants to merge 3 commits into
masterfrom
jtr/test-QuotaWrapper-coverage

Conversation

@joshtrichards

Copy link
Copy Markdown
Member
  • Resolves: #n/a

Summary

Fixes some existing test bugs for OC\Files\Storage\Wrapper\Quota and expands targeted PHPUnit coverage focusing on previously-untested branches and quota- boundary behavior.

Notes

  • The changes are test-only and motivated by some documentation work + possible production code changes (and just generally good test hygiene!); no production code is modified in this PR.
  • Fixes are in the first commit; bulk of new tests are in second commit; third commit adds a new test specifically for external storage quota inclusion handling.

New coverage:

  • Exact quota boundary behavior (file_put_contents/writeStream reject exact-fit writes due to current strict < comparison)
  • quotaCallback invocation and caching (getQuota() only calls the callback once)
  • Unlimited (FileInfo::SPACE_UNLIMITED) and negative quota values delegate to wrapped storage
  • enableQuota(false) disables enforcement
  • cache/ and uploads/ path prefixes bypass quota
  • .part files bypass quota when opened for writing
  • copyFromStorage/moveFromStorage correctly reject and leave source/target untouched when quota is exceeded
  • Sized writeStream() return values and exact-limit rejection (previously the return value wasn't asserted)
  • include_external_storage quota accounting correctly reflects mounted external storage in free_space()

Fix:

  • Removed an invalid assertion in testNoMkdirQuotaZero() (assertFalse($instance->mkdir('files'))). This was passing for the wrong reason: getLimitedStorage() already creates files/ on the underlying storage before the test runs. In addition, shouldApplyQuota() only applies quota to paths matching files/ (with trailing slash), so bare files was never quota-scoped to begin with.

TODO

  • Make sure all tests, particularly the integration ones pass reliably enough in CI run(s)
  • Backport? (I vote at least to v34 to have some better coverage at least thru it's lifecycle)

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Remove the invalid existing-directory mkdir assertion, cover exact quota boundaries for writes, and verify successful writeStream results and cleanup.

Assisted-by: Copilot:gpt-5.6-luna

Signed-off-by: Josh <josh.t.richards@gmail.com>
Cover quota callbacks, unlimited and disabled quotas, negative quotas, bypass paths, part files, sized streams, and cross-storage operations.

Signed-off-by: Josh <josh.t.richards@gmail.com>
Verify that the quota wrapper includes files from mounted external storage when `include_external_storage` is enabled.

Assisted-by: Copilot:gpt-5.6-luna

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added this to the Nextcloud 35 milestone Aug 7, 2026
@joshtrichards joshtrichards added 2. developing Work in progress feature: filesystem tests Related to tests ⚠️ risk: low Unlikely to cause meaningful regressions; limited runtime or compatibility impact. labels Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress AI assisted feature: filesystem ⚠️ risk: low Unlikely to cause meaningful regressions; limited runtime or compatibility impact. tests Related to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants