test: improve test coverage (#568)

* feat: add more tests and update the README.md

* update go test command

* update go version test in CI

Author: cpuschma

.githooks/pre-push

@@ -9,7 +9,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go: ["1.24", "1.23"]
+        go: ["1.25", "1.24"]
         directory: ["./v3"]
     name: Go ${{ matrix.go }}.x PR Validate ${{ matrix.directory }} (Modules)
     steps:
@@ -31,6 +31,5 @@ jobs:
         run: |
           cd ${{ matrix.directory }}
           go vet .
-          go test .
-          go test -cover -race -cpu 1,2,4 .
+          go test -v -cover -race -count=1 .
           go build .

.github/workflows/pr.yml

@@ -1,6 +1,6 @@
-.PHONY: default install build test quicktest fmt vet lint
+.PHONY: default install build test test fmt vet lint
 
-default: fmt vet lint build quicktest
+default: fmt vet lint build test
 
 CONTAINER_CMD := $(shell command -v podman 2>/dev/null)
 ifeq ($(CONTAINER_CMD),)
@@ -13,7 +13,7 @@ ifeq ($(CONTAINER_CMD),)
 endif
 
 install:
-	go get -t -v ./...
+	go get -t -x ./...
 
 build:
 	go build -v ./...
@@ -51,11 +51,11 @@ local-server:
 	@echo "Loading LDIF files..."
 	@$(CONTAINER_CMD) exec $(CONTAINER_NAME) /bin/sh -c 'for file in /testdata/*.ldif; do echo "Processing $$file..."; cat "$$file" | ldapadd -v -x -H $(LDAP_URL) -D "$(LDAP_ADMIN_DN)" -w $(LDAP_ADMIN_PASSWORD); done'
 
-delete-container:
+stop-local-server:
 	-$(CONTAINER_CMD) rm -f -t 10 $(CONTAINER_NAME)
 
-quicktest:
-	go test ./...
+test:
+	go test -v -cover -race -count=1 .
 
 fuzz:
 	go test -fuzz=FuzzParseDN				-fuzztime=600s .

Makefile

@@ -15,15 +15,20 @@ The library implements the following specifications:
 ## Features:
 
 - Connecting to LDAP server (non-TLS, TLS, STARTTLS, through a custom dialer)
-- Binding to LDAP server (Simple Bind, GSSAPI, SASL)
+- Bind Requests / Responses (Simple Bind, GSSAPI, SASL)
 - "Who Am I" Requests / Responses
-- Searching for entries (normal and asynchronous)
-- Filter Compile / Decompile
-- Paging Search Results
+- Search Requests / Responses (normal, paging and asynchronous)
 - Modify Requests / Responses
 - Add Requests / Responses
 - Delete Requests / Responses
 - Modify DN Requests / Responses
+- Unbind Requests / Responses
+- Password Modify Requests / Responses
+- Content Synchronization Requests / Responses
+- LDAPv3 Filter Compile / Decompile
+- Server Side Sorting of Search Results
+- LDAPv3 Extended Operations
+- LDAPv3 Control Support
 
 ## Go Modules:
 
@@ -36,13 +41,16 @@ Bug reports and pull requests are welcome!
 Before submitting a pull request, please make sure tests and verification scripts pass:
 
 ```
-make all
-```
+# Setup local directory server using Docker or Podman
+make local-server
 
-To set up a pre-push hook to run the tests and verify scripts before pushing:
+# Run gofmt, go vet and go test
+cd ./v3
+make -f ../Makefile
 
-```
-ln -s ../../.githooks/pre-push .git/hooks/pre-push
+# (Optionally) Stop and delete the directory server container afterwards
+cd ..
+make stop-local-server
 ```
 
 ---

README.md

@@ -1,7 +1,9 @@
 package ldap
 
 import (
+	"errors"
 	"fmt"
+
 	ber "github.com/go-asn1-ber/asn1-ber"
 )
 
@@ -66,6 +68,10 @@ func NewAddRequest(dn string, controls []Control) *AddRequest {
 
 // Add performs the given AddRequest
 func (l *Conn) Add(addRequest *AddRequest) error {
+	if addRequest == nil {
+		return NewError(ErrorNetwork, errors.New("AddRequest cannot be nil"))
+	}
+
 	msgCtx, err := l.doRequest(addRequest)
 	if err != nil {
 		return err

v3/add.go

@@ -0,0 +1,62 @@
+package ldap
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestConn_Add(t *testing.T) {
+	l, err := getTestConnection(true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer l.Close()
+
+	dn := "cn=new_user,ou=people,dc=example,dc=com"
+	// Delete the entry if it already exists from previous test runs
+	_ = l.Del(NewDelRequest(dn, nil))
+
+	t.Run("create entry", func(t *testing.T) {
+		err := l.Add(&AddRequest{
+			DN: dn,
+			Attributes: []Attribute{
+				{
+					Type: "objectClass",
+					Vals: []string{"top", "person", "organizationalPerson", "inetOrgPerson"},
+				},
+				{
+					Type: "cn",
+					Vals: []string{"testuser"},
+				},
+				{
+					Type: "givenName",
+					Vals: []string{"Test User"},
+				},
+				{
+					Type: "sn",
+					Vals: []string{"Dummy"},
+				},
+			},
+		})
+		assert.NoError(t, err)
+	})
+	t.Run("create entry with no attributes", func(t *testing.T) {
+		err := l.Add(&AddRequest{
+			DN:         dn,
+			Attributes: nil,
+		})
+		assert.Error(t, err)
+		assert.Truef(t, IsErrorWithCode(err, LDAPResultProtocolError), "Expected LDAPResultProtocolError, got %v", err)
+	})
+	t.Run("empty AddRequest", func(t *testing.T) {
+		err := l.Add(&AddRequest{})
+		assert.Error(t, err)
+	})
+	t.Run("nil AddRequest", func(t *testing.T) {
+		err := l.Add(nil)
+		fmt.Println("expected AddRequest, got nil")
+		assert.Error(t, err)
+	})
+}

v3/add_test.go

@@ -0,0 +1,68 @@
+package ldap
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestConn_Bind(t *testing.T) {
+	l, err := getTestConnection(false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer l.Close()
+
+	tests := []struct {
+		name      string
+		dn        string
+		password  string
+		wantError bool
+		errorCode uint16
+	}{
+		{
+			name:      "invalid credentials",
+			dn:        "cn=admin," + baseDN,
+			password:  "AAAAAAAAAA",
+			wantError: true,
+			errorCode: LDAPResultInvalidCredentials,
+		},
+		{
+			name:      "no credentials",
+			dn:        "",
+			password:  "",
+			wantError: true,
+			errorCode: ErrorEmptyPassword,
+		},
+		{
+			name:      "valid credentials",
+			dn:        "cn=admin," + baseDN,
+			password:  "admin123",
+			wantError: false,
+			errorCode: LDAPResultSuccess,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := l.Bind(tt.dn, tt.password)
+			if tt.wantError {
+				assert.Error(t, err)
+				assert.Truef(t, IsErrorWithCode(err, tt.errorCode), "Expected error code %v, got %d", tt.errorCode, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestConn_UnauthenticatedBind(t *testing.T) {
+	l, err := getTestConnection(false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer l.Close()
+
+	err = l.UnauthenticatedBind("cn=admin," + baseDN)
+	assert.Error(t, err)
+	assert.Truef(t, IsErrorWithCode(err, LDAPResultUnwillingToPerform), "Expected LDAPResultUnwillingToPerform, got %v", err)
+}

v3/bind_test.go

@@ -1,7 +1,9 @@
 package ldap
 
 import (
+	"errors"
 	"fmt"
+
 	ber "github.com/go-asn1-ber/asn1-ber"
 )
 
@@ -35,6 +37,10 @@ func NewDelRequest(DN string, Controls []Control) *DelRequest {
 
 // Del executes the given delete request
 func (l *Conn) Del(delRequest *DelRequest) error {
+	if delRequest == nil {
+		return NewError(ErrorNetwork, errors.New("DelRequest cannot be nil"))
+	}
+
 	msgCtx, err := l.doRequest(delRequest)
 	if err != nil {
 		return err

v3/del.go

@@ -0,0 +1,97 @@
+package ldap
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestConn_Del(t *testing.T) {
+	l, err := getTestConnection(true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer l.Close()
+
+	dn := "cn=testuser,ou=people,dc=example,dc=com"
+
+	// Remove the entry if it exists from previous test runs
+	_ = l.Del(NewDelRequest(dn, nil))
+
+	assert.NoError(t, l.Add(&AddRequest{
+		DN: dn,
+		Attributes: []Attribute{
+			{
+				Type: "objectClass",
+				Vals: []string{"top", "person", "organizationalPerson", "inetOrgPerson"},
+			},
+			{
+				Type: "cn",
+				Vals: []string{"testuser"},
+			},
+			{
+				Type: "givenName",
+				Vals: []string{"Test User"},
+			},
+			{
+				Type: "sn",
+				Vals: []string{"Dummy"},
+			},
+		},
+	}))
+	t.Logf("Added user")
+
+	tests := []struct {
+		name      string
+		dn        string
+		wantErr   bool
+		errorCode uint16
+	}{
+		{
+			name:      "empty DN",
+			dn:        "",
+			wantErr:   true,
+			errorCode: LDAPResultUnwillingToPerform,
+		},
+		{
+			name:      "invalid DN",
+			dn:        "AAAAAAAAAAAAAAAAAA",
+			wantErr:   true,
+			errorCode: LDAPResultInvalidDNSyntax,
+		},
+		{
+			name:    "delete user",
+			dn:      dn,
+			wantErr: false,
+		},
+		{
+			name:      "delete entry with children",
+			dn:        "ou=people," + baseDN,
+			wantErr:   true,
+			errorCode: LDAPResultNotAllowedOnNonLeaf,
+		},
+		{
+			name:      "delete non existing entry",
+			dn:        "ou=nonexisting," + baseDN,
+			wantErr:   true,
+			errorCode: LDAPResultNoSuchObject,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			delRequest := NewDelRequest(tt.dn, nil)
+			err := l.Del(delRequest)
+			if tt.wantErr && err != nil {
+				assert.Error(t, err)
+				assert.Truef(t, IsErrorWithCode(err, tt.errorCode), "Expected error with code %d, got %d", tt.errorCode, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+
+	t.Run("nil DelRequest", func(t *testing.T) {
+		err := l.Del(nil)
+		assert.Error(t, err)
+	})
+}

v3/del_test.go

@@ -1,7 +1,9 @@
 package ldap
 
 import (
+	"errors"
 	"fmt"
+
 	ber "github.com/go-asn1-ber/asn1-ber"
 )
 
@@ -56,6 +58,10 @@ type ExtendedResponse struct {
 // Extended performs an extended request. The resulting
 // ExtendedResponse may return a value in the form of a *ber.Packet
 func (l *Conn) Extended(er *ExtendedRequest) (*ExtendedResponse, error) {
+	if er == nil {
+		return nil, NewError(ErrorNetwork, errors.New("ExtendedRequest cannot be nil"))
+	}
+
 	msgCtx, err := l.doRequest(er)
 	if err != nil {
 		return nil, err