Skip to content

rename and refactor testfiles#356

Merged
gaissmai merged 1 commit intomainfrom
devel
Oct 31, 2025
Merged

rename and refactor testfiles#356
gaissmai merged 1 commit intomainfrom
devel

Conversation

@gaissmai
Copy link
Owner

@gaissmai gaissmai commented Oct 31, 2025

Summary by CodeRabbit

  • Documentation

    • Updated documentation links for test examples to reflect simplified naming conventions.
  • New Features

    • Added improved example demonstrating custom value type usage with equality and cloning support.

@coderabbitai
Copy link

coderabbitai bot commented Oct 31, 2025

Walkthrough

This PR reorganizes test infrastructure and examples by consolidating test scaffolding from zz-prefixed files into standard files. The route type with Equal and Clone methods is moved from a dedicated example file to example_test.go, test helpers are extracted into a new zz-helpers_test.go file, and zz-init_test.go is simplified. Documentation links are updated to reflect renamed files.

Changes

Cohort / File(s) Change Summary
Documentation
README.md
Updated links to concurrent test examples by removing "zz-" prefix from filenames
Example consolidation
example_test.go
Added maps import; introduced route type with ASN and Attrs fields; added Equal() and Clone() methods using maps.Equal and maps.Clone; added ExampleTable_customValue() function demonstrating table insertion, cloning, and equality comparison
Example removal
zz-example_custom_value_test.go
Removed entire file; content consolidated into example_test.go
Test helpers extraction
zz-helpers_test.go
New file added with test utilities: workLoadN() for controlling test workload, panic assertion helpers (mustPanic, noPanic), MyInt type with Clone() method, recursive node counting and IPv4/IPv6 validation functions, and noPanicRangeOverFunc() for iterator coverage across multiple generic signatures
Test scaffold cleanup
zz-init_test.go
Removed test helpers (workLoadN, type MyInt, validators, panic helpers, iteration utilities) and unused imports (iter, testing); retained only tier1 data holder declaration

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • example_test.go: Verify Equal() and Clone() implementations correctly use maps.Equal and maps.Clone for the Attrs field
  • zz-helpers_test.go: Review the noPanicRangeOverFunc() implementation and its support for multiple iterator signatures to ensure comprehensive coverage
  • File consolidation: Confirm that moving content from zz-example_custom_value_test.go to example_test.go preserves test behavior and example functionality

Possibly related PRs

Poem

🐰 Test files shuffle and reorganize,
From zz- prefixed homes they rise,
Helpers grouped in tidy lines,
Examples shine in redesigned shines,
Cleaner tests, consolidated chimes! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "rename and refactor testfiles" accurately reflects the main changes in the pull request, which involve reorganizing test files by consolidating helper functions from zz-init_test.go into a new zz-helpers_test.go, moving example code from zz-example_custom_value_test.go into example_test.go, and updating documentation references. The title clearly conveys that structural changes to test files are occurring and is sufficiently specific to communicate the nature of the changes without being misleading. While the title could be more precise about the consolidation strategy, it effectively summarizes the primary modification from the developer's perspective.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devel

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a19139f and 225c290.

📒 Files selected for processing (5)
  • README.md (1 hunks)
  • example_test.go (2 hunks)
  • zz-example_custom_value_test.go (0 hunks)
  • zz-helpers_test.go (1 hunks)
  • zz-init_test.go (1 hunks)
💤 Files with no reviewable changes (1)
  • zz-example_custom_value_test.go
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-23T21:31:58.767Z
Learning: In gaissmai/bart tests, write all source comments in English only. Provide copy/paste-ready files with English comments throughout.
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-30T08:30:12.716Z
Learning: In the bart tests, prefer the existing helpers mpp (MustParsePrefix) and mpa (MustParseAddr); do not re-declare local equivalents in new *_test.go files.
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-13T07:39:52.762Z
Learning: For bart routing table test files, use routeEntry struct with nextHop netip.Addr, exitIF string, and attributes map[string]int fields, with *routeEntry as the Fat[V] payload type in tests, instead of generic configuration objects. Implement Clone() method for deep cloning of the attributes map.
📚 Learning: 2025-09-17T13:02:04.787Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-17T13:02:04.787Z
Learning: In the gaissmai/bart codebase, all types including public types Table, Fat, and Lite are designed to work with zero values. Use &Table[T]{}, &Fat[T]{}, &Lite[T]{} instead of constructor functions NewTable[T](), NewFat[T](), NewLite[T]() in tests and code.

