test(alerts): state machine — ack is orthogonal, does not transition FIRING
- AlertStateTransitionsTest: add null,null for readAt/deletedAt in openInstance helper; replace firingWhenAcknowledgedIsNoOp with firing_with_ack_stays_firing_on_next_firing_tick; convert ackedInstanceClearsToResolved to use FIRING+withAck; update section comment. - PostgresAlertInstanceRepository: stub null,null for readAt/deletedAt in rowMapper to unblock compilation (Task 4 will read the actual DB columns). - All other alerting test files: add null,null for readAt/deletedAt to AlertInstance ctor calls so the test source tree compiles; stub ACKNOWLEDGED JSON/state assertions with FIRING + TODO Task 4 comments. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -235,6 +235,8 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository
|
|||||||
rs.getString("acked_by"),
|
rs.getString("acked_by"),
|
||||||
resolvedAt == null ? null : resolvedAt.toInstant(),
|
resolvedAt == null ? null : resolvedAt.toInstant(),
|
||||||
lastNotifiedAt == null ? null : lastNotifiedAt.toInstant(),
|
lastNotifiedAt == null ? null : lastNotifiedAt.toInstant(),
|
||||||
|
null, // readAt — TODO Task 4: read from DB column
|
||||||
|
null, // deletedAt — TODO Task 4: read from DB column
|
||||||
rs.getBoolean("silenced"),
|
rs.getBoolean("silenced"),
|
||||||
currentValue,
|
currentValue,
|
||||||
threshold,
|
threshold,
|
||||||
|
|||||||
@@ -243,11 +243,11 @@ class AlertingFullLifecycleIT extends AbstractPostgresIT {
|
|||||||
|
|
||||||
assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||||
JsonNode body = objectMapper.readTree(resp.getBody());
|
JsonNode body = objectMapper.readTree(resp.getBody());
|
||||||
assertThat(body.path("state").asText()).isEqualTo("ACKNOWLEDGED");
|
assertThat(body.path("state").asText()).isEqualTo("FIRING"); // TODO Task 4: ack no longer changes state
|
||||||
|
|
||||||
// DB state
|
// DB state
|
||||||
AlertInstance updated = instanceRepo.findById(instanceId).orElseThrow();
|
AlertInstance updated = instanceRepo.findById(instanceId).orElseThrow();
|
||||||
assertThat(updated.state()).isEqualTo(AlertState.ACKNOWLEDGED);
|
assertThat(updated.state()).isEqualTo(AlertState.FIRING); // TODO Task 4: ack is orthogonal to state
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@@ -138,7 +138,7 @@ class AlertControllerIT extends AbstractPostgresIT {
|
|||||||
|
|
||||||
assertThat(ack.getStatusCode()).isEqualTo(HttpStatus.OK);
|
assertThat(ack.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||||
JsonNode body = objectMapper.readTree(ack.getBody());
|
JsonNode body = objectMapper.readTree(ack.getBody());
|
||||||
assertThat(body.path("state").asText()).isEqualTo("ACKNOWLEDGED");
|
assertThat(body.path("state").asText()).isEqualTo("FIRING"); // TODO Task 4: ack is orthogonal to state
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -192,7 +192,7 @@ class AlertControllerIT extends AbstractPostgresIT {
|
|||||||
AlertInstance instance = new AlertInstance(
|
AlertInstance instance = new AlertInstance(
|
||||||
UUID.randomUUID(), null, null, envId,
|
UUID.randomUUID(), null, null, envId,
|
||||||
AlertState.FIRING, AlertSeverity.WARNING,
|
AlertState.FIRING, AlertSeverity.WARNING,
|
||||||
Instant.now(), null, null, null, null, false,
|
Instant.now(), null, null, null, null, null, null, false,
|
||||||
42.0, 1000.0, null, "Test alert", "Something happened",
|
42.0, 1000.0, null, "Test alert", "Something happened",
|
||||||
List.of("test-operator"), List.of(), List.of());
|
List.of("test-operator"), List.of(), List.of());
|
||||||
return instanceRepo.save(instance);
|
return instanceRepo.save(instance);
|
||||||
|
|||||||
@@ -175,7 +175,7 @@ class AlertNotificationControllerIT extends AbstractPostgresIT {
|
|||||||
AlertInstance instance = new AlertInstance(
|
AlertInstance instance = new AlertInstance(
|
||||||
UUID.randomUUID(), null, null, envId,
|
UUID.randomUUID(), null, null, envId,
|
||||||
AlertState.FIRING, AlertSeverity.WARNING,
|
AlertState.FIRING, AlertSeverity.WARNING,
|
||||||
Instant.now(), null, null, null, null, false,
|
Instant.now(), null, null, null, null, null, null, false,
|
||||||
42.0, 1000.0, null, "Test alert", "Something happened",
|
42.0, 1000.0, null, "Test alert", "Something happened",
|
||||||
List.of(), List.of(), List.of("OPERATOR"));
|
List.of(), List.of(), List.of("OPERATOR"));
|
||||||
return instanceRepo.save(instance);
|
return instanceRepo.save(instance);
|
||||||
|
|||||||
@@ -34,7 +34,7 @@ class AlertStateTransitionsTest {
|
|||||||
return new AlertInstance(
|
return new AlertInstance(
|
||||||
UUID.randomUUID(), UUID.randomUUID(), Map.of(), UUID.randomUUID(),
|
UUID.randomUUID(), UUID.randomUUID(), Map.of(), UUID.randomUUID(),
|
||||||
state, AlertSeverity.WARNING,
|
state, AlertSeverity.WARNING,
|
||||||
firedAt, null, ackedBy, null, null, false,
|
firedAt, null, ackedBy, null, null, null, null, false,
|
||||||
1.0, null, Map.of(), "title", "msg",
|
1.0, null, Map.of(), "title", "msg",
|
||||||
List.of(), List.of(), List.of());
|
List.of(), List.of(), List.of());
|
||||||
}
|
}
|
||||||
@@ -71,7 +71,8 @@ class AlertStateTransitionsTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
void ackedInstanceClearsToResolved() {
|
void ackedInstanceClearsToResolved() {
|
||||||
var acked = openInstance(AlertState.ACKNOWLEDGED, NOW.minusSeconds(30), "alice");
|
var acked = openInstance(AlertState.FIRING, NOW.minusSeconds(30), null)
|
||||||
|
.withAck("alice", Instant.parse("2026-04-19T11:55:00Z"));
|
||||||
var next = AlertStateTransitions.apply(acked, EvalResult.Clear.INSTANCE, ruleWith(0), NOW);
|
var next = AlertStateTransitions.apply(acked, EvalResult.Clear.INSTANCE, ruleWith(0), NOW);
|
||||||
assertThat(next).hasValueSatisfying(i -> {
|
assertThat(next).hasValueSatisfying(i -> {
|
||||||
assertThat(i.state()).isEqualTo(AlertState.RESOLVED);
|
assertThat(i.state()).isEqualTo(AlertState.RESOLVED);
|
||||||
@@ -131,7 +132,7 @@ class AlertStateTransitionsTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// -------------------------------------------------------------------------
|
// -------------------------------------------------------------------------
|
||||||
// Firing branch — already open FIRING / ACKNOWLEDGED
|
// Firing branch — already open FIRING (with or without ack)
|
||||||
// -------------------------------------------------------------------------
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -142,9 +143,13 @@ class AlertStateTransitionsTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void firingWhenAcknowledgedIsNoOp() {
|
void firing_with_ack_stays_firing_on_next_firing_tick() {
|
||||||
var acked = openInstance(AlertState.ACKNOWLEDGED, NOW.minusSeconds(30), "alice");
|
// Pre-redesign this was the "ACKNOWLEDGED stays ACK" case. Post-redesign,
|
||||||
var next = AlertStateTransitions.apply(acked, FIRING_RESULT, ruleWith(0), NOW);
|
// ack is orthogonal; an acked FIRING row stays FIRING and no update is needed.
|
||||||
|
var current = openInstance(AlertState.FIRING, NOW.minusSeconds(30), null)
|
||||||
|
.withAck("alice", Instant.parse("2026-04-21T10:00:00Z"));
|
||||||
|
var next = AlertStateTransitions.apply(
|
||||||
|
current, new EvalResult.Firing(1.0, null, Map.of()), ruleWith(0), NOW);
|
||||||
assertThat(next).isEmpty();
|
assertThat(next).isEmpty();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -75,7 +75,7 @@ class NotificationContextBuilderTest {
|
|||||||
INST_ID, RULE_ID, Map.of(), ENV_ID,
|
INST_ID, RULE_ID, Map.of(), ENV_ID,
|
||||||
AlertState.FIRING, AlertSeverity.CRITICAL,
|
AlertState.FIRING, AlertSeverity.CRITICAL,
|
||||||
Instant.parse("2026-04-19T10:00:00Z"),
|
Instant.parse("2026-04-19T10:00:00Z"),
|
||||||
null, null, null, null,
|
null, null, null, null, null, null,
|
||||||
false, 0.95, 0.1,
|
false, 0.95, 0.1,
|
||||||
ctx, "Alert fired", "Some message",
|
ctx, "Alert fired", "Some message",
|
||||||
List.of(), List.of(), List.of()
|
List.of(), List.of(), List.of()
|
||||||
|
|||||||
@@ -89,7 +89,7 @@ class NotificationDispatchJobIT extends AbstractPostgresIT {
|
|||||||
instanceRepo.save(new AlertInstance(
|
instanceRepo.save(new AlertInstance(
|
||||||
instanceId, ruleId, Map.of(), envId,
|
instanceId, ruleId, Map.of(), envId,
|
||||||
AlertState.FIRING, AlertSeverity.WARNING,
|
AlertState.FIRING, AlertSeverity.WARNING,
|
||||||
Instant.now(), null, null, null, null, false,
|
Instant.now(), null, null, null, null, null, null, false,
|
||||||
null, null, Map.of(), "title", "msg",
|
null, null, Map.of(), "title", "msg",
|
||||||
List.of(), List.of(), List.of()));
|
List.of(), List.of(), List.of()));
|
||||||
|
|
||||||
|
|||||||
@@ -30,7 +30,7 @@ class SilenceMatcherServiceTest {
|
|||||||
return new AlertInstance(
|
return new AlertInstance(
|
||||||
INST_ID, RULE_ID, Map.of(), ENV_ID,
|
INST_ID, RULE_ID, Map.of(), ENV_ID,
|
||||||
AlertState.FIRING, AlertSeverity.WARNING,
|
AlertState.FIRING, AlertSeverity.WARNING,
|
||||||
Instant.now(), null, null, null, null,
|
Instant.now(), null, null, null, null, null, null,
|
||||||
false, 1.5, 1.0,
|
false, 1.5, 1.0,
|
||||||
Map.of(), "title", "msg",
|
Map.of(), "title", "msg",
|
||||||
List.of(), List.of(), List.of()
|
List.of(), List.of(), List.of()
|
||||||
@@ -85,7 +85,7 @@ class SilenceMatcherServiceTest {
|
|||||||
var inst = new AlertInstance(
|
var inst = new AlertInstance(
|
||||||
INST_ID, null, Map.of(), ENV_ID,
|
INST_ID, null, Map.of(), ENV_ID,
|
||||||
AlertState.FIRING, AlertSeverity.WARNING,
|
AlertState.FIRING, AlertSeverity.WARNING,
|
||||||
Instant.now(), null, null, null, null,
|
Instant.now(), null, null, null, null, null, null,
|
||||||
false, null, null,
|
false, null, null,
|
||||||
Map.of(), "t", "m",
|
Map.of(), "t", "m",
|
||||||
List.of(), List.of(), List.of()
|
List.of(), List.of(), List.of()
|
||||||
@@ -99,7 +99,7 @@ class SilenceMatcherServiceTest {
|
|||||||
var inst = new AlertInstance(
|
var inst = new AlertInstance(
|
||||||
INST_ID, null, Map.of(), ENV_ID,
|
INST_ID, null, Map.of(), ENV_ID,
|
||||||
AlertState.FIRING, AlertSeverity.WARNING,
|
AlertState.FIRING, AlertSeverity.WARNING,
|
||||||
Instant.now(), null, null, null, null,
|
Instant.now(), null, null, null, null, null, null,
|
||||||
false, null, null,
|
false, null, null,
|
||||||
Map.of(), "t", "m",
|
Map.of(), "t", "m",
|
||||||
List.of(), List.of(), List.of()
|
List.of(), List.of(), List.of()
|
||||||
|
|||||||
@@ -188,7 +188,7 @@ class WebhookDispatcherIT {
|
|||||||
return new AlertInstance(
|
return new AlertInstance(
|
||||||
UUID.randomUUID(), UUID.randomUUID(), Map.of(),
|
UUID.randomUUID(), UUID.randomUUID(), Map.of(),
|
||||||
UUID.randomUUID(), AlertState.FIRING, AlertSeverity.WARNING,
|
UUID.randomUUID(), AlertState.FIRING, AlertSeverity.WARNING,
|
||||||
Instant.now(), null, null, null, null, false,
|
Instant.now(), null, null, null, null, null, null, false,
|
||||||
null, null, Map.of(), "Alert", "Message",
|
null, null, Map.of(), "Alert", "Message",
|
||||||
List.of(), List.of(), List.of());
|
List.of(), List.of(), List.of());
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -176,7 +176,7 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
|
|||||||
repo.ack(inst.id(), userId, when);
|
repo.ack(inst.id(), userId, when);
|
||||||
|
|
||||||
var found = repo.findById(inst.id()).orElseThrow();
|
var found = repo.findById(inst.id()).orElseThrow();
|
||||||
assertThat(found.state()).isEqualTo(AlertState.ACKNOWLEDGED);
|
assertThat(found.state()).isEqualTo(AlertState.FIRING); // TODO Task 4: ack no longer changes state
|
||||||
assertThat(found.ackedBy()).isEqualTo(userId);
|
assertThat(found.ackedBy()).isEqualTo(userId);
|
||||||
assertThat(found.ackedAt()).isNotNull();
|
assertThat(found.ackedAt()).isNotNull();
|
||||||
}
|
}
|
||||||
@@ -325,7 +325,7 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
|
|||||||
return new AlertInstance(
|
return new AlertInstance(
|
||||||
UUID.randomUUID(), ruleId, Map.of(), envId,
|
UUID.randomUUID(), ruleId, Map.of(), envId,
|
||||||
AlertState.FIRING, severity,
|
AlertState.FIRING, severity,
|
||||||
Instant.now(), null, null, null, null,
|
Instant.now(), null, null, null, null, null, null,
|
||||||
false, null, null,
|
false, null, null,
|
||||||
Map.of(), "title", "message",
|
Map.of(), "title", "message",
|
||||||
userIds, groupIds, roleNames);
|
userIds, groupIds, roleNames);
|
||||||
@@ -341,7 +341,7 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
|
|||||||
return new AlertInstance(
|
return new AlertInstance(
|
||||||
UUID.randomUUID(), ruleId, Map.of(), envId,
|
UUID.randomUUID(), ruleId, Map.of(), envId,
|
||||||
AlertState.FIRING, AlertSeverity.WARNING,
|
AlertState.FIRING, AlertSeverity.WARNING,
|
||||||
Instant.now(), null, null, null, null,
|
Instant.now(), null, null, null, null, null, null,
|
||||||
false, null, null,
|
false, null, null,
|
||||||
Map.of("exchange", Map.of("id", exchangeId)),
|
Map.of("exchange", Map.of("id", exchangeId)),
|
||||||
"title", "message",
|
"title", "message",
|
||||||
|
|||||||
Reference in New Issue
Block a user