Repository navigation
chore(tests): parallelize pkg/common test files - #70
Conversation
Adopts t.Parallel() across all three pkg/common test files (audit_test.go, matches_test.go, types_test.go) — 33 top-level test functions and 8 table-driven subtests now run in parallel. Audit findings (per #58 checklist): - No os.Setenv/t.Setenv usage — no process-wide state. - No package-level mutable fixtures — only engine.go's read-only engineNameMap, which is safe for concurrent readers. - All filesystem-touching tests use t.TempDir() — per-test isolation. - Loop variables captured (tc := tc / tt := tt) inside subtest loops for pre-Go-1.22 safety, though current go.mod targets 1.22+. Verified: - go test -race -count=2 ./common/... clean. - go test -race -count=3 ./common/... clean. - go vet ./common/... clean. - go build ./... clean (root + pkg submodule). Wall-clock: -race -count=2 went from ~3.12s to ~2.78s locally. This is one increment of the per-package adoption sweep tracked in issue #58. Subsequent commits will tackle the other listed packages (internal/api/* other test files, pkg/retry already done, etc.). The integration-test packages (internal/config/* sharing Postgres, internal/server/app_test.go with package vars) require additional schema-isolation work and stay sequential for now. Refs #58
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 11 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
cristim
added
triaged
1 participant
Summary
One incremental step toward closing #58 — adopts
t.Parallel()across all three test files inpkg/common/. Picked as the safest leaf-package starting point: noos.Setenv, no shared mocks, no package-level mutable state, no shared file paths (each filesystem-touching test usest.TempDir()).Per the issue's explicit guidance, this is one package per PR, not a sweeping change. #58 stays open for subsequent packages.
Changes
pkg/common/audit_test.go— 7 top-level tests + 1 table-drivent.Runblock parallelized.pkg/common/matches_test.go— 3 top-level tests + 1 table-drivent.Runblock parallelized.pkg/common/types_test.go— 23 top-level tests + 6 table-drivent.Runblocks parallelized.Total: 33 top-level test functions and 8 sub-test loops. Loop variables (
tc := tc,tt := tt) captured defensively for pre-Go-1.22 safety even though currentgo.modtargets a newer toolchain — costless and matches the existing style in already-parallelized packages.Audit findings
os.Setenv/t.Setenvengine.go's read-onlyengineNameMap(concurrent-read safe)t.TempDir()everywhereVerification
go test -race -count=2 ./common/...— clean (~2.78s wall-clock vs ~3.12s baseline).go test -race -count=3 ./common/...— clean.go vet ./common/...— clean.go build ./...— clean for both root module andpkg/submodule.=== PAUSEtraces in-voutput confirm the runtime is genuinely scheduling tests in parallel.Status of #58
Adopted (already done before this PR):
pkg/exchange/{auto,exchange,reshape}_test.goproviders/aws/services/ec2/client_test.gointernal/api/validation_test.gopkg/retry/exponential_test.goThis PR adds:
pkg/common/{audit,matches,types}_test.go.Still pending audit + parallelization (kept for follow-up PRs):
internal/api/other test files (handler fixtures + shared mocks)internal/config/*_test.gointegration tests (share Postgres container — needs schema isolation)internal/server/app_test.go(package-level varsrunMigrations,migrationsTimeout)pkg/errors/,pkg/logging/,pkg/scorer/,pkg/config/,pkg/provider/,providers/azure/,providers/gcp/,cmd/,cmd/server/,cmd/lambda/,internal/database/Test plan
go test -race -count=2 ./pkg/common/...frompkg/submodulego test -race -count=3 ./pkg/common/...(extra repetition for flake hunting)go vet ./pkg/common/...go build ./...(both modules)Refs #58