refactor(tests,lint): introduce shared cluster/eviction helpers and drop ireturn suppressions#102
Merged
refactor(tests,lint): introduce shared cluster/eviction helpers and drop ireturn suppressions#102
Conversation
Owner
hyp3rd
commented
May 3, 2026
- Add tests/dist_cluster_helper.go with DistCluster, SetupInProcessCluster, and SetupInProcessClusterRF to replace repeated in-process cluster boilerplate in integration, remove/read-repair, stale-quorum, and heartbeat tests.
- Add heartbeat test helpers (waitForAllAlive, waitForDeadTransition, waitForNodeRemoval, nodeRemovalResult, newDistPeerNode, newDistPeerNode, assertHeartbeatLiveness) to reduce per-test function length.
- Add pkg/eviction/contract_test.go with a runEvictionContract harness that validates the IAlgorithm contract (Set/Get/Delete/Evict semantics, capacity bounds, zero-capacity no-op); replace duplicate LRU and CAWOLFU test bodies with TestLRUContract / TestCAWOLFUContract.
- Remove all //nolint:ireturn directives across production and test code; remove the golangci.yaml exclusion block that was suppressing lint rules on test files.
- Extract repeated test_value string literals in cmap_test.go into a shared testValue constant.
…rop ireturn suppressions - Add tests/dist_cluster_helper.go with DistCluster, SetupInProcessCluster, and SetupInProcessClusterRF to replace repeated in-process cluster boilerplate in integration, remove/read-repair, stale-quorum, and heartbeat tests. - Add heartbeat test helpers (waitForAllAlive, waitForDeadTransition, waitForNodeRemoval, nodeRemovalResult, newDistPeerNode, newDistPeerNode, assertHeartbeatLiveness) to reduce per-test function length. - Add pkg/eviction/contract_test.go with a runEvictionContract harness that validates the IAlgorithm contract (Set/Get/Delete/Evict semantics, capacity bounds, zero-capacity no-op); replace duplicate LRU and CAWOLFU test bodies with TestLRUContract / TestCAWOLFUContract. - Remove all //nolint:ireturn directives across production and test code; remove the golangci.yaml exclusion block that was suppressing lint rules on test files. - Extract repeated test_value string literals in cmap_test.go into a shared testValue constant.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors test infrastructure and lint configuration to reduce duplicated boilerplate across distributed-cache tests, introduces a shared eviction-algorithm contract test harness, and removes ireturn suppressions (plus the .golangci.yaml test exclusions that were masking several linters in _test.go).
Changes:
- Added shared distributed-cluster and merkle-node test helpers and updated multiple dist/merkle/heartbeat tests to use them.
- Introduced an eviction
IAlgorithmcontract test harness and replaced duplicated LRU/CAWOLFU test bodies with contract coverage. - Removed
//nolint:ireturndirectives broadly and removed.golangci.yamltest-file exclusion rules; extracted repeated test constants to satisfygoconst.
Reviewed changes
Copilot reviewed 42 out of 43 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/merkle_sync_test.go | Uses a shared merkle node helper to cut setup boilerplate. |
| tests/merkle_node_helper.go | Adds newMerkleNode helper for merkle-sync tests. |
| tests/merkle_delete_tombstone_test.go | Refactors merkle tombstone test to use newMerkleNode. |
| tests/management_http_test.go | Extracts management listener wait loop into helper. |
| tests/key_owner_helper.go | Drops ireturn suppression from FindOwnerKey. |
| tests/integration/dist_rebalance_test.go | Extracts shared options + key population helpers for rebalance tests. |
| tests/integration/dist_phase1_test.go | Extracts node creation + replication wait helpers; cleans up value matching logic. |
| tests/hypercache_set_test.go | Extracts shared test key/value constants to satisfy goconst. |
| tests/hypercache_mgmt_dist_test.go | Adds helper to assert mgmt endpoint JSON shape with clearer failures. |
| tests/hypercache_http_merkle_test.go | Extracts HTTP merkle node creation + endpoint readiness polling helpers. |
| tests/hypercache_get_test.go | Refactors Get tests into a shared case runner + typed case struct. |
| tests/hypercache_get_or_set_test.go | Refactors GetOrSet tests into a shared case runner + typed case struct. |
| tests/hypercache_get_multiple_test.go | Reuses shared key constants for multi-get tests. |
| tests/hypercache_distmemory_write_quorum_test.go | Uses shared in-process cluster helper; simplifies quorum failure setup. |
| tests/hypercache_distmemory_versioning_test.go | Uses shared in-process cluster helper; extracts deterministic owner-key search. |
| tests/hypercache_distmemory_tiebreak_test.go | Uses shared in-process cluster helper; reuses deterministic owner-key search. |
| tests/hypercache_distmemory_stale_quorum_test.go | Uses shared in-process cluster helper; extracts requester selection helper. |
| tests/hypercache_distmemory_remove_readrepair_test.go | Replaces bespoke two-node fixture with shared in-process cluster helper. |
| tests/hypercache_distmemory_integration_test.go | Uses shared in-process cluster helper for forwarding/replication integration test. |
| tests/hypercache_distmemory_hinted_handoff_test.go | Extracts hinted-handoff helpers (key selection, metrics assertions, node creation). |
| tests/hypercache_distmemory_heartbeat_test.go | Extracts heartbeat liveness polling/assertion helpers and uses shared peer creation helper. |
| tests/hypercache_distmemory_heartbeat_sampling_test.go | Adds reusable heartbeat peer creation + dead-transition polling helper. |
| tests/hypercache_distmemory_failure_recovery_test.go | Refactors failure/recovery test using shared cluster + extracted hint/transition helpers. |
| tests/dist_cluster_helper.go | Adds shared DistCluster fixture + SetupInProcessCluster* constructors. |
| pkg/eviction/lru_test.go | Removes duplicated contract-style LRU tests (now covered by contract harness). |
| pkg/eviction/contract_test.go | Adds shared IAlgorithm contract test harness and runs it for LRU + CAWOLFU. |
| pkg/eviction/clock.go | Removes ireturn suppressions from helpers. |
| pkg/eviction/cawolfu_test.go | Removes duplicated contract-style CAWOLFU tests (now covered by contract harness). |
| pkg/cache/v2/cmap_test.go | Extracts repeated "test_value" into a constant. |
| pkg/backend/inmemory.go | Removes ireturn suppression from Touch. |
| pkg/backend/dist_transport.go | Removes ireturn suppressions from in-process transport methods. |
| pkg/backend/dist_memory_test_helpers.go | Removes ireturn suppressions from dist-memory test helpers. |
| pkg/backend/dist_memory.go | Removes ireturn suppressions across options/helpers and public methods. |
| pkg/backend/dist_latency.go | Removes ireturn suppressions from latency collector helpers. |
| pkg/backend/dist_http_transport.go | Removes ireturn suppressions from HTTP transport methods/helpers. |
| pkg/backend/dist_http_server.go | Removes ireturn suppressions from HTTP server methods. |
| management_http.go | Removes ireturn suppressions from management HTTP server helpers. |
| internal/transport/transport.go | Removes ireturn suppression from RPCError.Error(). |
| internal/dist/config.go | Removes ireturn suppression from Defaults(). |
| internal/cluster/membership.go | Removes ireturn suppressions from membership methods. |
| hypercache.go | Removes ireturn suppressions from dist introspection helpers. |
| cspell.config.yaml | Adds ignore patterns + dictionary words for new terminology. |
| .golangci.yaml | Removes test-file linter exclusion block so tests run under full lint rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+43
to
+44
| // Node IDs follow the convention "n1", "n2", ... so tests can reason about | ||
| // them deterministically. The shared options are applied to every node; |
Comment on lines
30
to
31
| _ = da.Remove(ctx, "del") | ||
|
|
…rnize ConcurrentMap iterator Split the monolithic hypercache.go (~950 lines removed) into focused files: - hypercache_construct.go: backend resolution and base construction - hypercache_io.go: Set/Get/GetOrSet/GetMultiple/Remove/Clear/List - hypercache_eviction.go: eviction configuration, loop, and triggers - hypercache_expiration.go: expiration routine and trigger logic - hypercache_dist.go: distributed metrics helpers (DistMetrics, ClusterOwners, etc.) Replace ConcurrentMap's channel-based IterBuffered() with iter.Seq2-based All(), eliminating fan-in goroutines and per-shard channel allocations. Update all callers in dist_memory, dist_http_server, and inmemory backends. Upgrade hash function from inlined FNV-1a to xxhash64 (XOR-folded to 32 bits) for better avalanche and ~1-3% speedup on longer keys. Add bench-step3.txt with benchmark results on Apple M4 Pro. BREAKING CHANGE: IterBuffered() and Tuple type removed from ConcurrentMap; use All() iter.Seq2[string, *Item] instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.