[PR #1315] [MERGED] Move LDAP search tests to their respective implementation files #1252

New issue

Closed

opened 2026-02-27 09:11:29 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 09:11:29 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/lldap/lldap/pull/1315
Author: @Copilot
Created: 10/4/2025
Status: ✅ Merged
Merged: 10/9/2025
Merged by: @nitnelave

Base: main ← Head: copilot/fix-3ad1040b-dda7-4fe2-8342-088c424ac0ab

📝 Commits (6)

789483b Initial plan
a6397a0 Split root_dse and subschema tests into dedicated modules
09771bb Fix unused imports in tests module
bd8decd Move user and group tests to their respective implementation files
69ba493 Add blank lines between tests in search.rs
4eb9171 Run cargo fmt to reformat code

📊 Changes

3 files changed (+700 additions, -736 deletions)

View changed files

📝 crates/ldap/src/core/group.rs (+302 -0)
📝 crates/ldap/src/core/user.rs (+371 -0)
📝 crates/ldap/src/search.rs (+27 -736)

📄 Description

Move LDAP search tests to their respective implementation files

Problem

Tests for LDAP search functionality were all located in search.rs, making it difficult to find and maintain tests when modifying user or group-specific functionality in separate files (core/user.rs, core/group.rs).

Solution

Reorganized tests to be in the same files as their implementations:

User tests → `core/user.rs` (6 tests)

test_search_regular_user - Tests user search with regular permissions
test_search_readonly_user - Tests user search with readonly permissions
test_search_member_of - Tests memberOf attribute for users
test_search_user_as_scope - Tests searching with user as base DN
test_search_users - Tests general user listing and conversion
test_pwd_changed_time_format - Tests password modification timestamp format

Group tests → `core/group.rs` (6 tests)

test_search_groups - Tests general group listing and conversion
test_search_groups_by_groupid - Tests group search by ID
test_search_groups_filter - Tests complex group filter logic
test_search_groups_filter_2 - Tests OR filter for groups
test_search_groups_filter_3 - Tests NOT filter for groups
test_search_group_as_scope - Tests searching with group as base DN

Tests remaining in `search.rs` (19 tests)

Root DSE and subschema tests
General search routing tests
Filter parsing tests
Error handling tests
OU search tests
Mixed user/group tests

Benefits

Better locality: Tests are now adjacent to the code they test
Easier maintenance: Changing user/group conversion logic and tests in same file
Clearer organization: Separation of concerns between search routing and data conversion
Follows Rust best practices: Standard pattern for organizing tests

Testing

All existing tests pass without modification:

68 tests total (unchanged)
0 clippy warnings
All workspace tests pass
No functional changes to code

Test output now shows organized module structure:

test core::user::tests::test_search_users ... ok
test core::group::tests::test_search_groups ... ok
test search::tests::test_search_filters ... ok

Fixes lldap/lldap#1314

Original prompt

This section details on the original issue you should resolve

<issue_title>[FEATURE REQUEST] Split up the tests from ldap/src/search.rs into their respective files</issue_title>
<issue_description>Motivation
When changing the way we handle an LDAP request, we need to change both the handling and the tests, and these are in different files.

Describe the solution you'd like
As much as possible, the tests should be in the same file as the implementation.
</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes lldap/lldap#1314

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/lldap/lldap/pull/1315 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 10/4/2025 **Status:** ✅ Merged **Merged:** 10/9/2025 **Merged by:** [@nitnelave](https://github.com/nitnelave) **Base:** `main` ← **Head:** `copilot/fix-3ad1040b-dda7-4fe2-8342-088c424ac0ab` --- ### 📝 Commits (6) - [`789483b`](https://github.com/lldap/lldap/commit/789483b4e3739738fd827a30ce3a3a369a9cbaa0) Initial plan - [`a6397a0`](https://github.com/lldap/lldap/commit/a6397a0cba64c0ef0336d0505b52514ea4eae9b3) Split root_dse and subschema tests into dedicated modules - [`09771bb`](https://github.com/lldap/lldap/commit/09771bb6bd0beb55204f985bed048b9b4faee1e5) Fix unused imports in tests module - [`bd8decd`](https://github.com/lldap/lldap/commit/bd8decd7668ded8776792d670b7fb1bf465b438c) Move user and group tests to their respective implementation files - [`69ba493`](https://github.com/lldap/lldap/commit/69ba4936aed906120a02d534816a86941a63826f) Add blank lines between tests in search.rs - [`4eb9171`](https://github.com/lldap/lldap/commit/4eb91712eeba424b9622c4d2be3a80b20285131a) Run cargo fmt to reformat code ### 📊 Changes **3 files changed** (+700 additions, -736 deletions) <details> <summary>View changed files</summary> 📝 `crates/ldap/src/core/group.rs` (+302 -0) 📝 `crates/ldap/src/core/user.rs` (+371 -0) 📝 `crates/ldap/src/search.rs` (+27 -736) </details> ### 📄 Description Move LDAP search tests to their respective implementation files ## Problem Tests for LDAP search functionality were all located in `search.rs`, making it difficult to find and maintain tests when modifying user or group-specific functionality in separate files (`core/user.rs`, `core/group.rs`). ## Solution Reorganized tests to be in the same files as their implementations: ### User tests → `core/user.rs` (6 tests) - `test_search_regular_user` - Tests user search with regular permissions - `test_search_readonly_user` - Tests user search with readonly permissions - `test_search_member_of` - Tests memberOf attribute for users - `test_search_user_as_scope` - Tests searching with user as base DN - `test_search_users` - Tests general user listing and conversion - `test_pwd_changed_time_format` - Tests password modification timestamp format ### Group tests → `core/group.rs` (6 tests) - `test_search_groups` - Tests general group listing and conversion - `test_search_groups_by_groupid` - Tests group search by ID - `test_search_groups_filter` - Tests complex group filter logic - `test_search_groups_filter_2` - Tests OR filter for groups - `test_search_groups_filter_3` - Tests NOT filter for groups - `test_search_group_as_scope` - Tests searching with group as base DN ### Tests remaining in `search.rs` (19 tests) - Root DSE and subschema tests - General search routing tests - Filter parsing tests - Error handling tests - OU search tests - Mixed user/group tests ## Benefits - **Better locality**: Tests are now adjacent to the code they test - **Easier maintenance**: Changing user/group conversion logic and tests in same file - **Clearer organization**: Separation of concerns between search routing and data conversion - **Follows Rust best practices**: Standard pattern for organizing tests ## Testing All existing tests pass without modification: - 68 tests total (unchanged) - 0 clippy warnings - All workspace tests pass - No functional changes to code Test output now shows organized module structure: ``` test core::user::tests::test_search_users ... ok test core::group::tests::test_search_groups ... ok test search::tests::test_search_filters ... ok ``` Fixes lldap/lldap#1314  <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[FEATURE REQUEST] Split up the tests from ldap/src/search.rs into their respective files</issue_title> > <issue_description>**Motivation** > When changing the way we handle an LDAP request, we need to change both the handling and the tests, and these are in different files. > > **Describe the solution you'd like** > As much as possible, the tests should be in the same file as the implementation. > </issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> Fixes lldap/lldap#1314  --- ✨ Let Copilot coding agent [set things up for you](https://github.com/lldap/lldap/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>