Skip to content

FINERACT-2494: Add unit tests for ApiParameterHelper in fineract-core.#5677

Open
San-43 wants to merge 2 commits intoapache:developfrom
San-43:FINERACT-2494-api-parameter-helper-tests
Open

FINERACT-2494: Add unit tests for ApiParameterHelper in fineract-core.#5677
San-43 wants to merge 2 commits intoapache:developfrom
San-43:FINERACT-2494-api-parameter-helper-tests

Conversation

@San-43
Copy link

@San-43 San-43 commented Mar 22, 2026

Description

This PR fixes FINERACT-2494.

New test coverage for API parameter extraction and parsing:

  • Added ApiParameterHelperTest with tests for extracting and validating commandId, fields, associations, and locale from query parameters, including handling of missing, blank, invalid, and duplicate values.
  • Included tests for boolean query parameter parsing for methods such as parameterType, template, makerCheckerable, includeJson, and genericResultSet, verifying correct handling of true, false, and missing values.
  • Provided tests for exclusion logic in associations and for presence checks on the genericResultSet flag.
  • Added a helper method to simplify test parameter creation, improving readability and maintainability of the test code.

Checklist

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

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

@Ambika-Sony
Copy link
Contributor

Ambika-Sony commented Mar 23, 2026

Hi @San-43 , I see you're working on this issue!

I’ve already submitted PR #5665 addressing FINERACT-2494. My approach focuses on building a comprehensive unit test suite for ApiParameterHelper, including coverage for edge cases such as null handling, negative offsets, malformed inputs, and boundary conditions.

The tests are designed as isolated JUnit 5 unit tests to ensure reliability and maintainability within fineract-core, and I’m also aligning with static analysis improvements from FINERACT-2510.

Happy to align approaches or incorporate any feedback from the community.

@San-43
Copy link
Author

San-43 commented Mar 23, 2026

Hi @Ambika-Sony, thanks for the note and for your work on this. I actually hadn’t noticed #5665 when I started #5677. I saw a couple of closed PR targeting FINERACT-2494, but I missed #5665, apologies for that.

After comparing #5665 and #5677, my view is that #5677 is a bit cleaner for FINERACT-2494 because it keeps the change strictly tests-only, in line with the ticket scope. In #5665, there are also ApiParameterHelper production-code changes; those hardening updates are valuable, but they may be better handled in a separate PR/ticket so scope and review impact stay clear.

I’m happy to align and avoid duplication. If maintainers prefer a single path, I can either:

  1. close this PR, or
  2. keep this one for tests-only and leave behavior changes to a follow-up PR.

Happy to follow maintainer guidance either way.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants