Skip to content

test(dav): make principal search assertions order-independent - #65970

Open
joshtrichards wants to merge 1 commit into
masterfrom
jtr/test-searchPrincipalUri-ordering
Open

joshtrichards wants to merge 1 commit into
masterfrom
jtr/test-searchPrincipalUri-ordering

Conversation

@joshtrichards

Copy link
Copy Markdown
Member

Summary

I didn't experience this, but testSearchPrincipal() assumed that CalDAV principal search results are returned in database-ID order, which the API does not guarantee.

Changes:

  • Normalize the expected and returned calendar payloads, then compare them without depending on result order. Cover both the user's and the sharer's results.

This is a test-only fix; production search behavior is unchanged.

Out of scope: CalDAV searchPrincipalUri() supports limit and offset, but its paginated ID query has no explicit ordering. That can make page membership inconsistent; a follow-up fix should probably define the order before applying pagination.

TODO

  • Backport?

Checklist

AI (if applicable)

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

CalDAV principal search does not guarantee result ordering, but testSearchPrincipal compared returned calendar data by array  index.

Compare normalized payloads canonically instead, so the test verifies the expected results without depending on database row order. Cover both the user's results and the sharer's results.

Resolves #37321

Assisted-by: Copilot:gpt-6-luna

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added this to the Nextcloud 36 milestone Oct 27, 2026
@joshtrichards joshtrichards added the 3. to review Waiting for reviews label Oct 27, 2026
@joshtrichards joshtrichards added the feature: caldav Related to CalDAV internals label Oct 27, 2026
@joshtrichards joshtrichards added the tests Related to tests label Oct 27, 2026
@joshtrichards joshtrichards added the ⚠️ risk: low Unlikely to cause meaningful regressions; limited runtime or compatibility impact. label Oct 27, 2026
@nickvergessen

Copy link
Copy Markdown
Member

CS is now failing 🙈

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews AI assisted feature: caldav Related to CalDAV internals ⚠️ 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.

tests/unit/CalDAV/CalDavBackendTest.php:1278 is flacky

3 participants