Skip to content

test: Correct the cause recorded for the acknowledged-alert test and drop the redundant action #2316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
eirsep merged 1 commit into from
Oct 30, 2026
Merged

test: Correct the cause recorded for the acknowledged-alert test and drop the redundant action #2316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1523,19 +1523,11 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() {
params.docCount > 0
""".trimIndent()

// The assertions below require lastNotificationTime to advance for a completed alert, and
// BucketLevelMonitorRunner only advances it for alerts a PER_ALERT action ran for: alertsToUpdate is
// populated inside the per-alert action loop. Rewriting the PER_ALERT policies is therefore not enough,
// because every generated action may independently be PER_EXECUTION, leaving nothing to advance the
// timestamp. Prepend one PER_ALERT action carrying COMPLETED, and keep the generated ones for their
// randomness.
val actions = listOf(
randomActionWithPolicy(
actionExecutionPolicy = ActionExecutionPolicy(
PerAlertActionScope(setOf(AlertCategory.NEW, AlertCategory.DEDUPED, AlertCategory.COMPLETED))
)
)
) + randomActionsForBucketLevelTrigger(min = 1).map {
// The rewrite below has to leave a non-empty list. randomBucketLevelTrigger replaces an empty
// actions list with freshly generated ones, and those never pass through this rewrite, so a
// PER_ALERT policy without COMPLETED can reach the trigger and leave the completed alert's
// lastNotificationTime untouched.
Comment on lines +1526 to +1529

@lezzago lezzago Oct 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Regenerated actions can also lack DEDUPED (e.g. PerAlertActionScope{NEW}), and then the test fails earlier, at "Active alert was not updated", rather than at the completed-alert check. Maybe: "...so a PER_ALERT policy missing DEDUPED or COMPLETED can reach the trigger and leave lastNotificationTime untouched."

val actions = randomActionsForBucketLevelTrigger(min = 1).map {

@lezzago lezzago Oct 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: The non-empty guarantee the comment above depends on lives in the helper's maxOf(min, 1), which the description suggests revisiting. An assertTrue(actions.isNotEmpty()) after building actions would make this test fail clearly instead of flaking again if that helper changes.

if (it.actionExecutionPolicy?.actionExecutionScope is PerAlertActionScope) {
it.copy(
actionExecutionPolicy = ActionExecutionPolicy(
Expand Down
4 changes: 3 additions & 1 deletion alerting/src/test/kotlin/org/opensearch/alerting/TestHelpers.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,9 @@ fun randomBucketLevelTrigger(
/**
* [min] is the fewest actions to generate, not the start of the range the count is drawn from:
* `(min..randomInt(max))` is empty whenever `randomInt` returns a value below [min], so asking for a
* minimum of one silently produced no actions about a tenth of the time.
* minimum of one silently produced no actions about a tenth of the time. Callers that post-process the
* result cannot treat an empty list as harmless, because [randomBucketLevelTrigger] replaces one with
* freshly generated actions that skip whatever the caller applied.
*/
fun randomActionsForBucketLevelTrigger(min: Int = 0, max: Int = 10, destinationId: String = ""): List<Action> =
(1..randomIntBetween(maxOf(min, 1), max)).map { randomActionWithPolicy(destinationId = destinationId) }
Expand Down
Loading