Applied to files:

  • README.md
📚 Learning: 2025-09-12T15:36:30.988Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-12T15:36:30.988Z
Learning: When working with Table.All() in Go 1.23+, remember that it returns iter.Seq2[netip.Prefix, V] (an iterator), not a slice. You cannot use len() on it, and must use range iteration or helper functions like countEntries() to work with the data. Always use getAllKV() helper when you need to compare table contents with reflect.DeepEqual().

Applied to files:

  • README.md
📚 Learning: 2025-09-21T17:48:39.206Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-21T17:48:39.206Z
Learning: In the gaissmai/bart codebase, the Lite type embeds liteTable[struct{}] directly, making methods like Contains, Delete, Get, and Modify automatically available on Lite instances through Go's method promotion without needing any adapter methods.

Applied to files:

  • README.md
📚 Learning: 2025-09-09T21:08:03.154Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 284
File: equal_test.go:73-85
Timestamp: 2025-09-09T21:08:03.154Z
Learning: In Go 1.22 and later versions, loop variables are per-iteration (not per-loop), eliminating the classic loop variable capture problem in goroutines. The `tc := tc` shadowing pattern is no longer needed for parallel subtests or other goroutine scenarios when the project uses Go 1.22+.

Applied to files:

  • README.md
📚 Learning: 2025-09-21T17:48:39.206Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-21T17:48:39.206Z
Learning: In the gaissmai/bart codebase, the Lite type embeds liteTable[struct{}] directly using Go's embedding syntax. All methods from liteTable[struct{}] are automatically promoted to Lite without needing adapter methods due to Go's embedding mechanism.

Applied to files:

  • README.md
📚 Learning: 2025-09-21T17:48:39.206Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-21T17:48:39.206Z
Learning: In the gaissmai/bart codebase, the Lite type embeds liteTable[struct{}] directly, so all methods from liteTable[struct{}] are automatically promoted to Lite without needing adapter methods. Go's embedding provides automatic method promotion.

Applied to files:

  • README.md
📚 Learning: 2025-09-13T07:39:52.762Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-13T07:39:52.762Z
Learning: For bart routing table test files, use routeEntry struct with nextHop netip.Addr, exitIF string, and attributes map[string]int fields, with *routeEntry as the Fat[V] payload type in tests, instead of generic configuration objects. Implement Clone() method for deep cloning of the attributes map.

Applied to files:

  • example_test.go
  • zz-init_test.go
  • zz-helpers_test.go
📚 Learning: 2025-09-13T07:30:12.421Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-13T07:30:12.421Z
Learning: For bart routing table documentation and examples, use routeEntry struct with nextHop netip.Addr, exitIF string, and attributes map[string]int fields, with *routeEntry as the Fat[V] payload type, instead of generic configuration objects.

Applied to files:

  • example_test.go
📚 Learning: 2025-09-12T19:34:43.211Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-12T19:34:43.211Z
Learning: Use netip.MustParsePrefix() from the standard library instead of creating custom helper functions for parsing network prefixes in tests. This follows the KISS principle and reduces unnecessary code.

Applied to files:

  • example_test.go
  • zz-init_test.go
  • zz-helpers_test.go
📚 Learning: 2025-09-23T20:54:23.347Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-23T20:54:23.347Z
Learning: When composing netip prefixes from an Addr in tests/utilities, prefer netip.PrefixFrom(addr, bits) (single return) rather than Addr.Prefix(bits) which returns (Prefix, error) in newer Go versions. This avoids “multiple-value … in single-value context” compile errors.

Applied to files:

  • example_test.go
  • zz-init_test.go
📚 Learning: 2025-09-23T21:09:41.454Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-23T21:09:41.454Z
Learning: In tests for CIDR overlap ground-truth, prefer using netip.Prefix.Overlaps(a, b) instead of hand-rolled logic. The repository targets Go ≥ 1.24 where Prefix.Overlaps is available and accurate.

