DeathGun44 opened a new pull request, #6158: URL: https://github.com/apache/fineract/pull/6158
## What this PR does Final batch of the loan integration-test migration. It moves **52 test classes (632 test methods)** off REST-assured onto the typed Feign client. None of those 52 classes reference REST-assured any more. With this merged, 149 integration-test classes run on the typed client. 90 files changed: - 52 test classes - 35 shared Feign helpers and builders (15 of them new) - 3 Swagger DTO files (details below) The tests assert the same things as before. This changes how they talk to the server, not what they check. The few assertions that did change are listed under "Bugs found" and each one is a fix. ## Commits **25 commits**, each a group of related tests (loan charges, COB, external asset owner, delinquency, advanced payment allocation, and so on). ## What's covered - **Client loan lifecycle** - charges of every calculation type, credit balance refund, external IDs, backdated disbursement, overpaid status, modifying approved amounts, floating-rate interest recalculation, date validation. - **Close of Business (COB)** - partitioning, catch-up, inline COB, account locks. - **External asset owner** - transfer, cancel, search, initiate/undisbursed. - **Advanced Payment Allocation** - repayment schedules, interest refunds, charges, reprocessing. - **Charges** - specific due date, after maturity, instalment fees, penalties, charge-off accounting. - **Delinquency** - buckets, business events, classification. - **Batch API, scheduler jobs, journal reversal, SQL-injection reporting.** ## Bugs found The typed client is stricter than REST-assured, so a few real problems showed up: - **Wrong charge calculation type.** `ChargeRequestBuilders` had `CHARGE_CALCULATION_TYPE_PERCENTAGE_AMOUNT_AND_INTEREST = 4`, but 4 is percent-of-interest. Percent-of-amount-and-interest is 3, which is what `ChargesHelper` uses and what the tests meant. Every test using that builder was charging on the wrong base and still passing, because the amounts were plausible. Now uses `ChargeCalculationType.PERCENT_OF_AMOUNT_AND_INTEREST`. - **A comparison that could never be true.** In `testLoanCharges_INSTALMENT_FEE` the instalment numbers are `Long` (the request model requires it) but the schedule reports its period as an `Integer`. `Long.equals(Integer)` is always false, so the "this is the waived instalment" and "this is the paid instalment" branches never ran and the test compared against the wrong expected values. Now compared numerically. - **A crash hidden in a helper.** `modifyLoanApplication` passed `command` through `Map.of(...)`, which rejects nulls. Three charge tests in `ClientLoanIntegrationTest` pass null here and were throwing `NullPointerException` before reaching the server. Now uses the typed query parameter, which drops a null. - **A dropped product setting.** `LoanProductTestBuilder.withDisallowExpectedDisbursements(true)` also quietly turned on a 100% over-applied disbursement allowance. The migrated builder set it to `false`, which broke `chargeOff` because it disburses two 1000 tranches against a 1000 approval. Restored. - **A COB step missing its prerequisite.** `CobPartitioningTest` registered the external-asset-owner COB step but had lost the `ASSET_TRANSFER` GL mapping that step needs. The step list is tenant-wide and never restored, so this broke an unrelated later test. - **Weak assertions.** The SQL-injection tests caught bare `RuntimeException` as "the server rejected this", so a client-side connection failure counted as a pass. They also looked for "404" in the exception message, which includes the request path, so a report whose ID contained `404` hid a real failure. Both now check the HTTP status. - **Interest calculation changed.** One charges test had moved from `SAME_AS_REPAYMENT_PERIOD` to `DAILY`. Reverted. - **Locale changed.** Account transfers were posted with `en` instead of `en_GB`. Restored. ## Shared test infrastructure - **`FeignRawHttpException`** (new). `FeignRawHttpHelper` now throws an exception with the HTTP status, body, method and path as fields instead of packing them into a message string. Tests branch on the status instead of matching substrings. It extends `RuntimeException`, so existing callers are unaffected. - **`LoanRequestBuilders`** - added a type-safe `paymentAllocationOrder(PaymentAllocationType...)` overload, replacing copies of the same private helper in individual tests. - **`ChargeRequestBuilders`** - dropped its three local `CHARGE_CALCULATION_TYPE_*` constants. All call sites now use the `ChargeCalculationType` enum, so the name and the value can't drift apart again. ## Checklist Please make sure these boxes are checked before submitting your pull request - thanks! - [ ] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests) - [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers. - [ ] Create/update [unit or integration tests](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made. - [ ] Follow our [coding conventions](https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions). - [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes - [ ] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute). - [ ] If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately. Your assigned reviewer(s) will follow our [guidelines for code reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
