[
https://issues.apache.org/jira/browse/FINERACT-2609?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Krishna Mewara updated FINERACT-2609:
-------------------------------------
Affects Version/s: 1.15.0
> Migrate integration test helpers from RestAssured to Feign client
> -----------------------------------------------------------------
>
> Key: FINERACT-2609
> URL: https://issues.apache.org/jira/browse/FINERACT-2609
> Project: Apache Fineract
> Issue Type: Improvement
> Affects Versions: 1.15.0
> Reporter: Krishna Mewara
> Assignee: Krishna Mewara
> Priority: Trivial
> Labels: beginner, beginner-friendly, feign-migration,
> test-infrastructure
> Fix For: 1.16.0
>
>
> h2. What this is
> Moving Fineract's integration tests off raw RestAssured HTTP and onto the
> generated Feign client ({{{}fineract-client-feign{}}}). I'm doing this
> Strangler Fig style: the new Feign helpers sit alongside the existing
> RestAssured ones and tests move over in batches, so nothing breaks in one go.
> The reason is simple. RestAssured tests build JSON by hand, so a broken API
> contract only shows up when CI runs. Feign is generated from the OpenAPI
> spec, so the same mistake won't compile.
> This is the umbrella issue. To close: FINERACT-2454
> h2. Scope note
> The 50 Retrofit-based tests are out of scope here. They're already type-safe,
> so they're well behind the 148 RestAssured ones in priority.
> {{IntegrationTest}} stays until someone picks them up in a follow-up.
> h2. Where I started — 2026-05-28
> Frozen so we can see the distance travelled. I won't edit this table again.
> ||Metric||Count||
> |Helper files in {{common/}}|123|
> |Helpers using RestAssured only|53|
> |Helpers using both RestAssured and Retrofit|18|
> |Helpers already on Retrofit ({{{}Calls.ok{}}})|35|
> |Helpers with no RestAssured (pure utility)|51|
> |Test files (excluding {{common/}} and Feign)|312|
> |Test files importing RestAssured|197 of 312 (63%)|
> |Feign test files|10|
> |Feign helpers|17|
> |Feign modules|7|
> |Wrapper interfaces|4|
> h2. Where it stands now — 2026-07-31
> ||Metric||Start||Now||Left||
> |Tests on Feign|10|*135* (869 test methods)|—|
> |Tests on RestAssured|197|*148* (1,050 methods, ~82k lines)|148|
> |Tests on Retrofit (neither RA nor Feign)|—|*50*|out of scope|
> |Total test classes|312|333|—|
> |Feign helpers|17|*30*|—|
> |Feign modules|7|*12*|—|
> |{{common/}} helpers still on RestAssured|53|*52*|52|
> |{{Utils.java}} methods marked {{@Deprecated}}|0|*24 of 64*|—|
> 49 test files have come off RestAssured so far. The helper layer is more or
> less built at this point; the bulk of what's left is moving the test files
> themselves.
> If anyone wants to check these numbers, this is how I count them:
> {code:bash}
> cd integration-tests/src/test/java/org/apache/fineract/integrationtests
> RA='io\.restassured|RequestSpecification|Utils\.performServer'
> FG='client\.feign|Feign[A-Z][A-Za-z]*Helper|FeignLoanTestBase|FeignIntegrationTest'
> grep -rl "@Test" . --include=*.java | wc -l
> # total
> grep -rl "@Test" . --include=*.java | xargs grep -lE "$RA" | wc -l
> # on RestAssured
> grep -rl "@Test" . --include=*.java | xargs grep -LE "$RA" | xargs grep -lE
> "$FG" | wc -l # on Feign
> grep -rl "@Test" . --include=*.java | xargs grep -LE "$RA" | xargs grep -LE
> "$FG" | wc -l # on Retrofit
> grep -rlE "$RA" ./common --include=*.java | wc -l
> # RA helpers
> {code}
> h2. What's left, by area
> ||Area||Classes||Tests||Blocked on||
> |Loan|57|470|nothing|
> |Savings and deposits|19|199|deposit helpers not written yet|
> |Organisation and misc|37|181|nothing|
> |Jobs and batch|5|69|nothing|
> |Client|9|55|nothing|
> |Accounting|11|40|nothing|
> |Datatable and entity|4|22|nothing|
> |Reporting, shares, bulkimport, group|10|36|nothing|
> |*Total*|*148*|*1,050*| |
> These six are big enough that each one needs its own PR:
> ||Class||Tests||Lines||
> |{{AdvancedPaymentAllocationLoanRepaymentScheduleTest}}|86|6,625|
> |{{ClientLoanIntegrationTest}}|66|8,354|
> |{{FixedDepositTest}}|47|3,200|
> |{{ClientSavingsIntegrationTest}}|45|3,662|
> |{{BatchApiTest}}|40|2,750|
> |{{RecurringDepositTest}}|38|3,128|
> h2. Old classes on the way out
> ||Class||Start||Now||Blocked on||
> |{{BaseLoanIntegrationTest}}|121|*53* (52 subclasses)|nothing|
> |{{BaseSavingsIntegrationTest}}|—|*3 subclasses*|savings batch|
> |{{ClientHelper}}|216|*170*|client batch|
> |{{LoanTransactionHelper}}|118|*125*|loan batch|
> |{{SavingsAccountHelper}}|37|*35*|savings batch|
> |{{Utils.java}}|309|*345*|everything else|
> |RestAssured-only helpers in {{common/}}|53|*52*|per-area Feign helpers|
> Worth flagging: {{Utils.java}} and {{LoanTransactionHelper}} both went
> {*}up{*}, not down. New tests are still being written against the old layer
> while I'm migrating the old ones off it.
> h2. Things I know are still open
> A few gaps I'd record:
> * {{FeignLoanHelper}} still has a handful of methods that take a JSON string
> and post it through the old RestAssured utility. Any test going down those
> paths isn't genuinely off RestAssured, so the 135 figure above is slightly
> generous. Separate ticket for it.
> * One test living under the Feign folder still builds a RestAssured request
> spec itself.
> * {{FeignLoanTestBase}} is now 1,562 lines, 224 methods and 99 subclasses.
> It's drifting towards being the same kind of god class I'm trying to delete.
> I'd rather split it once the area batches have landed than churn it
> mid-migration.
> h2. How the Feign side is put together
> Helpers live under {{client/feign/helpers/}} (30 of them so far). Each takes
> {{FineractFeignClient}} in the constructor, calls the API through {{ok(...)}}
> / {{{}fail(...) }}from \{{{}FeignCalls{}}}, and returns the whole response
> object rather than just an ID.
> Request building lives in {{client/feign/modules/}} (12 files: the
> {{{}*RequestBuilders{}}}, {{{}LoanProductTemplates{}}},
> {{{}LoanTestValidators{}}}, test data holders). Tests extend
> {{{}FeignIntegrationTest{}}}, {{FeignLoanTestBase}} or
> {{{}FeignSavingsTestBase{}}}.
> Both clients are on the classpath at the same time. The {{exclude}} in
> \{{dependencies.gradle }}keeps the duplicate model classes apart.
> Conventions I've settled on as I've gone:
> * Feign helpers are new classes, not extra methods bolted onto the old
> helpers
> * Wrapper interfaces only where the generated name is unreadable
> ({{{}create6(){}}} → {{{}createClient(){}}})
> * New shared code goes in a helper or a module, never on the test base.
> One-off methods stay {{private}} in the test that needs them.
> * Where the generated model genuinely can't express a call, I drop to a
> small raw-HTTP helper and leave a comment explaining why. Never RestAssured.
> If the gap is only a field or two, I fix the Swagger DTO and regenerate
> instead. I keep a running list of every one of these so none of them get
> forgotten.
> * Coexistence was the right call for the first phase. It ends when criterion
> #4 lands.
> h2. PR roadmap
> I'm keeping the PR count deliberately low and putting the detail into commits
> instead. Each remaining PR is one area, broken into commits that each build
> and pass on their own, so they can be reviewed one at a time without the PR
> itself becoming unreviewable.
> PRs 1 to 8 are what I'm doing in GSoC. Everything from 9 down is stretch,
> listed in the order
> I'd pick it up.
> ||PR||What||Files||Tests||Ticket||GitHub||Status||
> |1|Savings helper, test base, Swagger fields|—|—|FINERACT-2620, 2626|#5907,
> #5934|Merged|
> |2|Client helper build-out, Swagger fields|—|—|FINERACT-2631, 2632|#5954,
> #5955|Merged|
> |3|Charges and delinquency helpers|—|—|FINERACT-2635|#5978|Merged|
> |4|Loan helper: charges, transitions,
> disbursement|—|—|FINERACT-2640|#5997|Merged|
> |5|Loan tests, 4 tiers|—|—|FINERACT-2649|4 PRs, see ticket|Last tier in
> review|
> |6|Group and centre|3|16|FINERACT-2718|#6194|*In review — I'm here*|
> |7|Loan helper RestAssured leak, plus the two biggest loan files|2|152|to
> raise|—|*Next*|
> |8|Guardrail and contributor guide|—|—|to raise|—|*Planned*|
> |9|Rest of loan money movement|19|208|to raise|—|Stretch|
> |10|Savings and deposits|20|209|FINERACT-2732|3 Prs Planned|Tier 1 in review|
> |11|Loan lifecycle, product, COB|33|113|to raise|—|Stretch|
> |12|Client, charges, accounting|20|101|to raise|—|Stretch|
> |13|Organisation, jobs, batch, config|33|228|FINERACT-2746|—|Stretch|
> |14|Closeout: delete the old base classes and helpers|—|—|to raise|—|Stretch|
> |15|Bulk import, shares, reporting, smoke tests|21|39|to raise|—|Stretch|
> |16|Split {{FeignLoanTestBase}}|—|—|to raise|—|Stretch|
> |17|The 50 Retrofit tests|50|—|to raise|—|Stretch|
> PRs 7 to 11 add up to 127 files and 1,011 tests. S1 covers the last 21 files
> and 39 tests.
> h2. Log
> One line per merge. I don't edit old entries, only add to the bottom.
> ||Date||PR||Ticket||What went in||
> |2026-06-04|[#5934|https://github.com/apache/fineract/pull/5934]|FINERACT-2626|Added
> {{rejectedOnDate}} and {{withdrawnOnDate}} to the savings Swagger spec. The
> generated model was missing them, so a reject call silently did nothing.
> Needed before #5907 could land.|
> |2026-06-07|[#5907|https://github.com/apache/fineract/pull/5907]|FINERACT-2620|First
> area done end to end. 10 new files: savings helper, test base, lifecycle
> extension, request builders, and an 8-test lifecycle test to prove the
> pattern works.|
> |2026-06-09|[#5955|https://github.com/apache/fineract/pull/5955]|FINERACT-2632|Client
> Swagger fields the server accepts but the model didn't have:
> {{{}submittedOnDate{}}}, the withdrawal fields, and
> {{{}firstname{}}}/{{{}lastname{}}} on PUT.|
> |2026-06-09|[#5954|https://github.com/apache/fineract/pull/5954]|FINERACT-2631|{{FeignClientHelper}}
> from 4 methods to 16, plus {{ClientRequestBuilders}} and a full client
> lifecycle test.|
> |2026-06-11|[#5978|https://github.com/apache/fineract/pull/5978]|FINERACT-2635|Charges
> and delinquency: {{FeignChargesHelper}} (14 methods),
> {{{}ChargeRequestBuilders{}}}, and a 7-test delinquency lifecycle. Also fixed
> 4 fields missing from {{PostChargesRequest}} that were sending nulls on PUT.|
> |2026-06-21|[#5997|https://github.com/apache/fineract/pull/5997]|FINERACT-2640|Loan
> helper grew charge management, state transitions, disbursement details and
> reschedule: 11 charge methods, 6 transitions, 4 disbursement methods.|
> |from 2026-06-26|4 PRs|FINERACT-2649|The loan test migration, done in four
> tiers. Tier 4 is the big one at 92 files. Per-PR detail and review notes are
> on that ticket rather than duplicated here. Final tier is in review.|
> |2026-07-28|[#6194|https://github.com/apache/fineract/pull/6194]|FINERACT-2718|Group
> and centre. New helpers for group, centre, staff and GLIM. Migrated
> {{{}CenterIntegrationTest{}}}, {{GroupTest}} and {{GroupLoanIntegrationTest}}
> (16 tests) and retired a raw-HTTP stopgap helper that was Feign in name only.
> No changes to the shared loan test base. In review.|
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
