Skip to content

test(files): make FolderTest multi-storage pagination coverage deterministic#61579

Draft
joshtrichards wants to merge 2 commits into
masterfrom
jtr/fix-test-sharding-FolderTest
Draft

test(files): make FolderTest multi-storage pagination coverage deterministic#61579
joshtrichards wants to merge 2 commits into
masterfrom
jtr/fix-test-sharding-FolderTest

Conversation

@joshtrichards

Copy link
Copy Markdown
Member
  • Resolves: #

Summary

This fixes flaky multi-storage search pagination coverage in FolderTest and adds clearer separation between default-order and explicit-order assertions.

Changes:

  • applied minor clarity/readability refactoring to Folder::search() (originally thought this may be a production code matter)
  • split the existing sub-storage pagination coverage into:
    • default-order tests that verify safe invariants only
    • explicit-order tests that verify exact path ordering when the sort is deterministic
  • removed reliance on implicit cross-storage fileid ordering in test expectations
  • extracted shared fixture/query setup helpers to reduce duplication and make the tests easier to follow

Why:

The previous test mixed two different concerns:

  • pagination behavior with no explicit ordering
  • pagination behavior with explicit deterministic ordering

For multi-storage search results, asserting an exact default order across sub-storages was brittle and could fail depending on how file IDs were assigned across caches. The updated tests keep coverage for the default case, but only assert properties that are safe under unspecified ordering, while preserving strict sequence assertions for explicitly ordered queries.

  • fixes a flaky CI/test failure unrelated to the PR under test
  • preserves useful pagination coverage
  • makes the intent of the test suite clearer
  • avoids over-asserting ordering semantics that the fixture does not guarantee

Fixes failures like https://github.com/nextcloud/server/actions/runs/28053658919/job/83050710177?pr=61555

There was 1 failure:

1) Test\Files\Node\FolderTest::testSearchSubStoragesLimitOffset with data set #1 (0, 5, ['/bar/foo/foo1', '/bar/foo/foo2', '/bar/foo/foo3', '/bar/foo/foo4', '/bar/foo/sub1/foo5'], [])
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
     1 => '/bar/foo/foo2'
     2 => '/bar/foo/foo3'
     3 => '/bar/foo/foo4'
-    4 => '/bar/foo/sub1/foo5'
+    4 => '/bar/foo/sub2/foo7'
 )

/home/runner/actions-runner/_work/server/server/tests/lib/Files/Node/FolderTest.php:1061

TODO

  • Double-check
  • Backport to 34 + 33?

Checklist

AI (if applicable)

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

- clearer and slightly more robust owner-limited home-folder validation
- replace array_map/array_merge with nested foreach loop for better readability, performance, and maintainability
- eliminate mixed usage of of $this->getPath() and $this->path for local for path-related checks; introduce local $currentPath for readability/micro-optimization in loop
- improve comments around cache merging, filtering, and ordering
- skip sorting when the result set has fewer than two items
- drop redundant docblock; covered in interface

Signed-off-by: Josh <josh.t.richards@gmail.com>
- make multi-storage search pagination test deterministic
- split sub-storage pagination coverage by explicit vs default ordering

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added this to the Nextcloud 35 milestone Jun 24, 2026
@joshtrichards joshtrichards added 2. developing Work in progress tests Related to tests developer experience ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Jun 24, 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 developer experience ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) tests Related to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant