You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "Cocoa-Puffs (via GitHub)" <gi...@apache.org> on 2026/09/23 09:49:08 UTC
[PR] FINERACT-2455: Fix e2e execution times of DelinquencyPause and GoodwillCreditPaymentAllocation features [fineract]
Cocoa-Puffs opened a new pull request, #6497:
URL: https://github.com/apache/fineract/pull/6497
## Description
Describe the changes made and why they were made. (Ignore if these details are present on the associated Apache Fineract JIRA ticket.)
## 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.
- [ ] I followed the [AI Policy](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#ai-policy).
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Fix e2e execution times of DelinquencyPause and GoodwillCreditPaymentAllocation features [fineract]
Posted by "Cocoa-Puffs (via GitHub)" <gi...@apache.org>.
Cocoa-Puffs commented on PR #6497:
URL: https://github.com/apache/fineract/pull/6497#issuecomment-5813229053
All four coverage points are fair, and I've fixed them. The product.name one I'd push back on, reasoning at the bottom.
## UC6.4 — restored both negative cases (the real loss)
Adopted the reviewer's layout: business date 03 Jan, first pause 05–08, backdated pause 01–02, errors expected for 02–03 and 01–04.
That gets the two distinct cases back:
candidate | why it's rejected | case it covers
-- | -- | --
02-03 | shares only day 02 with the backdated pause | partial overlap with only the backdated pause
01-04 | contains 01-02, stops at 04 | full containment, ending before the first pause
The thing that makes this clean rather than ambiguous is worth stating, since the file also has a scenario asserting "touching consecutive pauses are rejected". I checked the validator and overlap needs a shared day, not adjacency.
```java
return !parsedPauseStart.isAfter(existingPauseEnd) && !parsedPauseEnd.isBefore(existingPauseStart);
```
So 01–04 against a pause at 05–08 is not an overlap, and its rejection can only come from containing the backdated pause. (The old scenario was actually weaker here — its 02–14 butted straight against a pause starting 15 Jan, so it had the ambiguity this version doesn't.)
I also moved the schedule assertion from after the first pause to after the backdated one, where it now shows something: period 1 goes 01–03 → 01–05, i.e. base 3 days plus the 2 inclusive pause days.
## UC6.1 / UC8 / UC5
| change | effect on assertions
-- | -- | --
UC6.1 | future pause 14-16 → 13-15 | none — period 3 is 13-18 either way (both are 3 inclusive days). Now starts exactly on period 3's first day, as the old 18 Apr pause did on its 18 Apr - 25 May period.
UC8 | second pause 04-05 → 05-06, business date 04 → 05 | none — period 1 still 01-03 → 01-05 → 01-07. Restores a clear gap day (04) between the two pauses.
UC5 | pause 05-07 → 05-05 | period 2 04-09 → 04-07
UC5 needs a caveat. With 3-day periods, period 2 is 04-06, so the only range strictly inside it is a single day. A one-day pause is legal (validateStartBeforeEnd rejects only start > end) and it does sit in the middle (04 < 05 < 06), so the scenario title still holds, but it is an unusual shape. If you'd rather have a multi-day interior pause, UC5 needs a bucket wider than the 3-day guideline. Happy to give it its own if you prefer that, however I believe that while the shape is not common, it nevertheless accurately tests the overlapping pause validations it's supposed to.
## product.name — I think the linkage is already covered
Real concern, but I'd argue it's checked structurally rather than by name. If WCLP_DELINQUENCY resolved to the wrong product, the next assertion in every affected scenario fails:
| periodNumber | fromDate | toDate |
| -- | -- | -- |
| 1 | 2026-01-01 | 2026-01-03 |
A 01-03 first period can only come from a 3-day bucket. The stock WCLP product gives 01-01 .. 01-30 — which is exactly what the pre-change baseline asserted — and a product with no bucket produces no schedule rows at all. So a mislinked loan can't reach the end of any of these scenarios.
The period boundaries are also a stronger check than the name was, because unlike the name they aren't random. That's the reason the column went: each scenario now builds its own bucket and product, so the name is a fresh DB-WCL-_random_ every run and can't be asserted. This could be done by touching the shared stepdefs and creating a version which stores the created product name in the test context, but I would rather not touch them unless you feel strongly.
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Fix e2e execution times of DelinquencyPause and GoodwillCreditPaymentAllocation features [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #6497:
URL: https://github.com/apache/fineract/pull/6497
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org