Skip to content

test(user_ldap): speed up AbstractMappingTestCase chunking test#60756

Open
miaulalala wants to merge 1 commit into
masterfrom
test/noid/ldap-mapping-test-speedup
Open

test(user_ldap): speed up AbstractMappingTestCase chunking test#60756
miaulalala wants to merge 1 commit into
masterfrom
test/noid/ldap-mapping-test-speedup

Conversation

@miaulalala
Copy link
Copy Markdown
Contributor

@miaulalala miaulalala commented May 27, 2026

Summary

  • The testGetListOfIdsByDn test mapped a DN every 20 entries across 66k entries, producing 3332 DB inserts just to exercise query chunking behaviour
  • Only 14 mapped entries are needed to exceed Postgres's 65535-value IN limit and trigger chunking — map every 5000th entry instead (14 inserts, down from 3332)
  • Clarify the loop comment: it explains why the list size forces chunking, not a per-iteration invariant

Split out from #60739 at reviewer request.

Test plan

  • NOCOVERAGE=1 ./autotest.sh sqlite apps/user_ldap/tests/Mapping/ passes
  • Test still exercises the chunking code path (list of 66640 DNs exceeds 65535 limit)
  • Assertion count matches: 14 mapped entries (66640 / 5000 rounded = 13, plus index 0 = 14)

🤖 Generated with Claude Code

@miaulalala miaulalala requested a review from a team as a code owner May 27, 2026 07:52
@miaulalala miaulalala requested review from CarlSchwan, leftybournes, provokateurin and salmart-dev and removed request for a team May 27, 2026 07:52
@miaulalala miaulalala self-assigned this May 27, 2026
@miaulalala miaulalala added 3. to review Waiting for reviews tests Related to tests CI AI assisted labels May 27, 2026
@miaulalala miaulalala added this to the Nextcloud 35 milestone May 27, 2026
@miaulalala miaulalala requested review from Copilot and kesselb May 27, 2026 07:53
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Speeds up the LDAP mapping chunking test by drastically reducing the number of DB inserts while still exercising the getListOfIdsByDn() query chunking path.

Changes:

  • Map only every 5000th DN (instead of every 20th) to reduce inserts from thousands to a handful.
  • Update the explanatory loop comment for why the DN list size triggers chunking behavior.
  • Adjust the assertion to match the new number of mapped entries returned.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/user_ldap/tests/Mapping/AbstractMappingTestCase.php Outdated
@miaulalala miaulalala force-pushed the test/noid/ldap-mapping-test-speedup branch from bf3982e to 40456ae Compare May 27, 2026 08:40
@miaulalala
Copy link
Copy Markdown
Contributor Author

/backport to stable34

Copy link
Copy Markdown
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technical detail, we can stay accurate. Would keep Postgres as reference as well actually.

Comment thread apps/user_ldap/tests/Mapping/AbstractMappingTestCase.php Outdated
Reduce mapped entries from 3332 to 14 (every 5000th instead of every
20th) so the test exercises the chunking path without inserting thousands
of rows. Move the explanatory comment above the loop where it belongs.

Note: the implementation chunks at its own 65000 total-parameter limit
(not Postgres's 65535 IN-list limit), so the comment uses 65000.

Signed-off-by: Anna Larch <anna@nextcloud.com>
AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@miaulalala miaulalala force-pushed the test/noid/ldap-mapping-test-speedup branch from bc5a7ab to 20d8540 Compare May 27, 2026 14:07
@miaulalala miaulalala requested a review from blizzz May 27, 2026 16:30
@miaulalala miaulalala enabled auto-merge May 28, 2026 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants