unbridled-41 commented on PR #4873:
URL:
https://github.com/apache/rocketmq-dashboard/pull/4873#issuecomment-5929226691
Thanks for the review — all three blockers are addressed in 406900ee:
1. **Page-2 coverage** — new
`relatedAlertsShouldFetchACandidateThatSitsOnPageTwoTest` stubs page 1 with 100
non-matching rows (total=150) and the matching candidate on page 2, and asserts
`findAlertsPage` is called exactly twice. Against a single-page read this fails
(the candidate never appears), so the paging half is now pinned.
2. **Scan cap** — added `MAX_RELATED_CANDIDATE_PAGES = 5` (500 candidates);
the loop breaks with `log.warn("Related-alert candidate scan for alert {} hit
the {} page cap; candidates beyond page {} were not considered", ...)` when the
cap is reached. New `relatedAlertsShouldStopTheCandidateScanAtThePageCapTest`
pins it: with total=1000 stubbed, exactly 5 `findAlertsPage` calls happen
(uncapped, the loop would walk to page 10 and the assertion fails).
3. **Dedup** — window matches now merge by `fingerprint` keeping the latest
event, mirroring `AlertNotificationSuppressionService`'s `later()` semantics
(same null-safe comparison, as a private `laterEvent` helper). Rows without a
fingerprint cannot collide and keep their own identity. New
`relatedAlertsShouldMergeTheInWindowRowsOfOneIncidentKeepingTheLatestTest`
builds one incident with a FIRING (-5 min) and a REMINDER (-2 min) row sharing
a fingerprint and asserts the panel lists only the REMINDER row. One deliberate
addition: explicit suppression causes are pinned by id and never dropped by the
merge, because they can sit outside the display window and the UI points at
them directly.
`AlertServiceTest` 87/87 (3 new cases), checkstyle clean.
--
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]
