Repository navigation
test: replace no-value tests with exact oracles (#370) - #403
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The shed case drops the carry against add's NatSpec. Its value is not asserted until that ruling, rather than absorbed by a two-unit bound. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Unbounded exponents never reach the window where the quotient sheds digits below the floor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 29 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThis PR updates decimal-float tests to check exact or bounded results for addition, division, agreement, coefficient truncation, and exponentiation. It removes fuzz tests that only checked for non-reversion and adds an audit scan record for Issue ChangesDecimal-float test assertions
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other Merge Risk: ⚪ Minimal · up to The reviewed test changes are mergeable after normal checks; no concrete regression remains identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1
✨ Finishing Touches 💡 1
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 |
Restores the exact overflow-shed assertion in checkAddRoundsTheExactSum now that #414 keeps add's carry: the shed sum rounds towards zero to ten units. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve onto main's one-helper-per-job LibTestExactDecimal:
- add: testAddRoundsTheExactSum{,NearbyExponents} now assert checkAddExact
(addPartsWide exact parts) in place of the local bound oracle
checkAddRoundsTheExactSum; magnitudeAt, sub512 and sameValue (a copy of
LibTestExactDecimal.eq) are dropped.
- div: checkDivByMaxTruncatesTheExactQuotient asserts the exact parts from
divParts moved to the call's exponents and atFloor, in place of the
local truncation bounds.
- audit/mutation-test-scans.json is the jq union of both sides.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Closes #370
Replaces the tests that only checked for no revert with exact oracles. Where a value fuzz already covers the value, the test is deleted instead.
add:testAddNeverRevertbecomestestAddRoundsTheExactSumandtestAddRoundsTheExactSumNearbyExponents. They check add's documented rule against the exact 512-bit sum: round to the larger operand's int256 unit, towards zero when the signs agree and away from zero when they differ. A same-signed sum past int256 must land at exponent T+1, rounded towards zero to ten units. This shed value was left out until add: keep the operands' carry when the sum overflows int256 (#394) #414 fixed add's carry, and is now asserted.div:testDivMaxPositiveValueDenominatorNotRevertbecomes an exact-quotient truncation oracle. This PR adds a fuzz near the int256 floor and the one-unit-at-the-floor case, because unbounded exponents never reach the window where the quotient sheds digits.pow: 2^1e9 is checked againstbc -lat scale 200.packArithmeticResult: int224.max at exponent 2, in place ofassertGt(coef, 0).agree: the sub-underflow counterexample asserts its answers: forward is true and backward is false.testAgreeNeverRevertsOnValuesis deleted. The no-revert property it stood for is covered by the Rustlibrary_agreeproptest (crates/tests) and bytestAgreeAcrossTheWholeRange(see G2).testEqNotReverts,testWithTargetExponentLargerTargetExponentNoRevert,testFracNotReverts,testIntegerNotReverts,testFloorNotReverts,testCeilNotReverts.No src changes.
QA
testAddRoundsTheExactSum(NearbyExponents),testDivMaxPositiveValueDenominatorTruncatesTheExactQuotient,testDivMaxPositiveValueDenominatorNearTheFloor,testDivMaxPositiveValueDenominatorOneUnitAtTheFloor,testPowIntegerExponentSquaringOverflow,testPackArithmeticResultToleratesCoefficientTruncation,testAgreeSurvivesTheSubUnderflowCounterexample. Each was probed withmutation-probeagainst its old version on main 8fcfeb3. The old versions kill only the revert-introducing mutants, and the new versions also kill the value mutants. See the matrix.test/lib/LibTestExactDecimal.solfor add and div.bc -lat scale 200 for 2^1e9. The int224 range for pack. Hand arithmetic in units of 1e-2147483648 for the agree counterexample. None of these is derived from the implementation.Columns: old alone is the replaced or deleted test alone on main. new alone is the replacement alone. file main and file branch are the whole test file on each tree.
>→>=<→<=testAddAtFloorandtestAddNearFloorMatchesShiftedkill A2.testAddAtFloorandtestAddingSmallToLargeReturnsLargeExampleskilled A7. add: keep the operands' carry when the sum overflows int256 (#394) #414 rewrote that branch, so A7's target no longer exists. The table below probes the new branch.testDivAdjustExponentFullDivisor,testDivBy1and others kill D1 and D2.testAgreeNeverRevertsOnValuesdid not kill G2.testAgreeAcrossTheWholeRange,testAgreeExtremeToleranceand the Rustlibrary_agreeproptest kill it. A probe withforge build && cargo test -p rain-math-float-tests library_agreeas the suite killed both G1 and G2. The agree no-revert deletion relies onlibrary_agree.testIntFracExamplesandtestIntFracNegExponentSmallkill it.Overflow shed after #414
Merging main at 783048c brought in #414, which keeps add's carry past int256.
checkAddRoundsTheExactSumnow asserts the value it shed:|result| × 10^78 ≤ |sum| < (|result| + 1) × 10^78, in units of10^(T-77). That is the exact sum rounded towards zero to ten units at T+1, which is add's NatSpec rule. The comment pointing at #363 is gone.The S mutants target #414's overflow branch. They were probed with
mutation-probeagainst three suites: the restored tests alone, the add test file on main 783048c, and the add test file on this branch (0afe3dd).#414's own tests on main already kill every S mutant. The restored assertion also kills all 7 by itself. I did not probe the restored tests with the shed assertion removed, so this table does not separate what the assertion kills from what the rest of the check kills. Full suites at 0afe3dd:
forge testpassed 836 of 836 andcargo test(crates/tests) passed 152 of 152.Scan recorded in
audit/mutation-test-scans.json.🤖 Generated with Claude Code
Summary by CodeRabbit