Skip to content

test(carddav): refactor testSearch to use clearer assertions and unique UIDs#60463

Merged
sorbaugh merged 2 commits into
masterfrom
jtr/test-carddav-search
Jun 8, 2026
Merged

test(carddav): refactor testSearch to use clearer assertions and unique UIDs#60463
sorbaugh merged 2 commits into
masterfrom
jtr/test-carddav-search

Conversation

@joshtrichards

@joshtrichards joshtrichards commented May 15, 2026

Copy link
Copy Markdown
Member

Summary

Refactors testSearch() and its data provider in CardDavBackendTest to improve clarity and correctness:

  • Split $expected into $expectedUris + $expectedNeedles: separates two concerns that were previously mixed into a single tuple array
  • Use unique UIDs per VCard (uid-0, uid-1, uid-2): the old shared uid value was invalid per the vCard spec and could mask bugs
  • Replace nested loop assertion with direct URI list comparison + per-card assertNotFalse(strpos(...)), giving actionable failure messages instead of a silent count mismatch
  • Collapse repetitive property inserts into a $propertyRows loop
  • Named keys in data provider entries for readability

No production code changes.

Checklist

AI (if applicable)

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

Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added this to the Nextcloud 35 milestone May 15, 2026
@joshtrichards joshtrichards added 3. to review Waiting for reviews feature: carddav Related to CardDAV internals tests Related to tests labels May 15, 2026
@joshtrichards joshtrichards marked this pull request as ready for review May 15, 2026 20:46
@sorbaugh sorbaugh merged commit ac961a7 into master Jun 8, 2026
238 of 253 checks passed
@sorbaugh sorbaugh deleted the jtr/test-carddav-search branch June 8, 2026 12:16
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 feature: carddav Related to CardDAV internals tests Related to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants