test(pair): cover StalePrice and PriceNotFinalized from both fetch legs#192
Open
thedavidmeister wants to merge 2 commits into
Open
test(pair): cover StalePrice and PriceNotFinalized from both fetch legs#192thedavidmeister wants to merge 2 commits into
thedavidmeister wants to merge 2 commits into
Conversation
Adds four revert-path tests to LibOpFtsoCurrentPricePairTest. The pair op fetches symbolB (denominator) first then symbolA (numerator), so first-leg tests only mock FTSO_B with mockRegistry(1) — FTSO_A is never touched and must not have vm.expectCall set up. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughFour new test functions are added to ChangesPair op revert propagation tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Formatting: trailing double-space before // and multi-line vm.expectRevert. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #58
Summary
The pair op (
LibOpFtsoCurrentPricePair) fetches symbolB (denominator) first, then symbolA (numerator), via two calls toLibOpFtsoCurrentPriceUsd.run. Issue #58 noted that onlyInactiveFtsowas exercised;StalePrice,PriceNotFinalized, andDecimalsTooLargepaths had no tests.This PR adds four revert-path tests:
testRunPairStalePriceFirstLeg— symbolB stale; only FTSO_B mocked (symbolA never reached)testRunPairStalePriceSecondLeg— symbolB fresh, symbolA stale; both FTSOs mockedtestRunPairNotFinalFirstLeg— symbolBNOT_FINALIZED; only FTSO_B mockedtestRunPairNotFinalSecondLeg— symbolB finalized, symbolANOT_FINALIZED; both FTSOs mockedKey insight for first-leg tests: because the pair op's trampoline fetches symbolB first, any error on symbolB means symbolA is never queried. First-leg tests use
mockRegistry(1)and omit all FTSO_A mocks — addingvm.expectCallfor FTSO_A would create an unsatisfied expectation and cause spurious failures.DecimalsTooLargeis not included here: the pair op delegates decimal handling toLibDecimalFloat, and the existingtestRunPairZeroQuoteReverts/testRunPairZeroBaseIsZeroalready cover division-by-zero; aDecimalsTooLargepath would require fabricating an impossible decimal count from the mock FTSO, which is out of scope.Test plan
LibOpFtsoCurrentPricePairTesttests pass locallyrainix-sol / test / testgreen (note: pre-existing fork tests using the public Ankr RPC are intermittently rate-limited on CI — not caused by this PR)🤖 Generated with Claude Code
Summary by CodeRabbit