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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
