test(reconcile): guard canonicalJSON number precision with a 2^53 pair - #1884
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe canonical JSON precision test now uses ChangesJSON Precision Test
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to This test-only change strengthens protection against JSON number-precision regressions without changing production behavior; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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 |
Coverage Report for CI Build 31784755875Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage remained the same at 48.641%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Summary
Fixes the
TestCanonicalJSONlarge-integer case so it actually guards theUseNumberdecode incanonicalJSON.The problem
The test compared
10000000000000001and10000000000000002. Under the oldfloat64decode those round to10000000000000000and10000000000000002, which already differ, so the test passed even without the fix. RevertingdecodeJSONtojson.Unmarshalleft the test green, so it caught nothing.The fix
Use the
2^53pair instead:9007199254740993and9007199254740992both round to9007199254740992underfloat64, so they compare equal on the old code. The test now fails withoutUseNumberand passes with it.Test Plan
UseNumber(current code) the test passes.dec.UseNumber()makes it fail (both inputs canonicalize to9007199254740992).UseNumberpasses again.go build ./...andgofmtclean.