test(evaluator): exercise ResolveStringValue in concurrency test#1986
test(evaluator): exercise ResolveStringValue in concurrency test#1986anxkhn wants to merge 1 commit into
Conversation
In TestFlagStateSafeForConcurrentReadWrites, the "Update_ResolveStringValue" case called ResolveBooleanValue with StaticStringValue (a variant value) passed as the flag key, instead of ResolveStringValue with the StaticStringFlag key like every sibling case. Because the concurrent goroutine discards the resolution result and only surfaces data races or panics, the mismatch passed silently: ResolveStringValue was never driven under concurrent read/write, while ResolveBooleanValue was exercised twice. Call ResolveStringValue with StaticStringFlag so the case matches its name and actually covers the string-resolve path concurrently with SetState. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
✅ Deploy Preview for polite-licorice-3db33c canceled.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA test case in the concurrency safety test suite is corrected to invoke ChangesTest fix
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
|



This PR
In
TestFlagStateSafeForConcurrentReadWrites(
core/pkg/evaluator/json_test.go), the"Update_ResolveStringValue"casedoes not resolve a string flag. It calls
ResolveBooleanValueand passesStaticStringValue(a variant value,"#CC0000") as the flag key, whereasevery sibling case calls its namesake resolver with the matching
*Flagkeyconstant:
The resolving goroutine discards the result (
_ = tt.flagResolution(...)) andonly surfaces data races and panics, so the mismatch passes silently. The net
effect is that
ResolveStringValueis never driven under concurrentread/write (the path this case is named for gets zero race coverage), while
ResolveBooleanValueis exercised twice.This is a test-only change; no production code is touched.
What's changed
One line in the
"Update_ResolveStringValue"case so it matches its name andthe sibling pattern:
StaticStringFlag("staticStringFlag") is a realENABLEDstring flag inthe test's
flagConfig, so the corrected call resolves an existing flag andnow genuinely covers the string-resolve path concurrently with
SetState.Testing
All green under
-race.golangci-lint(repo-pinned v2.7.2) reports 0 issueson the package.