fix(registry): wait for background meta refresh before test reset#894
Conversation
TestComputeMinimumScopeSet can start doBackgroundRefresh via Init() while the next test's resetInit() mutates package-level globals the goroutine still reads (e.g. remoteMetaURL / configuredBrand), causing data races under -race in the coverage job. Track the refresh goroutine with a WaitGroup and drain it at the start of resetInit() in tests. Change-Id: Ibf9475c87bb655e403fb6ffcc3431ef0f340ceb6 Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA package-level ChangesBackground refresh synchronization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Move the drain helper next to resetInit in remote_test.go so non-test builds cannot import a blocking wait API; bgRefreshInFlight stays in production for triggerBackgroundRefresh accounting. Addresses review nit: test-only lifecycle hook belongs in _test compilation. Change-Id: I58b74bcd0fcf5c5e60eb8a3a24be503ea7466c35 Co-authored-by: Cursor <cursoragent@cursor.com>
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@6c2993cc634ee573e741b58c61b981588b645fb1🧩 Skill updatenpx skills add larksuite/cli#fix/registry-wait-background-refresh -y -g |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #894 +/- ##
=======================================
Coverage 65.90% 65.90%
=======================================
Files 520 520
Lines 49274 49277 +3
=======================================
+ Hits 32474 32477 +3
Misses 14026 14026
Partials 2774 2774 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Fixes a data race in
internal/registrytests that causedgo test -race(coverage CI job) to fail intermittently.Root cause
TestComputeMinimumScopeSetcallsInit()→triggerBackgroundRefresh(), which startsdoBackgroundRefreshin a goroutine.TestComputeMinimumScopeSet_Tenant, callsresetInit()while that goroutine can still be runningfetchRemoteMerged/remoteMetaURLand reading package-level globals thatresetInit()writes → race under-race.This is unrelated to doc-only PRs (e.g. #891); it is test-isolation / lifecycle hygiene.
Change
sync.WaitGroup(bgRefreshInFlight).resetInit(), callwaitBackgroundRefresh()before mutating globals.Verification
Review
Per team thread: @codex code owner review; cross-check welcome on test/race narrative.
Made with Cursor
Summary by CodeRabbit
Bug Fixes
Tests