testing: Builders,FakeClient operations #29

billf · 2025-12-11T04:19:13Z

perf(testing): use RWMutex for read-only FakeClient operations

Convert FakeClient from sync.Mutex to sync.RWMutex to allow concurrent
reads while maintaining exclusive writes. This improves parallel test
performance.

Changes:

Changed mu from sync.Mutex to sync.RWMutex
Read-only methods now use RLock/RUnlock:
- GetServer, GetSSHKey, GetBGPSessions, GetIPs, GetLocations, GetOSs
Added defensive copies for returned slices to prevent caller mutations

Write methods continue to use Lock/Unlock for exclusive access.

Resolves todo #9: consider RWMutex for read-only operations.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com

test(testing): add comprehensive tests for FakeClient and builders

Add thorough test coverage for the testing utilities:

fake_test.go:

FakeClient CRUD operations (server, SSH key, BGP sessions)
Input validation errors for all mutating methods
Error injection via exported error fields
Call tracking with GetCalls()
Reset() method clearing all state
Defensive copy verification for GetLocations

builders_test.go:

All builder types (Server, SSHKey, BGPSession, IPs, Location, OS)
Default values verification
Fluent API method chaining
Quick helper functions (NewRunningServer, NewTestSSHKey, etc.)

All tests pass with race detector: go test -race ./testing

Resolves todo #6: testing utilities have no tests.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com

billf · 2025-12-11T04:19:16Z

This change is part of the following stack:

docs(test): add comprehensive testing #26
- refactor(test): mockery-generated mocks #27
  - fix(tests): various fixes for mocks and fakes #28
    - testing: Builders,FakeClient operations #29 ◀

_{Change managed by git-spice.}

Convert FakeClient from sync.Mutex to sync.RWMutex to allow concurrent reads while maintaining exclusive writes. This improves parallel test performance. Changes: - Changed mu from sync.Mutex to sync.RWMutex - Read-only methods now use RLock/RUnlock: - GetServer, GetSSHKey, GetBGPSessions, GetIPs, GetLocations, GetOSs - Added defensive copies for returned slices to prevent caller mutations Write methods continue to use Lock/Unlock for exclusive access. Resolves todo #9: consider RWMutex for read-only operations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Add thorough test coverage for the testing utilities: fake_test.go: - FakeClient CRUD operations (server, SSH key, BGP sessions) - Input validation errors for all mutating methods - Error injection via exported error fields - Call tracking with GetCalls() - Reset() method clearing all state - Defensive copy verification for GetLocations builders_test.go: - All builder types (Server, SSHKey, BGPSession, IPs, Location, OS) - Default values verification - Fluent API method chaining - Quick helper functions (NewRunningServer, NewTestSSHKey, etc.) All tests pass with race detector: `go test -race ./testing` Resolves todo #6: testing utilities have no tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

github-actions · 2025-12-13T01:06:54Z

Merging this branch will increase overall coverage

Impacted Packages	Coverage Δ	🤖
github.com/andyl-technologies/gona/testing	53.24% (+53.24%)	🌟

Coverage by file

Changed files (no unit tests)

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/andyl-technologies/gona/testing/fake.go	49.10% (+49.10%)	499 (+18)	245 (+245)	254 (-227)	🌟

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

github.com/andyl-technologies/gona/testing/builders_test.go
github.com/andyl-technologies/gona/testing/fake_test.go

Copilot

Pull request overview

This PR enhances the testing utilities with performance optimizations and comprehensive test coverage. The main improvement converts FakeClient from sync.Mutex to sync.RWMutex, enabling concurrent reads while maintaining exclusive writes for better parallel test performance. Additionally, defensive copies are added to read-only methods to prevent caller mutations, and comprehensive tests are introduced for both FakeClient and the builder utilities.

Key Changes:

Optimized FakeClient concurrency by using RWMutex with RLock for read operations
Added defensive copies for GetBGPSessions, GetLocations, and GetOSs methods
Introduced extensive test coverage for all FakeClient CRUD operations, error injection, and call tracking
Added builder tests covering default values, fluent API chaining, and helper functions

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
testing/fake.go	Converted mutex to RWMutex for read operations; added defensive slice copies for GetBGPSessions, GetLocations, and GetOSs
testing/fake_test.go	Added comprehensive tests for FakeClient operations including CRUD, validation, error injection, call tracking, and defensive copy verification
testing/builders_test.go	Added tests for all builder types (Server, SSHKey, BGPSession, IPs, Location, OS) covering defaults, method chaining, and quick helpers

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

