Enhance test utilities with comprehensive helper functions #122
Conversation
WalkthroughA new suite of generic test helper functions was introduced in Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Function
participant Helpers as Test Helpers (testutils_test.go)
participant SUT as System Under Test
Test->>Helpers: S(args...) to create slices
Test->>Helpers: AssertEqual/AssertError/AssertPanic(...)
Helpers->>Test: Report assertion results
Test->>Helpers: RunConcurrentTest/RunBenchmark(...)
Helpers->>SUT: Call target code in goroutines/bench loops
Helpers->>Test: Aggregate and report results
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (7)
🧰 Additional context used📓 Path-based instructions (4)**/*.goInstructions used from: Sources:
**/*_test.goInstructions used from: Sources:
**/*Instructions used from: Sources:
**/*.{md,go}Instructions used from: Sources:
🧠 Learnings (2)📓 Common learningstestutils_test.go (7)🔇 Additional comments (14)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
|
Here's the code health analysis summary for commits Analysis Summary
|
amery
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
AGENT.md(3 hunks)addrport_test.go(1 hunks)addrs_test.go(6 hunks)as_test.go(8 hunks)compounderror_test.go(16 hunks)errors_test.go(2 hunks)generics_test.go(5 hunks)internal/build/cspell.json(2 hunks)lists_test.go(1 hunks)maps_test.go(25 hunks)panic_test.go(7 hunks)panicerror_test.go(4 hunks)slices.go(1 hunks)slices_test.go(5 hunks)spinlock_test.go(5 hunks)stack.go(1 hunks)testutils_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
internal/build/cspell.json
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
**/*.go
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
**/*.{md,go}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
**/*_test.go
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
AGENT.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Only use the Go standard library and minimal golang.org/x packages; zero external dependencies.
internal/build/cspell.json (10)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to internal/build/cspell.json : Add technical terms to internal/build/cspell.json for spelling and grammar tools.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*.{md,go} : Markdown and Go source files must be checked with CSpell for spelling (British English, custom word list).
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*.md : When editing Markdown files, ensure compliance with LanguageTool (articles, punctuation, hyphenation), CSpell (spelling), and markdownlint (formatting).
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*.go : Enforce strict linting rules via revive: max function length 40 lines, max function results 3, max arguments 5, cognitive complexity 7, cyclomatic complexity 10 (configured in internal/build/revive.toml).
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Only use the Go standard library and minimal golang.org/x packages; zero external dependencies.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: ALWAYS run `make tidy` before committing: Go code formatting, whitespace clean-up, Markdown files checked with CSpell, LanguageTool, and markdownlint.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*.go : Everything must be in the `core` package; do not create subpackages.
errors_test.go (4)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: amery
PR: darvaza-proxy/core#82
File: errors.go:22-23
Timestamp: 2024-10-11T13:59:15.993Z
Learning: `ErrNilReceiver`, defined in `errors.go`, is intended for use by other packages even if it's not used within the current codebase.
lists_test.go (5)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: amery
PR: darvaza-proxy/core#84
File: list.go:160-171
Timestamp: 2024-11-15T04:50:14.792Z
Learning: In the Go `core` package's `list.go` file, the method `unsafeForEachElement` in `List[T]` does not need to check if `l` is `nil` because all public methods that call it already perform this check.
Learnt from: amery
PR: darvaza-proxy/core#86
File: list.go:83-89
Timestamp: 2024-11-16T07:43:16.106Z
Learning: In `list.go`, the `List[T]` methods `Len` and `ForEach` handle a nil receiver gracefully.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
slices.go (1)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
addrport_test.go (3)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
panicerror_test.go (4)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: amery
PR: darvaza-proxy/core#86
File: list.go:32-36
Timestamp: 2024-11-16T05:20:11.432Z
Learning: In Go, avoid suggesting dereferencing a `nil` pointer receiver within a method to initialize it, as this will cause a runtime panic.
compounderror_test.go (5)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: amery
PR: darvaza-proxy/core#82
File: errors.go:22-23
Timestamp: 2024-10-11T13:59:15.993Z
Learning: `ErrNilReceiver`, defined in `errors.go`, is intended for use by other packages even if it's not used within the current codebase.
Learnt from: amery
PR: darvaza-proxy/core#86
File: list.go:32-36
Timestamp: 2024-11-16T05:20:11.432Z
Learning: In Go, avoid suggesting dereferencing a `nil` pointer receiver within a method to initialize it, as this will cause a runtime panic.
addrs_test.go (3)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
slices_test.go (3)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
AGENT.md (9)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to AGENT.md : Update AGENT.md to reflect any changes in development workflow or standards.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to README.md : Update README.md to reflect significant changes in functionality or API.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Only use the Go standard library and minimal golang.org/x packages; zero external dependencies.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*.go : Enforce strict linting rules via revive: max function length 40 lines, max function results 3, max arguments 5, cognitive complexity 7, cyclomatic complexity 10 (configured in internal/build/revive.toml).
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: ALWAYS run `make tidy` before committing: Go code formatting, whitespace clean-up, Markdown files checked with CSpell, LanguageTool, and markdownlint.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Always run `make tidy` before committing to ensure proper formatting.
as_test.go (5)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: amery
PR: darvaza-proxy/core#86
File: list.go:83-89
Timestamp: 2024-11-16T07:43:16.106Z
Learning: In `list.go`, the `List[T]` methods `Len` and `ForEach` handle a nil receiver gracefully.
Learnt from: amery
PR: darvaza-proxy/core#82
File: errors.go:22-23
Timestamp: 2024-10-11T13:59:15.993Z
Learning: `ErrNilReceiver`, defined in `errors.go`, is intended for use by other packages even if it's not used within the current codebase.
panic_test.go (3)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
generics_test.go (3)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
testutils_test.go (4)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Only use the Go standard library and minimal golang.org/x packages; zero external dependencies.
maps_test.go (5)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: amery
PR: darvaza-proxy/core#86
File: list.go:83-89
Timestamp: 2024-11-16T07:43:16.106Z
Learning: In `list.go`, the `List[T]` methods `Len` and `ForEach` handle a nil receiver gracefully.
Learnt from: amery
PR: darvaza-proxy/core#84
File: list.go:160-171
Timestamp: 2024-11-15T04:50:14.792Z
Learning: In the Go `core` package's `list.go` file, the method `unsafeForEachElement` in `List[T]` does not need to check if `l` is `nil` because all public methods that call it already perform this check.
spinlock_test.go (9)
Learnt from: amery
PR: darvaza-proxy/core#105
File: sync.go:11-12
Timestamp: 2025-04-17T20:14:08.093Z
Learning: In the darvaza.org/core module, the SpinLock type is being deprecated in favor of darvaza.org/x/sync/spinlock. The implementation must remain in place (not an alias) to avoid circular dependencies, as the new package likely imports from core.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: amery
PR: darvaza-proxy/core#86
File: list.go:32-36
Timestamp: 2024-11-16T05:20:11.432Z
Learning: In Go, avoid suggesting dereferencing a `nil` pointer receiver within a method to initialize it, as this will cause a runtime panic.
Learnt from: amery
PR: darvaza-proxy/core#86
File: list.go:83-89
Timestamp: 2024-11-16T07:43:16.106Z
Learning: In `list.go`, the `List[T]` methods `Len` and `ForEach` handle a nil receiver gracefully.
Learnt from: amery
PR: darvaza-proxy/core#82
File: errors.go:22-23
Timestamp: 2024-10-11T13:59:15.993Z
Learning: `ErrNilReceiver`, defined in `errors.go`, is intended for use by other packages even if it's not used within the current codebase.
Learnt from: amery
PR: darvaza-proxy/core#84
File: list.go:160-171
Timestamp: 2024-11-15T04:50:14.792Z
Learning: In the Go `core` package's `list.go` file, the method `unsafeForEachElement` in `List[T]` does not need to check if `l` is `nil` because all public methods that call it already perform this check.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Only use the Go standard library and minimal golang.org/x packages; zero external dependencies.
🧬 Code Graph Analysis (10)
errors_test.go (2)
testutils_test.go (2)
S(20-25)AssertBool(186-195)errors.go (2)
IsErrorFn(234-257)IsErrorFn2(265-291)
lists_test.go (2)
lists.go (2)
ListForEach(29-40)ListForEachElement(43-55)testutils_test.go (1)
S(20-25)
addrport_test.go (2)
testutils_test.go (1)
S(20-25)addrport.go (1)
AddrPort(20-49)
panicerror_test.go (1)
testutils_test.go (1)
S(20-25)
compounderror_test.go (1)
testutils_test.go (1)
S(20-25)
addrs_test.go (1)
testutils_test.go (1)
S(20-25)
as_test.go (1)
testutils_test.go (1)
S(20-25)
panic_test.go (2)
testutils_test.go (2)
AssertEqual(55-64)AssertError(94-101)panic.go (1)
Recovered(8-11)
maps_test.go (2)
testutils_test.go (3)
AssertEqual(55-64)S(20-25)AssertSliceEqual(73-82)maps.go (11)
SortedKeys(17-30)SortedValuesUnlikelyCond(55-61)MapListInsert(136-138)MapListAppend(175-177)MapListContainsFn(98-107)MapListInsertUnique(154-158)MapListAppendUnique(181-185)MapListForEach(110-120)MapListCopyFn(207-214)MapAllListContainsFn(227-246)MapListAppendUniqueFn(189-197)
spinlock_test.go (2)
testutils_test.go (6)
AssertEqual(55-64)RunConcurrentTest(207-213)AssertError(94-101)AssertPanic(132-139)AssertNoPanic(163-175)RunBenchmark(249-256)sync.go (1)
SpinLock(13-13)
🔇 Additional comments (50)
internal/build/cspell.json (1)
45-45: LGTM! Appropriate dictionary additions for test utilities.The new words "govet", "linters", and "testutils" are relevant technical terms that align with the PR's focus on test infrastructure improvements and linting enhancements.
Also applies to: 52-52, 79-79
stack.go (1)
28-38: LGTM! Comment simplification improves clarity.Removing explicit byte size annotations from struct field comments makes them more maintainable whilst retaining the alphabetical ordering information that's actually useful for developers.
slices.go (1)
282-286: LGTM! Consistent comment cleanup.The removal of explicit byte size annotations from the
sortable[T]struct comments aligns with the broader PR pattern of simplifying comments whilst maintaining useful information about field ordering.testutils_test.go (10)
20-25: LGTM! Excellent generic slice helper implementation.The
S[T]function provides a concise way to create test slices with proper type inference. The zero-length slice handling is correct and the documentation clearly explains usage patterns.
27-46: LGTM! Clean TestCase interface design.The
TestCaseinterface andRunTestCasesfunction provide good abstraction for structured test execution. Proper use oft.Helper()ensures clean test output.
55-64: LGTM! Robust generic assertion helper.
AssertEqualprovides excellent error reporting with optional custom messages. The generic constraint and error formatting are well-implemented.
73-82: LGTM! Deep equality checking for slices.
AssertSliceEqualcorrectly usesreflect.DeepEqualfor comprehensive slice comparison. The error messaging is clear and helpful.
94-123: LGTM! Well-structured error assertion with helper functions.The
AssertErrorfunction with its internal helpersassertExpectedErrorandassertNoErrorprovides clean separation of concerns and consistent error reporting patterns.
132-154: LGTM! Comprehensive panic testing implementation.
AssertPanicwithvalidatePanicRecoveryprovides robust panic testing with optional panic value validation. The defer/recover pattern is correctly implemented.
163-175: LGTM! Clean no-panic assertion.
AssertNoPanicprovides the complementary functionality toAssertPanicwith proper error reporting for unexpected panics.
186-195: LGTM! Specialised boolean assertion helper.
AssertBoolprovides targeted boolean assertion functionality with clear error messages and optional custom formatting.
207-237: LGTM! Excellent concurrent testing infrastructure.The
RunConcurrentTestfunction with its helper functionsrunWorkersandcollectErrorsprovides a clean abstraction for concurrent testing. Proper use ofsync.WaitGroupand error collection via channels.
249-256: LGTM! Proper benchmark helper implementation.
RunBenchmarkcorrectly handles benchmark setup and timer management with the standard pattern of setup, reset timer, and execution loop.generics_test.go (5)
10-10: LGTM! Field reordering for better alignment.Moving the
expectedfield to the first position incoalesceTestCase[T]likely improves memory layout and aligns with the PR's field alignment optimisation goals.
149-149: LGTM! Consistent field alignment optimisation.The
expectedfield reordering incoalescePointerTestCase[T]follows the same pattern as the generic test case, maintaining consistency across the codebase.
422-422: LGTM! Field reordering for memory efficiency.Moving the
condboolean field to the last position iniifStringTestCaselikely improves struct packing and memory efficiency.
483-485: LGTM! Consistent field alignment across IIf test cases.The field reordering in
iifPointerTestCasemaintains consistency with other IIf test case structs, placing the booleancondfield at the end for better memory layout.
562-562: LGTM! Final field alignment consistency.The
condfield placement iniifStructTestCasecompletes the consistent field ordering pattern across all IIf test case structures.panicerror_test.go (1)
82-82: LGTM! Consistent adoption of ST helper function.The changes correctly replace explicit slice literals with the generic ST helper function, improving code consistency and maintainability. The removal of specific byte counts from struct field comments (line 82) also makes the code more maintainable.
Also applies to: 150-150, 156-156, 203-203, 404-404
errors_test.go (1)
24-29: LGTM! Proper adoption of test helper functions.The changes correctly implement the new test utilities:
- ST helper function for concise slice creation
- AssertBool helper for boolean assertions with clear error messages
The explicit type parameter usage (e.g.,
S[error]()) is appropriate for empty slices and mixed content scenarios.Also applies to: 33-33, 59-64, 68-69
addrport_test.go (1)
228-228: LGTM! Consistent usage of ST for byte slices.The changes correctly replace explicit byte slice literals with the Sbyte helper function, maintaining consistency with the broader refactoring effort whilst preserving the test logic for invalid IP address scenarios.
Also applies to: 234-234
addrs_test.go (1)
12-20: LGTM! Comprehensive adoption of ST helper across multiple types.The changes demonstrate excellent consistency in adopting the ST helper function across various slice types (string, netip.Addr, net.Addr). The explicit type specifications (e.g.,
S[string](),S[netip.Addr]) and the use ofnetip.Addr{}for invalid addresses improve code clarity. The comment updates removing specific byte counts also enhance maintainability.Also applies to: 183-190, 327-327, 433-441, 516-524, 532-537, 585-590
as_test.go (2)
11-11: Good simplification of field comments.The removal of explicit size annotations from the struct field comments improves readability whilst maintaining clarity about the field purposes.
Also applies to: 15-15
295-295: Excellent adoption of the ST helper function.The consistent replacement of explicit slice literals with calls to the generic S() helper function improves test code readability and maintainability. The usage correctly handles both typed empty slices (
S[string]()) and value-filled slices (S("a", "b", "c")).Also applies to: 300-300, 320-320, 343-343, 446-446, 456-456, 462-462, 468-468, 474-474, 484-484
compounderror_test.go (1)
18-18: Excellent systematic adoption of the ST helper.The consistent replacement of explicit
[]errorslice literals with calls to the generic S() helper function throughout the test cases improves code consistency and readability. The usage correctly handles both populated error slices (S(errors.New("first error"))) and empty typed slices (S[error]()).Also applies to: 23-23, 28-28, 33-33, 38-38, 60-63, 80-83, 108-108, 118-118, 123-123, 153-153, 163-163, 168-168, 208-209, 214-215, 220-221, 226-227, 262-262, 263-263, 304-304, 306-306, 336-336, 345-345, 354-354, 363-363, 372-372, 468-468
slices_test.go (2)
20-20: Excellent adoption of standardised assertion helpers.The replacement of manual conditional checks and error calls with helper assertion functions like
AssertSliceEqual,AssertBool, andAssertEqualsignificantly improves test consistency and readability. The assertion helpers provide clearer error messages and reduce boilerplate code whilst maintaining the same test logic.Also applies to: 62-62, 66-66, 71-71, 72-72, 77-77, 78-78, 109-109, 134-134, 153-153, 154-154, 157-157
29-29: Proper usage of ST helper for test data.The consistent replacement of explicit slice literals with calls to the generic S() helper function in the test data definitions improves maintainability and follows the established testing patterns.
Also applies to: 30-30, 32-35, 36-36, 38-38, 39-39
spinlock_test.go (4)
41-41: Excellent adoption of assertion helpers.The replacement of manual conditional checks and error calls with standardised assertion functions like
AssertEqual,AssertError,AssertPanic, andAssertNoPanicsignificantly improves test consistency and provides clearer, more informative error messages.Also applies to: 96-96, 104-104, 151-151, 153-153, 167-167, 173-173, 179-179, 205-205, 208-208
79-94: Proper usage of RunConcurrentTest helper.The replacement of manual goroutine and wait group management with the
RunConcurrentTesthelper function simplifies concurrent testing whilst maintaining the same test logic. The error handling approach with return values from worker functions is cleaner than manual synchronisation.Also applies to: 195-203
212-225: Good adoption of RunBenchmark helper.The use of the
RunBenchmarkhelper with setup and execution closures provides a cleaner structure for benchmark tests, separating initialisation from the actual benchmark execution.Also applies to: 229-238
240-257: Well-structured benchmark helper function.The
runContentionBenchmarkhelper function properly encapsulates the contended benchmark logic, making the main benchmark function cleaner and more focused.AGENT.md (2)
168-228: Excellent comprehensive documentation of test helper functions.The new "Test Helper Functions" section provides thorough documentation of the helper functions introduced in
testutils_test.go. The documentation includes:
- Clear descriptions of each helper function's purpose
- Practical usage examples showing before/after comparisons
- Explanation of the benefits (consistent error messages, reduced boilerplate, better maintainability)
- Proper categorisation of helpers (slice creation, assertions, advanced helpers)
The examples accurately demonstrate how the helpers simplify test code and improve consistency across the codebase.
350-361: Good punctuation and formatting improvements.The addition of periods at the end of bullet points and clarification of sentence endings improves consistency and readability throughout the documentation without changing the semantic content.
Also applies to: 407-410, 414-419
maps_test.go (8)
12-13: Field alignment optimisation looks correct.The reordering of fields in the struct follows Go best practices by placing larger fields (
string) before smaller ones (int) to improve memory alignment and reduce padding.
44-44: Good use of the generic AssertEqual helper.The refactoring from manual assertion to
AssertEqualimproves consistency and provides better error messages whilst maintaining the same test logic.
78-78: Excellent adoption of the ST helper for slice creation.The conversion from slice literals to
S[T]()calls reduces boilerplate and aligns with the project's standardisation efforts. The empty slice creation usingS[string]()is particularly clean.Also applies to: 83-83, 88-88, 93-93, 98-98
106-106: AssertSliceEqual usage is correct and improves readability.The replacement of manual slice comparisons with
AssertSliceEqualprovides better error messages and standardises the assertion pattern throughout the test suite.Also applies to: 121-121, 158-158, 218-218
118-118: Consistent use of S() helper for test data.The adoption of
S()for creating test slices is consistent with the project's guidelines and reduces boilerplate code effectively.Also applies to: 231-231, 247-247
239-239: Good use of AssertEqual for boolean and length comparisons.The helper function usage maintains the original test semantics whilst providing clearer error messages when assertions fail.
Also applies to: 243-243
300-301: Comprehensive and correct adoption of assertion helpers.The systematic replacement of manual assertions with
AssertEqualthroughout the test functions maintains the original test logic whilst improving consistency and error reporting. The helper functions are used correctly with appropriate error messages.Also applies to: 349-349, 364-365, 372-372, 381-382, 386-387, 389-390, 403-404, 439-440, 443-444, 447-448, 456-456, 460-460, 464-464, 472-472, 476-476, 480-480, 507-508, 516-516, 595-595, 598-598, 622-622, 631-631, 636-637, 656-658, 665-665, 678-678, 710-710, 716-716, 720-720, 736-736, 746-746, 761-761, 770-770, 782-782, 790-790
429-429: Proper field alignment optimisation in struct.The reordering of fields in the
customTypestruct follows Go best practices by placing theintfield after thestringfield. This optimisation reduces memory padding and improves struct layout efficiency.Also applies to: 433-434
panic_test.go (9)
9-9: Field alignment optimisation improves memory layout.The addition of the comment clarifies the intention behind field ordering, placing interface types and strings before smaller boolean fields to reduce memory padding.
76-76: Correct use of AssertEqual for string comparisons.The replacement of manual
t.Fatalfcalls withAssertEqualmaintains the original test logic whilst providing better error messages and consistency across the test suite.Also applies to: 82-82, 87-87
165-165: Good use of AssertError for error expectation validation.The
AssertErrorhelper correctly validates whether an error is expected or not, replacing manual error checking logic whilst maintaining the same test semantics.
167-175: Improved error handling logic with clearer structure.The refactored conditional logic for panic recovery validation is more readable and maintains the original test intent. The type assertion and nil check for recovered values are properly preserved.
230-230: Consistent error expectation handling.The use of
AssertErrorin thecatcherTryTestCasetest maintains the original logic whilst standardising the error checking pattern.
238-240: Cleaner conditional logic for panic recovery checks.The refactored logic maintains the original test semantics whilst improving readability. The error message format is preserved for consistency.
269-269: Appropriate use of AssertEqual for error message validation.The helper function usage correctly validates the panic error message whilst maintaining the original test logic.
351-351: Consistent assertion pattern maintained.The use of
AssertErrorin thecatchTestCasetest follows the established pattern and maintains the original error checking logic.
381-381: Proper error and value assertions in panic recovery tests.The combination of
AssertErrorfor error expectation andAssertEqualfor value comparisons maintains the original test logic whilst providing better error messages and consistency.Also applies to: 389-389, 394-394
- Reorder coalesceTestCase fields: expected, name, inputs (better alignment) - Reorder coalescePointerTestCase fields: expected, name, inputs (better alignment) - Reorder iifStringTestCase fields: name, yes, no, expected, cond (better alignment) - Reorder iifPointerTestCase fields: yes, no, expected, name, cond (better alignment) - Reorder iifStructTestCase fields: name, yes, no, expected, cond (better alignment) Field alignment optimization improves memory efficiency for test structures. All struct initializations remain compatible with new field ordering. Signed-off-by: Alejandro Mery <amery@apptly.co>
- Reorder keysTestCase fields: input, name, expected (better alignment) - Reorder customType fields: name, id (better alignment) - Update struct literals to use named field initialization - Maintain compatibility with reordered field layout Field alignment optimization improves memory efficiency for test structures. All struct initializations updated to work with new field ordering. Signed-off-by: Alejandro Mery <amery@apptly.co>
- Add "govet" to internal/build/cspell.json - Add "linters" to internal/build/cspell.json - Fix spelling check failures in golangci-lint documentation - Support technical terms used in AGENT.md configuration sections Technical linter names are now recognized in spell checking. Enables clean documentation of golangci-lint configuration. Signed-off-by: Alejandro Mery <amery@apptly.co>
- Add missing periods to bullet points in field alignment section - Fix numbered list punctuation for golangci-lint schema issues - Ensure consistent punctuation in resolution status section - Satisfy markdownlint requirements for list formatting All documentation lists now follow consistent punctuation standards. Markdownlint validation passes without punctuation warnings. Signed-off-by: Alejandro Mery <amery@apptly.co>
…ments - Remove hard-coded byte counts from struct field comments - Keep field ordering information without platform-specific sizes - Maintain field alignment rationale in comments - Apply changes to Frame, sortable, and test case structs Field alignment comments now focus on ordering without stale byte counts. Comments remain informative while avoiding platform-specific assumptions. Signed-off-by: Alejandro Mery <amery@apptly.co>
amery
force-pushed
the
enhance-test-utilities
branch
from
aee1898 to
2129f1d
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
testutils_test.go (1)
257-257: Add missing newline at end of file.The file must end with a newline according to the coding guidelines.
} +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
AGENT.md(3 hunks)addrport_test.go(1 hunks)addrs_test.go(6 hunks)as_test.go(8 hunks)compounderror_test.go(16 hunks)errors_test.go(2 hunks)generics_test.go(5 hunks)internal/build/cspell.json(2 hunks)lists_test.go(1 hunks)maps_test.go(29 hunks)panic_test.go(7 hunks)panicerror_test.go(4 hunks)slices.go(1 hunks)slices_test.go(5 hunks)spinlock_test.go(5 hunks)stack.go(1 hunks)testutils_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- internal/build/cspell.json
- stack.go
- slices.go
- panicerror_test.go
- lists_test.go
- as_test.go
- addrs_test.go
- addrport_test.go
- AGENT.md
- maps_test.go
- errors_test.go
- panic_test.go
- slices_test.go
- generics_test.go
- spinlock_test.go
- compounderror_test.go
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
**/*_test.go
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
**/*
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
**/*.{md,go}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Only use the Go standard library and minimal golang.org/x packages; zero external dependencies.
testutils_test.go (4)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Only use the Go standard library and minimal golang.org/x packages; zero external dependencies.
🔇 Additional comments (5)
testutils_test.go (5)
1-8: Package declaration and imports look correct.The package is properly declared as
coreand all imports are from the standard library with no external dependencies, following the project guidelines.
10-25: Excellent slice creation helper implementation.The
S[T]function is well-designed with proper generic constraints and clear documentation. It will significantly reduce boilerplate in table-driven tests as intended.
27-46: Well-designed TestCase interface and runner.The
TestCaseinterface andRunTestCasesfunction provide a clean abstraction for structured test execution, properly usingt.Helper()andt.Run().
197-237: Excellent concurrent testing implementation.The concurrent testing utilities are well-designed with proper goroutine management, synchronization using
sync.WaitGroup, and clean error collection through channels. The separation into helper functions improves maintainability.
239-256: Well-designed benchmark helper.The
RunBenchmarkfunction provides a clean abstraction for benchmark testing with proper timer management and setup/execution separation.
- Create testutils_test.go with generic S[T]() helper function
- Provide concise syntax for test slice creation: S(1, 2, 3) vs []int{1, 2, 3}
- Support empty slice creation with type specification: S[int]()
- Add comprehensive documentation with usage examples
Test slice creation is now more concise and readable.
Foundation for converting all test slice literals to use helper function.
Signed-off-by: Alejandro Mery <amery@apptly.co>
- Replace 119 slice literals across 9 test files with S[T]() helper calls
- Convert []int{1, 2, 3} to S(1, 2, 3) for improved readability
- Convert []string{} to S[string]() for type-safe empty slices
- Standardize slice creation syntax across all test files
Files updated:
- addrport_test.go: 2 literals → S() calls
- addrs_test.go: 8 literals → S() calls
- as_test.go: 10 literals → S() calls
- compounderror_test.go: 21 literals → S() calls
- errors_test.go: 12 literals → S() calls
- lists_test.go: 6 literals → S() calls
- maps_test.go: 8 literals → S() calls
- panicerror_test.go: 4 literals → S() calls
- slices_test.go: 9 literals → S() calls
All tests maintain functionality while improving code consistency.
Signed-off-by: Alejandro Mery <amery@apptly.co>
amery
force-pushed
the
enhance-test-utilities
branch
from
2129f1d to
1d78987
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
testutils_test.go (7)
57-66: Fix potential panic in type assertion.The type assertion
msgAndArgs[0].(string)on line 62 could panic if the first argument is not a string. This is a duplicate of a previously identified issue.- if len(msgAndArgs) > 0 { - msg = fmt.Sprintf(msgAndArgs[0].(string), msgAndArgs[1:]...) + ": " + msg - } + if len(msgAndArgs) > 0 { + if format, ok := msgAndArgs[0].(string); ok { + msg = fmt.Sprintf(format, msgAndArgs[1:]...) + ": " + msg + } + }
75-84: Fix potential panic in type assertion.The type assertion
msgAndArgs[0].(string)on line 80 could panic if the first argument is not a string. This is a duplicate of a previously identified issue.- if len(msgAndArgs) > 0 { - msg = fmt.Sprintf(msgAndArgs[0].(string), msgAndArgs[1:]...) + ": " + msg - } + if len(msgAndArgs) > 0 { + if format, ok := msgAndArgs[0].(string); ok { + msg = fmt.Sprintf(format, msgAndArgs[1:]...) + ": " + msg + } + }
105-114: Fix potential panic in type assertion.The type assertion
msgAndArgs[0].(string)on line 110 could panic if the first argument is not a string. This is a duplicate of a previously identified issue.- if len(msgAndArgs) > 0 { - msg = fmt.Sprintf(msgAndArgs[0].(string), msgAndArgs[1:]...) - } + if len(msgAndArgs) > 0 { + if format, ok := msgAndArgs[0].(string); ok { + msg = fmt.Sprintf(format, msgAndArgs[1:]...) + } + }
116-125: Fix potential panic in type assertion.The type assertion
msgAndArgs[0].(string)on line 121 could panic if the first argument is not a string. This is a duplicate of a previously identified issue.- if len(msgAndArgs) > 0 { - msg = fmt.Sprintf(msgAndArgs[0].(string), msgAndArgs[1:]...) + ": %v" - } + if len(msgAndArgs) > 0 { + if format, ok := msgAndArgs[0].(string); ok { + msg = fmt.Sprintf(format, msgAndArgs[1:]...) + ": %v" + } + }
134-156: Fix potential panic in type assertion.The type assertion
msgAndArgs[0].(string)on line 148 invalidatePanicRecoverycould panic if the first argument is not a string. This is a duplicate of a previously identified issue.- if len(msgAndArgs) > 0 { - msg = fmt.Sprintf(msgAndArgs[0].(string), msgAndArgs[1:]...) - } + if len(msgAndArgs) > 0 { + if format, ok := msgAndArgs[0].(string); ok { + msg = fmt.Sprintf(format, msgAndArgs[1:]...) + } + }
165-177: Fix potential panic in type assertion.The type assertion
msgAndArgs[0].(string)on line 171 could panic if the first argument is not a string. This is a duplicate of a previously identified issue.- if len(msgAndArgs) > 0 { - msg = fmt.Sprintf(msgAndArgs[0].(string), msgAndArgs[1:]...) + ": " + msg - } + if len(msgAndArgs) > 0 { + if format, ok := msgAndArgs[0].(string); ok { + msg = fmt.Sprintf(format, msgAndArgs[1:]...) + ": " + msg + } + }
187-197: Fix potential panic in type assertion.The type assertion
msgAndArgs[0].(string)on line 193 could panic if the first argument is not a string. This is a duplicate of a previously identified issue.- if len(msgAndArgs) > 0 { - msg = fmt.Sprintf(msgAndArgs[0].(string), msgAndArgs[1:]...) + ": " + msg - } + if len(msgAndArgs) > 0 { + if format, ok := msgAndArgs[0].(string); ok { + msg = fmt.Sprintf(format, msgAndArgs[1:]...) + ": " + msg + } + }
🧹 Nitpick comments (1)
testutils_test.go (1)
259-259: Missing final newline.The file should end with a newline character according to the coding guidelines.
+
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
AGENT.md(3 hunks)addrport_test.go(1 hunks)addrs_test.go(6 hunks)as_test.go(8 hunks)compounderror_test.go(16 hunks)errors_test.go(2 hunks)internal/build/cspell.json(2 hunks)lists_test.go(1 hunks)maps_test.go(29 hunks)panic_test.go(7 hunks)panicerror_test.go(4 hunks)slices_test.go(5 hunks)spinlock_test.go(5 hunks)testutils_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- internal/build/cspell.json
- errors_test.go
- lists_test.go
- panicerror_test.go
- maps_test.go
- addrport_test.go
- addrs_test.go
- compounderror_test.go
- panic_test.go
- spinlock_test.go
- AGENT.md
- slices_test.go
- as_test.go
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
**/*_test.go
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
**/*
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
**/*.{md,go}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENT.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Only use the Go standard library and minimal golang.org/x packages; zero external dependencies.
testutils_test.go (7)
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Helper functions like `S[T]()` should be used to create test slices in Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Comprehensive coverage for generic functions is expected in tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Applies to **/*_test.go : Table-driven tests are preferred for Go tests.
Learnt from: CR
PR: darvaza-proxy/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-10T13:13:22.040Z
Learning: Only use the Go standard library and minimal golang.org/x packages; zero external dependencies.
Learnt from: amery
PR: darvaza-proxy/core#86
File: list.go:32-36
Timestamp: 2024-11-16T05:20:11.432Z
Learning: In Go, avoid suggesting dereferencing a `nil` pointer receiver within a method to initialize it, as this will cause a runtime panic.
Learnt from: amery
PR: darvaza-proxy/core#79
File: internal/build/gen_mk.sh:161-169
Timestamp: 2024-10-01T19:43:25.764Z
Learning: When suggesting refactoring of build scripts, consider the possibility of nested `go.mod` files within subdirectories like `./cmd/`, and ensure that the proposed changes handle nested modules correctly.
Learnt from: amery
PR: darvaza-proxy/core#79
File: internal/build/gen_mk.sh:161-169
Timestamp: 2024-10-08T15:33:35.792Z
Learning: When suggesting refactoring of build scripts, consider the possibility of nested `go.mod` files within subdirectories like `./cmd/`, and ensure that the proposed changes handle nested modules correctly.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: multi-go (1.24)
- GitHub Check: multi-go (1.23)
- GitHub Check: test
- GitHub Check: race
- GitHub Check: make (1.23)
- GitHub Check: make (1.24)
🔇 Additional comments (7)
testutils_test.go (7)
1-8: Package declaration and imports look good.The file correctly uses the
corepackage and imports only standard library packages, adhering to the coding guidelines.
10-27: Excellent generic slice helper implementation.The
S[T]function provides a clean, type-safe way to create slices and aligns with the retrieved learning that helper functions likeS[T]()should be used in Go tests. The comprehensive documentation with examples is particularly valuable.
29-48: Well-designed TestCase interface and runner.The
TestCaseinterface andRunTestCasesfunction provide excellent abstraction for table-driven tests, which aligns with the preferred testing pattern mentioned in the coding guidelines.
95-96: Appropriate use of revive disable comment.The
revive:disable-next-line:flag-parametercomment is correctly used for theexpectErrorbool parameter, which is a legitimate design choice for this assertion helper.
209-215: Well-structured concurrent testing helper.The
RunConcurrentTestfunction provides excellent abstraction for concurrent testing patterns with proper error collection and synchronisation.
217-239: Excellent separation of concerns in worker management.The
runWorkersandcollectErrorsfunctions provide clean separation of concerns for goroutine management and error collection. The use ofWaitGroupand proper channel closure is correctly implemented.
251-258: Proper benchmark helper implementation.The
RunBenchmarkfunction correctly handles benchmark timing withb.ResetTimer()and provides a clean interface for setup and execution phases.
- Add AssertEqual, AssertSliceEqual, AssertError assertion helpers - Add AssertPanic, AssertNoPanic, AssertBool for specialized testing - Create RunConcurrentTest for standardized goroutine testing patterns - Add RunBenchmark for setup/execution phase benchmark helpers - Expand testutils_test.go with 238 lines of helper functions - Update AGENT.md with comprehensive test helper documentation - Add "testutils" to cspell dictionary for spell checking Test file improvements: - maps_test.go: 204 lines → 133 lines (reduced boilerplate) - panic_test.go: 74 lines → 45 lines (simplified assertions) - slices_test.go: 47 lines → 32 lines (cleaner tests) - spinlock_test.go: 151 lines → 118 lines (standardized concurrency) - errors_test.go: 9 lines → 6 lines (assertion helpers) Net change: +456 lines utility functions, -329 lines test boilerplate. Test maintainability significantly improved with standardized patterns. Signed-off-by: Alejandro Mery <amery@apptly.co>
- Add sliceReverseTestCase struct with name, input, and expected fields - Implement test(t) method following established table-driven patterns - Define test cases with descriptive names outside function - Use t.Run() for proper subtest execution and hierarchy - Utilize AssertSliceEqual helper for cleaner assertions - Maintain all existing test coverage while improving organization Test structure now aligns with codebase patterns and improves readability. Proper subtest naming enables better test failure identification. Signed-off-by: Alejandro Mery <amery@apptly.co>
amery
force-pushed
the
enhance-test-utilities
branch
from
1d78987 to
9a9f0cc
Compare
|
|
Summary
This PR enhances the test infrastructure with comprehensive helper functions and optimizations, significantly improving test maintainability and code quality across the codebase.
Key Changes
🧪 Test Helper Functions
⚡ Performance Optimizations
generics_test.goandmaps_test.gofor memory efficiency📚 Documentation & Build
govet,linters)🔧 Code Standardization
S[T]()slice creationCommit Structure
This PR is organized into 9 logical commits with conventional commit messages:
fix(tests): Fix field alignment in generics_test.go structs- Memory optimization for test structuresfix(tests): Fix field alignment in maps_test.go structs- Memory optimization for test structuresbuild(cspell): Add linter terminology to cspell dictionary- Support for technical linter termsdocs(agent): Fix punctuation in documentation lists- Markdownlint compliance improvementsrefactor(types): Remove specific byte counts from field alignment comments- Remove stale byte countsfeat(tests): Add S[T]() helper function for concise slice creation- Core slice creation helperrefactor(tests): Convert all test slice literals to use S[T]() helper- Standardize slice creationfeat(tests): Enhance test utilities with comprehensive helper functions- Main test helper additionsrefactor(tests): Convert TestSliceReverse to use structured test cases- Improve test structureTest Plan
✅ All CI checks passing:
✅ Local verification:
make all coverage- All tests passmake tidy- All linting passesImpact
Backward Compatibility
✅ Fully backward compatible - All existing tests continue to work unchanged while gaining access to new helper functions.
Summary by CodeRabbit
New Features
Documentation
Refactor
Chores