Applied to files:

  • example_test.go
  • zz-init_test.go
📚 Learning: 2025-09-29T13:51:30.534Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-29T13:51:30.534Z
Learning: In gaissmai/bart, prefer netip.Addr.Prefix(bits) over netip.PrefixFrom(addr, bits) because Addr.Prefix() automatically normalizes/masks the prefix with Masked(), ensuring canonicalized prefixes. This is crucial for routing table correctness, even if it allocates an error in the hot path.

Applied to files:

  • example_test.go
  • zz-init_test.go
📚 Learning: 2025-09-30T08:30:12.716Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-30T08:30:12.716Z
Learning: In the bart tests, prefer the existing helpers mpp (MustParsePrefix) and mpa (MustParseAddr); do not re-declare local equivalents in new *_test.go files.

Applied to files:

  • zz-init_test.go
  • zz-helpers_test.go
📚 Learning: 2025-09-19T07:28:23.239Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-19T07:28:23.239Z
Learning: In the bart repository nodeiface_test.go file, the constant declarations like `const c = uint8(100)` at line 984 are already properly scoped within their respective test functions and should not be moved.

Applied to files:

  • zz-init_test.go
  • zz-helpers_test.go
📚 Learning: 2025-09-30T08:38:57.278Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-30T08:38:57.278Z
Learning: Lite signatures (Sept 2025) in gaissmai/bart:
- Lite.Lookup(addr netip.Addr) bool
- Lite.LookupPrefix(pfx netip.Prefix) bool
- Lite.LookupPrefixLPM(pfx netip.Prefix) (netip.Prefix, bool)
Tests must not bind a dummy value and should use these bool-returning forms (and (prefix,bool) for LPM).

Applied to files:

  • zz-init_test.go
  • zz-helpers_test.go
📚 Learning: 2025-09-12T06:23:05.913Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-12T06:23:05.913Z
Learning: The leafNode[V] struct in this codebase has two fields: `prefix netip.Prefix` and `value V`. The prefix field is of type netip.Prefix, not []byte. When generating test code for leafNode, use netip.MustParsePrefix() or similar functions to create valid netip.Prefix values.

Applied to files:

  • zz-init_test.go
  • zz-helpers_test.go
📚 Learning: 2025-10-12T15:21:07.795Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 337
File: internal/nodes/gold_test.go:74-85
Timestamp: 2025-10-12T15:21:07.795Z
Learning: In the internal/nodes package, test files define a helper `mpp` (short for "must parse prefix") that wraps netip.MustParsePrefix. This helper is used throughout the internal/nodes test files to create netip.Prefix values. Do not flag uses of mpp in internal/nodes test files as undefined.

Applied to files:

  • zz-init_test.go
  • zz-helpers_test.go
📚 Learning: 2025-10-07T17:47:15.556Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-10-07T17:47:15.556Z
Learning: netip.Prefix internal structure in Go: Addr (24 bytes: uint128 with 2×uint64 = 16B + unique.Handle = 8B) + bitsPlusOne uint8 (1 byte) + padding (7 bytes for 8-byte alignment) = 32 bytes total. The uint128 representation enables efficient bit operations on 64-bit registers for IPv6 addresses.

Applied to files:

  • zz-init_test.go
📚 Learning: 2025-10-17T18:36:43.515Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-10-17T18:36:43.515Z
Learning: In the bart package (all.go), the blank import of `internal/bitset` at the top level is essential for proper inlining of BitSet256 functions in production code. Without it, the Go inliner fails due to the deep nesting path (bart -> internal/nodes -> internal/sparse -> internal/bitset). This has been verified via CPU profiling.

Applied to files:

  • zz-helpers_test.go
📚 Learning: 2025-09-19T07:28:23.239Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-19T07:28:23.239Z
Learning: In the bart repository test files, constant declarations are properly scoped within test functions and should not be moved unless they are actually declared at package level.

Applied to files:

  • zz-helpers_test.go
📚 Learning: 2025-09-17T14:59:15.443Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-09-17T14:59:15.443Z
Learning: The DumpList4() and DumpList6() methods in the gaissmai/bart codebase return a hierarchical tree structure ([]DumpListNode[V]) where each node contains CIDR, Value, and Subnets fields. The Subnets field is recursive, containing child nodes. Tests should count total nodes recursively, not expect a flat list.