Copilot · 2025-12-13T01:10:41Z

testing/fake.go

+	// Return defensive copy
+	result := make([]*gona.BGPSession, len(sessions))
+	copy(result, sessions)


The defensive copy is incomplete. Copying the slice of pointers doesn't prevent callers from modifying the BGPSession objects that the pointers reference. To fully protect internal state, you need to create new BGPSession objects (deep copy). Consider iterating through the sessions and creating copies of each BGPSession object, then returning pointers to those copies.

Suggested change

// Return defensive copy

result := make([]*gona.BGPSession, len(sessions))

copy(result, sessions)

// Return deep defensive copy

result := make([]*gona.BGPSession, len(sessions))

for i, session := range sessions {

if session != nil {

copied := *session

result[i] = &copied

} else {

result[i] = nil

}

}

Copilot · 2025-12-13T01:10:41Z

testing/fake_test.go

+func contains(s, substr string) bool {
+	return len(s) >= len(substr) && (s == substr || len(substr) == 0 ||
+		(len(s) > 0 && len(substr) > 0 && findSubstring(s, substr)))
+}
+
+func findSubstring(s, substr string) bool {
+	for i := 0; i <= len(s)-len(substr); i++ {
+		if s[i:i+len(substr)] == substr {
+			return true
+		}
+	}
+	return false
+}


This helper function reimplements string containment logic that's available in the standard library. Replace this custom implementation with strings.Contains from the strings package for better maintainability and readability.

This was referenced Dec 11, 2025

docs(test): add comprehensive testing #26

Merged

refactor(test): mockery-generated mocks #27

Merged

fix(tests): various fixes for mocks and fakes #28

Merged

billf force-pushed the billf/testing/fixes branch from d369405 to 3a2f36e Compare December 11, 2025 04:22

billf force-pushed the billf/testing/tests branch from c395047 to abfa20d Compare December 11, 2025 04:22

billf force-pushed the billf/testing/fixes branch from 3a2f36e to 8949783 Compare December 12, 2025 23:57

billf force-pushed the billf/testing/tests branch from abfa20d to 83ccbcd Compare December 12, 2025 23:57

billf changed the title ~~perf(testing): use RWMutex for read-only FakeClient operations~~ testing: Builders,FakeClient operations Dec 12, 2025

billf force-pushed the billf/testing/tests branch from 83ccbcd to 021df0d Compare December 13, 2025 00:00

billf and others added 2 commits December 12, 2025 17:06

billf changed the base branch from billf/testing/fixes to master December 13, 2025 01:06

billf force-pushed the billf/testing/tests branch from 021df0d to 7bba61a Compare December 13, 2025 01:06

billf marked this pull request as ready for review December 13, 2025 01:06

Copilot AI review requested due to automatic review settings December 13, 2025 01:06

Copilot started reviewing on behalf of billf December 13, 2025 01:06 View session

Copilot AI reviewed Dec 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

testing: Builders,FakeClient operations #29

testing: Builders,FakeClient operations #29

Uh oh!

billf commented Dec 11, 2025

Uh oh!

billf commented Dec 11, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Dec 13, 2025

Changed files (no unit tests)

Changed unit test files

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 13, 2025

Uh oh!

Copilot AI Dec 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-	// Return defensive copy
-	result := make([]*gona.BGPSession, len(sessions))
-	copy(result, sessions)
+	// Return deep defensive copy
+	result := make([]*gona.BGPSession, len(sessions))
+	for i, session := range sessions {
+		if session != nil {
+			copied := *session
+			result[i] = &copied
+		} else {
+			result[i] = nil
+		}
+	}

testing: Builders,FakeClient operations #29

Are you sure you want to change the base?

testing: Builders,FakeClient operations #29

Uh oh!

Conversation

billf commented Dec 11, 2025

Uh oh!

billf commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 13, 2025

Merging this branch will increase overall coverage

Changed files (no unit tests)

Changed unit test files

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

billf commented Dec 11, 2025 •

edited

Loading