test: Split Loan_test.cpp into topical suites - #7864
Open
Tapanito wants to merge 3 commits into
Open
Conversation
Loan_test.cpp had grown to ~9700 lines covering lifecycle, invariant regressions, RIPD/bug repros, and cash-basis accounting in one class. Split into Loan (core transactor tests), LoanInvariant (fuzzer/invariant repros), LoanRegression (RIPD-*/bug:/edge: repros), and LoanCashBasis (cash-basis accounting), sharing fixtures and helpers via a new LoanTestBase. LoanBatch_test/LoanArbitrary_test are rebased onto LoanTestBase instead of Loan_test for the same reason. Pure reorganization: combined case/assertion counts across the four new suites match the pre-split totals exactly (1070 cases, 116951 assertions, 0 failures).
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Supersedes the previous 4-file split (Loan/LoanInvariant/ LoanRegression/LoanCashBasis) with a more granular, 10-suite layout: - Loan: core LoanSet lifecycle - LoanLifecycle: full lifecycle, self-loan, issuer/broker-as-borrower - LoanValidation: disabled-amendment and invalid-transaction checks - LoanPay: payment-mechanics regressions (fees, near-zero rate, overflow) - LoanInvariants: fuzzer-derived invariant regressions - LoanRounding: precision/scale/dust rounding regressions - LoanCoverFreezeAuth: first-loss-cover, freeze, and MPT auth permissions - LoanSecurity: PoC and RIPD-numbered regressions - LoanCashBasis: unchanged, LendingProtocolV1_1 cash-basis coverage - LoanMisc: RPC signing flow, plus the LoanBatch/LoanArbitrary manual fuzz suites relocated here Shared fixtures/helpers remain in LoanTestBase.h; testCaseWrapper moves there too since it's used by both LoanSet (Loan) and testLifecycle (LoanLifecycle), not just the former. Pure reorganization: every function body is byte-identical to the prior split, and independent/sensitive call-site wiring matches exactly (verified via diff). The combined suite total is 1071 cases (vs. 1070 previously) with an identical 116951 assertions and 0 failures. The extra case is a benign case-boundary bookkeeping artifact: testLoanSet's (unchanged) setup code performs real transaction-application work before its own first testcase() call, and now that it's the sole/first function in its own suite, that pre-existing activity surfaces as a new blank case instead of attaching to a predecessor's case as it did when other functions ran before it in the old grouping. No assertions were gained, lost, or duplicated.
Run clang-tidy (misc-include-cleaner) scoped to the files touched by the Loan test split. Trims LoanTestBase.h down to only the includes its shared helpers actually need now that most test bodies moved out, and adds direct includes to each split file for symbols they use. Restores two includes the automated fixer incorrectly dropped (test/jtx/jtx_json.h for Json(...), test/jtx/sponsor.h for sponsor::As) which broke the build; clang-tidy's usage tracking doesn't reliably see through these. Verified with a full clean rebuild and test run: 1071 cases, 116951 assertions, 0 failures, unchanged from before this cleanup.
gregtatcam
approved these changes
Jul 24, 2026
gregtatcam
left a comment
gregtatcam
left a comment
Contributor
There was a problem hiding this comment.
LGTM
One thought. Does it make sense adding a brief comment to each module describing what tests it covers so it's easier to figure out where the future tests should go to? Some file names are obvious but some are too general.
a1q123456
approved these changes
Jul 27, 2026
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
3 participants
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.
Summary
Loan_test.cpphad grown to ~9700 lines covering core transactor tests, fuzzer/invariant regressions, RIPD/bug repros, and cash-basis accounting all in oneSuite. Split into four topical suites —Loan,LoanInvariant,LoanRegression,LoanCashBasis— following the existing precedent for large domains in this codebase (e.g.AMM_test/AMMCalc_test/AMMClawback_test).BrokerInfo,createVaultAndBroker,createFundedIouAsset,nextLoanKeylet,makeLoanPayments, etc.) move into a newLoanTestBase, which every split suite — including the existingLoanBatch_test/LoanArbitrary_testmanual fuzz suites — inherits from instead ofLoan_test.createFundedIouAsset,nextLoanKeylet,createFundedRippleIouAsset,brokerPseudoAccount), and four near-identical fuzzer-derived invariant tests were merged into one (testLoanPayComputePeriodicPaymentInvariants), each finding kept as its owntestcase().Based on
tapanito/lending-cash-basis(#7817) rather thandevelop, since this only reorganizes tests already on that branch.Test plan
rw make) compiles all 5 changed/new files with no new warnings../xrpld -u Loan,LoanInvariant,LoanRegression,LoanCashBasisreports 1070 cases, 116951 assertions, 0 failures — an exact match to the pre-split monolithic suite's totals.LoanBatch,LoanArbitrary) still compile and run; confirmed their underlying logic (runLoan/createLoan/makeLoanPayments/testRandomLoan, deterministic RNG seed) is byte-identical to the pre-split code, so their behavior (includingLoanBatch's pre-existing, seed-dependent failures on some random draws) is unchanged by this move.