Applied to files:

  • zz-helpers_test.go
📚 Learning: 2025-10-05T21:26:24.231Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-10-05T21:26:24.231Z
Learning: BartNode DumpString signature in internal/nodes: DumpString(octets []uint8, depth int, is4 bool, printVals bool) string. It traverses down the child path (expecting *BartNode at each step) before dumping the subtree with Dump. Tests must pass octets (first N octets) and a matching depth to target deeper nodes.

Applied to files:

  • zz-helpers_test.go
📚 Learning: 2025-10-05T21:46:51.386Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-10-05T21:46:51.386Z
Learning: In internal/nodes, FastNode and LiteNode expose DumpString(octets []uint8, depth int, is4 bool, printVals bool) string with identical traversal/error semantics as BartNode; tests must construct multi-level paths via InsertChild with node-typed children to avoid Fringe/Leaf along the traversal path, and assert the two error branches (kid not set, kid wrong type).

Applied to files:

  • zz-helpers_test.go
📚 Learning: 2025-10-05T21:27:32.948Z
Learnt from: gaissmai
Repo: gaissmai/bart PR: 0
File: :0-0
Timestamp: 2025-10-05T21:27:32.948Z
Learning: BartNode DumpString signature in internal/nodes: func (n *BartNode[V]) DumpString(octets []uint8, depth int, is4 bool, printVals bool) string. It traverses down the child chain using GetChild(path[i]) and requires BartNode children along the octet/depth path; otherwise it returns an "ERROR: kid ..." string. Tests must ensure traversal paths are BartNode nodes (e.g., by inserting BartNode children via InsertChild) instead of Fringe/Leaf children.

Applied to files:

  • zz-helpers_test.go
🧬 Code graph analysis (2)
example_test.go (1)
internal/value/value.go (1)
  • Equal (90-97)
zz-helpers_test.go (1)
common.go (1)
  • DumpListNode (56-60)
🔇 Additional comments (9)
README.md (1)

180-182: LGTM! Documentation correctly updated.

The links to concurrent test examples have been properly updated to reflect the renamed test files.

example_test.go (2)

190-204: LGTM! Well-implemented custom value type.

The route type properly implements both equality and cloning semantics:

  • Equal correctly uses maps.Equal for deep comparison of the Attrs map
  • Clone performs a deep copy using maps.Clone to avoid shared map references

This follows the established pattern for custom value types in bart routing tables.


206-215: LGTM! Clear example demonstrating custom value usage.

The example effectively demonstrates:

  • Using InsertPersist with a custom value type
  • Cloning a table containing custom values
  • Verifying equality after cloning
zz-init_test.go (1)

40-41: LGTM! Comment clarification.

The comment clearly describes the purpose of the tier1 variable.

zz-helpers_test.go (5)

18-35: LGTM! Well-designed test utilities.

The workload control and parsing helpers are well-implemented:

  • workLoadN() appropriately adjusts test sizes based on testing.Short()
  • mpp enforces that input prefixes are masked, which improves test correctness by catching non-canonicalized test data early

38-44: LGTM! Clean cloneable test type.

MyInt with its Clone method provides a simple, correct example for testing deep copy behavior.


46-72: LGTM! Correct recursive validators.

All three functions correctly:

  • Recurse through the DumpListNode hierarchy via the Subnets field
  • Use t.Helper() (for the validation functions) to improve test output context
  • Validate their respective invariants

74-92: LGTM! Proper panic assertion helpers.

Both helpers correctly use t.Helper() and defer/recover patterns to assert panic behavior in tests.


94-120: LGTM! Comprehensive iterator testing helper.

The function properly handles multiple iterator signatures and uses t.Helper() for better test diagnostics. The hardcoded prefix "1.2.3.4/32" (lines 110, 114) is acceptable for basic panic testing since the goal is to verify the iterator doesn't panic, not to test specific prefix behavior.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 18970510383

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 91.231%

Totals Coverage Status
Change from base Build 18919093611: 0.0%
Covered Lines: 6887
Relevant Lines: 7549

💛 - Coveralls

@gaissmai gaissmai merged commit 4bf1c08 into main Oct 31, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants