Skip to content

FINERACT-2649: Migrate Tier 1 loan integration tests to Feign client#6005

Open
DeathGun44 wants to merge 1 commit into
apache:developfrom
DeathGun44:FINERACT-2649/batch-migrate-loan-tests-to-feign-s1
Open

FINERACT-2649: Migrate Tier 1 loan integration tests to Feign client#6005
DeathGun44 wants to merge 1 commit into
apache:developfrom
DeathGun44:FINERACT-2649/batch-migrate-loan-tests-to-feign-s1

Conversation

@DeathGun44

@DeathGun44 DeathGun44 commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Migrates 5 Tier 1 loan integration tests from BaseLoanIntegrationTest (REST-assured) to FeignLoanTestBase (Feign client).

Migrated Tests

  • LoanPrepayAmountTest
  • LoanProductWithChargeOffBehaviourTest
  • LoanInterestRateFrequencyTest
  • LoanDueCalculationTest
  • FixedLengthLoanProductIntegrationTest

Infrastructure Additions

  • Added retrieveLoanProduct and updateLoanProduct (typed) methods to FeignLoanHelper
  • Added FeignRawHttpHelper for sending raw JSON when explicit null values are required, since the NON_NULL ObjectMapper prevents typed null serialization
  • Added loan product templates to LoanProductTemplates:
    • onePeriod30DaysPeriodicAccrual
    • fourPeriod1MonthWithoutInterest
    • create4IProgressive
  • Added payment allocation helpers and loan request builders to LoanRequestBuilders:
    • applyLoanRequest
    • applyLP2ProgressiveLoanRequest
    • approveLoan with expected disbursement date support
  • Added schedule and transaction validation helpers to LoanTestValidators:
    • verifyRepaymentSchedule
    • verifyTransactions
  • Extended FeignLoanTestBase with helpers migrated from BaseLoanIntegrationTest, including:
    • Loan product builders
    • Installment wrappers
    • Transaction wrappers
    • Other commonly used test utilities
  • Centralized ISO_DATE_PATTERN constant in FeignTestConstants and LoanTestData

Bug Fixes

  • FeignClientHelper.createClient()
    • Uses a fixed past activation date (04 March 2011) matching ClientHelper.DEFAULT_DATE instead of the current system date
  • FeignBusinessDateHelper
    • Added overloads that accept an explicit dateFormat parameter
  • FeignLoanTestBase.runAt() and updateBusinessDate()
    • Added automatic date format detection (ISO vs human-readable) to maintain compatibility with both migrated and existing Feign tests

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Write the commit message as per our guidelines
  • 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 for verifying the changes made.
  • Follow our 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". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.

Your assigned reviewer(s) will follow our guidelines for code reviews.

@Aman-Mittal

Aman-Mittal commented Jun 20, 2026

Copy link
Copy Markdown
Member

@DeathGun44 pls try rebase

@DeathGun44 DeathGun44 force-pushed the FINERACT-2649/batch-migrate-loan-tests-to-feign-s1 branch from 97fc7c9 to bd1f0eb Compare June 20, 2026 19:14
@DeathGun44

DeathGun44 commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

@Aman-Mittal Done!

@DeathGun44 DeathGun44 force-pushed the FINERACT-2649/batch-migrate-loan-tests-to-feign-s1 branch 4 times, most recently from b3c87d6 to d1fbbe7 Compare June 21, 2026 10:19
budaidev
budaidev reviewed Jun 21, 2026
View reviewed changes
Comment thread ...tests/src/test/java/org/apache/fineract/integrationtests/client/feign/FeignLoanTestBase.java Outdated
* Updates a loan product using raw JSON. Use this overload when you need to send explicit null values in the
* request body, which the typed Feign client cannot express due to the global NON_NULL ObjectMapper configuration.
*/
protected void updateLoanProduct(Long productId, String rawJsonBody) {

@budaidev budaidev Jun 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand this is a workaround, but this change will force to load restassured on every subclass, which is not ideal. Can we separate this

@DeathGun44 DeathGun44 Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! It's now a private method in FixedLengthLoanProductIntegrationTest.

Comment thread ...tests/src/test/java/org/apache/fineract/integrationtests/client/feign/FeignLoanTestBase.java Outdated
LoanTestValidators.verifyRepaymentSchedule(loanDetails, installments);
}

protected PostLoanProductsRequest create4IProgressive() {

@budaidev budaidev Jun 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we consolidate this kind of product creation into the LoanProductTemplates? I think this belongs there

@DeathGun44 DeathGun44 Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Moved create4IProgressive() to LoanProductTemplates as a default method.

Comment thread ...c/test/java/org/apache/fineract/integrationtests/client/feign/helpers/FeignClientHelper.java Outdated

public Long createClient() {
return createClient(Utils.dateFormatter.format(Utils.getLocalDateOfTenant()));
return createClient("04 March 2011");

@budaidev budaidev Jun 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic string

@DeathGun44 DeathGun44 Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you ,its now fixed!

@DeathGun44 DeathGun44 force-pushed the FINERACT-2649/batch-migrate-loan-tests-to-feign-s1 branch 2 times, most recently from 74ae125 to db1f724 Compare June 21, 2026 16:45
adamsaghy
adamsaghy reviewed Jun 22, 2026
View reviewed changes
Comment thread ...rc/test/java/org/apache/fineract/integrationtests/FixedLengthLoanProductIntegrationTest.java Outdated
* Updates a loan product using raw JSON. Required here because the global NON_NULL ObjectMapper prevents sending
* explicit null values through the typed Feign client.
*/
private void updateLoanProduct(Long productId, String rawJsonBody) {

@adamsaghy adamsaghy Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our plan is to completely remove RestAssured at some point in the future. I was wondering if we should instead create a fineract client with an object mapper that allows null values to be sent for this test instead? Can you look into whether we can do this?

@DeathGun44 DeathGun44 Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. I've replaced the REST-assured workaround with a FeignRawHttpHelper that uses java.net.HttpURLConnection directly and no REST-assured dependency at all now. this helper can also serve as a drop-in replacement for all remaining Utils.performServer* calls as we phase out REST-assured entirely.

@DeathGun44
FINERACT-2649: Migrate Tier 1 loan integration tests to Feign client
0efd018 
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
@DeathGun44 DeathGun44 force-pushed the FINERACT-2649/batch-migrate-loan-tests-to-feign-s1 branch from db1f724 to 0efd018 Compare June 22, 2026 18:06
@DeathGun44

DeathGun44 commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Updated the pr description

@DeathGun44

DeathGun44 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

unrelated test failures

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamsaghy adamsaghy adamsaghy left review comments

@budaidev budaidev budaidev left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@DeathGun44 @Aman-Mittal @adamsaghy @budaidev