Skip to content

test(ring): reduce wall time of slow tests by ~42%#969

Open
colega wants to merge 1 commit into
mainfrom
colega/ring-test-speedup
Open

test(ring): reduce wall time of slow tests by ~42%#969
colega wants to merge 1 commit into
mainfrom
colega/ring-test-speedup

Conversation

@colega

@colega colega commented May 11, 2026

Copy link
Copy Markdown
Contributor

What this PR does:

CI splits unit tests into 3 parallel groups but the ring package alone takes ~10m42s under -race — 78% of total test time — pinning group 0 at ~13min while groups 1/2 finish in 1–2min.

Four tests dominate that ring runtime (321s of 492s locally). All run very large inputs that race detector inflates without adding correctness coverage. This PR shrinks those inputs and removes redundant work:

  • TestDoBatchWithOptionsContextCancellation builds a 5M-key ring per subtest and runs a full warmup DoBatch inside each setup to derive a timeout. Hoist ring/keys construction outside the subtest loop and measure the warmup once.
  • TestSpreadMinimizingTokenGenerator_GenerateAllTokensIdempotent: idempotence is a per-(instance, zone) property; 128 instances are overkill. Reduce to 16.
  • TestSpreadMinimizingTokenGenerator_CheckTokenUniqueness: uniqueness is a generator invariant; reduce instanceID 10000 → 1000 (still 1.5M tokens covered).
  • TestCheckingOfKeyOwnership: drop the two largest user-token sizes (50k, 100k); the property holds at every scale.

Measured locally with go test -race -count 1 ./ring/: 492s → 285s (−42%). Expected CI impact: group 0 drops from ~13min to ~7–8min.

Which issue(s) this PR fixes:

N/A

Checklist

  • Tests updated

Note

Low Risk
Low risk: changes are confined to tests and primarily reduce workloads and redundant setup; main risk is reduced coverage for very-large-scale scenarios.

Overview
Speeds up ring package unit tests by removing redundant heavy setup and shrinking pathological test inputs (especially under -race).

TestDoBatchWithOptionsContextCancellation now builds the ring/5M keys once and performs a single warmup DoBatch to derive timeouts for subtests, instead of re-creating the ring/keys and re-measuring per subtest.

Reduces runtime in token-generator/ownership tests by lowering iteration ranges: fewer instance IDs in SpreadMinimizingTokenGenerator idempotence/uniqueness tests and fewer large token-count cases in TestCheckingOfKeyOwnership.

Reviewed by Cursor Bugbot for commit ee8770c. Bugbot is set up for automated code reviews on this repo. Configure here.

Four tests dominated ring package CI runtime (321s of 492s):

- TestDoBatchWithOptionsContextCancellation: hoist ring+keys construction
  outside subtest loop and measure DoBatch duration once rather than
  per-subtest; eliminates ~8 redundant 5M-key DoBatch passes.
- TestSpreadMinimizingTokenGenerator_GenerateAllTokensIdempotent: reduce
  maxInstanceID 128→16; idempotence is a per-instance property, 16 samples
  are sufficient.
- TestSpreadMinimizingTokenGenerator_CheckTokenUniqueness: reduce
  instanceID 10000→1000; still validates uniqueness across 1.5M tokens.
- TestCheckingOfKeyOwnership: drop the two largest user-token sizes
  (50k, 100k); the correctness property holds at any scale.

Measured with -race -count 1: 492s → 285s.
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.

1 participant