RockteMQ-AI commented on code in PR #3579:
URL:
https://github.com/apache/rocketmq-dashboard/pull/3579#discussion_r3941671062
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/NativeAlertRulePolicyTest.java:
##########
@@ -20,104 +20,153 @@
import org.junit.jupiter.api.Test;
import java.util.List;
-import java.util.Locale;
-import static org.assertj.core.api.Assertions.assertThatCode;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class NativeAlertRulePolicyTest {
Review Comment:
The PR description says this adds the 'first dedicated unit test suite', but
the diff replaces an existing 12-test AssertJ suite with a 10-test JUnit
Jupiter suite — net branch coverage decreases (see findings above). The rewrite
also drops the legacy Prometheus-metric compatibility test
(rocketmq_consumer_lag_messages), though unknownMetricSkipsNativeChecks does
exercise the same metricDomain == null early return, so that is mainly a loss
of documented intent. Please update the description and prefer extending the
existing suite over rewriting it, including keeping the AssertJ style used by
the file.
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/NativeAlertRulePolicyTest.java:
##########
@@ -20,104 +20,153 @@
import org.junit.jupiter.api.Test;
import java.util.List;
-import java.util.Locale;
-import static org.assertj.core.api.Assertions.assertThatCode;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class NativeAlertRulePolicyTest {
- @Test
- void acceptsScopedBusinessRuleTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "consumer.lag.total")
-
.instanceId("local").consumerGroup("orders").consecutiveSamples(2).build()))
- .doesNotThrowAnyException();
+ private AlertRuleVO validNativeRule() {
+ return AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.availability")
+ .operator("UNAVAILABLE")
+ .threshold(0)
+ .duration("5m")
+ .instanceId("inst-1")
+ .channels(List.of("email"))
+ .build();
}
- @Test
- void acceptsConsumerDelayForAConsumerGroupTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
"consumer.delay.seconds")
-
.instanceId("local").consumerGroup("orders").build())).doesNotThrowAnyException();
+ private BusinessException rejectionOf(AlertRuleVO rule) {
+ return assertThrows(BusinessException.class, () ->
NativeAlertRulePolicy.validate(rule));
}
@Test
- void acceptsTopicSelectorOnlyForTopicBacklogTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "topic.backlog.total")
-
.instanceId("local").consumerGroup("orders").topic("orders-topic").build()))
- .doesNotThrowAnyException();
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "consumer.lag.total")
- .instanceId("local").topic("orders-topic").build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("topic is not
supported");
+ void acceptsValidNativeAvailabilityRule() {
+ assertDoesNotThrow(() ->
NativeAlertRulePolicy.validate(validNativeRule()));
}
@Test
- void rejectsNativeRuleWithoutInstanceScopeTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "broker.availability")
-
.build())).isInstanceOf(BusinessException.class).hasMessageContaining("instanceId");
+ void unknownMetricSkipsNativeChecks() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.BUSINESS)
+ .metric("custom.app.latency")
+ .operator(">")
+ .threshold(200)
+ .duration("5m")
+ .channels(List.of("email"))
+ .build();
+
+ assertDoesNotThrow(() -> NativeAlertRulePolicy.validate(rule));
}
@Test
- void rejectsNativeMetricInWrongDomainTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "consumer.lag.total")
-
.instanceId("local").build())).isInstanceOf(BusinessException.class).hasMessageContaining("BUSINESS");
+ void rejectsUnsupportedChannel() {
+ AlertRuleVO rule = validNativeRule();
+ rule.setChannels(List.of("slack"));
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertEquals(400, ex.getCode());
+ assertTrue(ex.getMessage().contains("Unsupported notification
channel"));
}
@Test
- void acceptsProxyAvailabilityAsAClusterMetricTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "proxy.availability")
- .instanceId("local").build())).doesNotThrowAnyException();
+ void rejectsDomainMismatchForNativeMetric() {
+ AlertRuleVO rule = validNativeRule();
+ rule.setDomain(AlertDomain.BUSINESS);
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("belongs to the CLUSTER alert
domain"));
}
@Test
- void acceptsExplicitUnavailableAvailabilityRuleTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "broker.availability")
-
.instanceId("local").operator("UNAVAILABLE").build())).doesNotThrowAnyException();
+ void rejectsMissingInstanceIdForNativeMetric() {
+ AlertRuleVO rule = validNativeRule();
+ rule.setInstanceId(null);
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("instanceId is required for native
alert rules"));
}
@Test
- void rejectsUnavailableForNonAvailabilityMetricTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER,
-
"broker.disk.usage_ratio").instanceId("local").operator("UNAVAILABLE").build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("only supported");
+ void rejectsUnavailableOperatorOnNonAvailabilityMetric() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.disk.usage_ratio")
+ .operator("UNAVAILABLE")
+ .threshold(0)
+ .duration("5m")
+ .instanceId("inst-1")
+ .build();
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("UNAVAILABLE is only supported for
native availability metrics"));
}
@Test
- void leavesLegacyPrometheusRulesCompatibleTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
-
"rocketmq_consumer_lag_messages").build())).doesNotThrowAnyException();
+ void rejectsConsumerGroupOnNonGroupScopedMetric() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.disk.usage_ratio")
+ .operator(">")
+ .threshold(0.8)
+ .duration("5m")
+ .instanceId("inst-1")
+ .consumerGroup("cg-1")
+ .build();
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("consumerGroup is not supported
for metric"));
}
@Test
- void rejectsUnsupportedNotificationChannelsOutsideTheHttpApiTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
-
"rocketmq_consumer_lag_messages").channels(List.of("webhook")).build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("Unsupported
notification channel");
+ void rejectsTopicOnNonTopicScopedMetric() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.disk.usage_ratio")
+ .operator(">")
+ .threshold(0.8)
+ .duration("5m")
+ .instanceId("inst-1")
+ .topic("order-topic")
+ .build();
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("topic is not supported for
metric"));
}
@Test
- void acceptsNotificationChannelsIndependentlyOfTheDefaultLocaleTest() {
- Locale previous = Locale.getDefault();
- try {
- Locale.setDefault(Locale.forLanguageTag("tr-TR"));
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
- "rocketmq_consumer_lag_messages").channels(List.of("
DINGTALK ")).build()))
- .doesNotThrowAnyException();
- } finally {
- Locale.setDefault(previous);
- }
+ void acceptsConsumerLagRuleWithGroupScopedChecks() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.BUSINESS)
+ .metric("consumer.lag.total")
+ .operator(">")
+ .threshold(1000)
+ .duration("5m")
+ .instanceId("inst-1")
+ .consumerGroup("cg-1")
+ .build();
+
+ assertDoesNotThrow(() -> NativeAlertRulePolicy.validate(rule));
}
@Test
- void rejectsOverflowingNativeRuleDurationsBeforePersistenceTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "broker.availability")
- .instanceId("local").duration("9223372036854775807y").build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("Invalid alert
duration");
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "broker.availability")
-
.instanceId("local").reminderInterval("9223372036854775807y").build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("Invalid alert
duration");
- }
+ void rejectsMalformedDuration() {
+ AlertRuleVO rule = validNativeRule();
Review Comment:
Coverage regression: the deleted
rejectsOverflowingNativeRuleDurationsBeforePersistenceTest exercised the
ArithmeticException overflow guard in AlertRuleDuration.parse
(Math.multiplyExact for 'w'/'y' units, AlertRuleDuration.java:64, caught at
:48) via duration "9223372036854775807y". The new rejectsMalformedDuration uses
"5x", which only hits the regex/no-match rejection path
(AlertRuleDuration.java:44) — a different branch. Please restore the overflow
case so the multiplyExact guard stays covered.
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/NativeAlertRulePolicyTest.java:
##########
@@ -20,104 +20,153 @@
import org.junit.jupiter.api.Test;
import java.util.List;
-import java.util.Locale;
-import static org.assertj.core.api.Assertions.assertThatCode;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class NativeAlertRulePolicyTest {
- @Test
- void acceptsScopedBusinessRuleTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "consumer.lag.total")
-
.instanceId("local").consumerGroup("orders").consecutiveSamples(2).build()))
- .doesNotThrowAnyException();
+ private AlertRuleVO validNativeRule() {
+ return AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.availability")
+ .operator("UNAVAILABLE")
+ .threshold(0)
+ .duration("5m")
+ .instanceId("inst-1")
+ .channels(List.of("email"))
+ .build();
}
- @Test
- void acceptsConsumerDelayForAConsumerGroupTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
"consumer.delay.seconds")
-
.instanceId("local").consumerGroup("orders").build())).doesNotThrowAnyException();
+ private BusinessException rejectionOf(AlertRuleVO rule) {
+ return assertThrows(BusinessException.class, () ->
NativeAlertRulePolicy.validate(rule));
}
@Test
- void acceptsTopicSelectorOnlyForTopicBacklogTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "topic.backlog.total")
-
.instanceId("local").consumerGroup("orders").topic("orders-topic").build()))
- .doesNotThrowAnyException();
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "consumer.lag.total")
- .instanceId("local").topic("orders-topic").build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("topic is not
supported");
+ void acceptsValidNativeAvailabilityRule() {
+ assertDoesNotThrow(() ->
NativeAlertRulePolicy.validate(validNativeRule()));
}
@Test
- void rejectsNativeRuleWithoutInstanceScopeTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "broker.availability")
-
.build())).isInstanceOf(BusinessException.class).hasMessageContaining("instanceId");
+ void unknownMetricSkipsNativeChecks() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.BUSINESS)
+ .metric("custom.app.latency")
+ .operator(">")
+ .threshold(200)
+ .duration("5m")
+ .channels(List.of("email"))
+ .build();
+
+ assertDoesNotThrow(() -> NativeAlertRulePolicy.validate(rule));
}
@Test
- void rejectsNativeMetricInWrongDomainTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "consumer.lag.total")
-
.instanceId("local").build())).isInstanceOf(BusinessException.class).hasMessageContaining("BUSINESS");
+ void rejectsUnsupportedChannel() {
Review Comment:
Regression test removed:
acceptsNotificationChannelsIndependentlyOfTheDefaultLocaleTest guarded the
channel whitelist against locale-sensitive lowercasing by validating " DINGTALK
" under Locale tr-TR. validateChannels relies on toLowerCase(Locale.ROOT)
(NativeAlertRulePolicy.java:90); if that ever regresses to default-locale
toLowerCase(), Turkish locales map I to dotless ı and channel matching breaks.
rejectsUnsupportedChannel("slack") cannot catch that. Please keep the tr-TR
happy-path test.
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/NativeAlertRulePolicyTest.java:
##########
@@ -20,104 +20,153 @@
import org.junit.jupiter.api.Test;
import java.util.List;
-import java.util.Locale;
-import static org.assertj.core.api.Assertions.assertThatCode;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class NativeAlertRulePolicyTest {
- @Test
- void acceptsScopedBusinessRuleTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "consumer.lag.total")
-
.instanceId("local").consumerGroup("orders").consecutiveSamples(2).build()))
- .doesNotThrowAnyException();
+ private AlertRuleVO validNativeRule() {
+ return AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.availability")
+ .operator("UNAVAILABLE")
+ .threshold(0)
+ .duration("5m")
+ .instanceId("inst-1")
+ .channels(List.of("email"))
+ .build();
}
- @Test
- void acceptsConsumerDelayForAConsumerGroupTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
"consumer.delay.seconds")
-
.instanceId("local").consumerGroup("orders").build())).doesNotThrowAnyException();
+ private BusinessException rejectionOf(AlertRuleVO rule) {
+ return assertThrows(BusinessException.class, () ->
NativeAlertRulePolicy.validate(rule));
}
@Test
- void acceptsTopicSelectorOnlyForTopicBacklogTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "topic.backlog.total")
-
.instanceId("local").consumerGroup("orders").topic("orders-topic").build()))
- .doesNotThrowAnyException();
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "consumer.lag.total")
- .instanceId("local").topic("orders-topic").build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("topic is not
supported");
+ void acceptsValidNativeAvailabilityRule() {
+ assertDoesNotThrow(() ->
NativeAlertRulePolicy.validate(validNativeRule()));
}
@Test
- void rejectsNativeRuleWithoutInstanceScopeTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "broker.availability")
-
.build())).isInstanceOf(BusinessException.class).hasMessageContaining("instanceId");
+ void unknownMetricSkipsNativeChecks() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.BUSINESS)
+ .metric("custom.app.latency")
+ .operator(">")
+ .threshold(200)
+ .duration("5m")
+ .channels(List.of("email"))
+ .build();
+
+ assertDoesNotThrow(() -> NativeAlertRulePolicy.validate(rule));
}
@Test
- void rejectsNativeMetricInWrongDomainTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "consumer.lag.total")
-
.instanceId("local").build())).isInstanceOf(BusinessException.class).hasMessageContaining("BUSINESS");
+ void rejectsUnsupportedChannel() {
+ AlertRuleVO rule = validNativeRule();
+ rule.setChannels(List.of("slack"));
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertEquals(400, ex.getCode());
+ assertTrue(ex.getMessage().contains("Unsupported notification
channel"));
}
@Test
- void acceptsProxyAvailabilityAsAClusterMetricTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "proxy.availability")
- .instanceId("local").build())).doesNotThrowAnyException();
+ void rejectsDomainMismatchForNativeMetric() {
+ AlertRuleVO rule = validNativeRule();
+ rule.setDomain(AlertDomain.BUSINESS);
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("belongs to the CLUSTER alert
domain"));
}
@Test
- void acceptsExplicitUnavailableAvailabilityRuleTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "broker.availability")
-
.instanceId("local").operator("UNAVAILABLE").build())).doesNotThrowAnyException();
+ void rejectsMissingInstanceIdForNativeMetric() {
+ AlertRuleVO rule = validNativeRule();
+ rule.setInstanceId(null);
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("instanceId is required for native
alert rules"));
}
@Test
- void rejectsUnavailableForNonAvailabilityMetricTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER,
-
"broker.disk.usage_ratio").instanceId("local").operator("UNAVAILABLE").build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("only supported");
+ void rejectsUnavailableOperatorOnNonAvailabilityMetric() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.disk.usage_ratio")
+ .operator("UNAVAILABLE")
+ .threshold(0)
+ .duration("5m")
+ .instanceId("inst-1")
+ .build();
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("UNAVAILABLE is only supported for
native availability metrics"));
}
@Test
- void leavesLegacyPrometheusRulesCompatibleTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
-
"rocketmq_consumer_lag_messages").build())).doesNotThrowAnyException();
+ void rejectsConsumerGroupOnNonGroupScopedMetric() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.disk.usage_ratio")
+ .operator(">")
+ .threshold(0.8)
+ .duration("5m")
+ .instanceId("inst-1")
+ .consumerGroup("cg-1")
+ .build();
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("consumerGroup is not supported
for metric"));
}
@Test
- void rejectsUnsupportedNotificationChannelsOutsideTheHttpApiTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
-
"rocketmq_consumer_lag_messages").channels(List.of("webhook")).build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("Unsupported
notification channel");
+ void rejectsTopicOnNonTopicScopedMetric() {
+ AlertRuleVO rule = AlertRuleVO.builder()
Review Comment:
The topic-scoped happy path was dropped: the old suite verified
topic.backlog.total with a topic selector is accepted (TOPIC_SCOPED_METRICS,
NativeAlertRulePolicy.java:74). Only the rejection side (topic on
broker.disk.usage_ratio) remains. Consider adding a positive topic-scoped rule
test next to this one.
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/NativeAlertRulePolicyTest.java:
##########
@@ -20,104 +20,153 @@
import org.junit.jupiter.api.Test;
import java.util.List;
-import java.util.Locale;
-import static org.assertj.core.api.Assertions.assertThatCode;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class NativeAlertRulePolicyTest {
- @Test
- void acceptsScopedBusinessRuleTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "consumer.lag.total")
-
.instanceId("local").consumerGroup("orders").consecutiveSamples(2).build()))
- .doesNotThrowAnyException();
+ private AlertRuleVO validNativeRule() {
+ return AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.availability")
+ .operator("UNAVAILABLE")
+ .threshold(0)
+ .duration("5m")
+ .instanceId("inst-1")
+ .channels(List.of("email"))
+ .build();
}
- @Test
- void acceptsConsumerDelayForAConsumerGroupTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
"consumer.delay.seconds")
-
.instanceId("local").consumerGroup("orders").build())).doesNotThrowAnyException();
+ private BusinessException rejectionOf(AlertRuleVO rule) {
+ return assertThrows(BusinessException.class, () ->
NativeAlertRulePolicy.validate(rule));
}
@Test
- void acceptsTopicSelectorOnlyForTopicBacklogTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "topic.backlog.total")
-
.instanceId("local").consumerGroup("orders").topic("orders-topic").build()))
- .doesNotThrowAnyException();
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS, "consumer.lag.total")
- .instanceId("local").topic("orders-topic").build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("topic is not
supported");
+ void acceptsValidNativeAvailabilityRule() {
+ assertDoesNotThrow(() ->
NativeAlertRulePolicy.validate(validNativeRule()));
}
@Test
- void rejectsNativeRuleWithoutInstanceScopeTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "broker.availability")
-
.build())).isInstanceOf(BusinessException.class).hasMessageContaining("instanceId");
+ void unknownMetricSkipsNativeChecks() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.BUSINESS)
+ .metric("custom.app.latency")
+ .operator(">")
+ .threshold(200)
+ .duration("5m")
+ .channels(List.of("email"))
+ .build();
+
+ assertDoesNotThrow(() -> NativeAlertRulePolicy.validate(rule));
}
@Test
- void rejectsNativeMetricInWrongDomainTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "consumer.lag.total")
-
.instanceId("local").build())).isInstanceOf(BusinessException.class).hasMessageContaining("BUSINESS");
+ void rejectsUnsupportedChannel() {
+ AlertRuleVO rule = validNativeRule();
+ rule.setChannels(List.of("slack"));
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertEquals(400, ex.getCode());
+ assertTrue(ex.getMessage().contains("Unsupported notification
channel"));
}
@Test
- void acceptsProxyAvailabilityAsAClusterMetricTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "proxy.availability")
- .instanceId("local").build())).doesNotThrowAnyException();
+ void rejectsDomainMismatchForNativeMetric() {
+ AlertRuleVO rule = validNativeRule();
+ rule.setDomain(AlertDomain.BUSINESS);
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("belongs to the CLUSTER alert
domain"));
}
@Test
- void acceptsExplicitUnavailableAvailabilityRuleTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "broker.availability")
-
.instanceId("local").operator("UNAVAILABLE").build())).doesNotThrowAnyException();
+ void rejectsMissingInstanceIdForNativeMetric() {
+ AlertRuleVO rule = validNativeRule();
+ rule.setInstanceId(null);
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("instanceId is required for native
alert rules"));
}
@Test
- void rejectsUnavailableForNonAvailabilityMetricTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER,
-
"broker.disk.usage_ratio").instanceId("local").operator("UNAVAILABLE").build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("only supported");
+ void rejectsUnavailableOperatorOnNonAvailabilityMetric() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.disk.usage_ratio")
+ .operator("UNAVAILABLE")
+ .threshold(0)
+ .duration("5m")
+ .instanceId("inst-1")
+ .build();
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("UNAVAILABLE is only supported for
native availability metrics"));
}
@Test
- void leavesLegacyPrometheusRulesCompatibleTest() {
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
-
"rocketmq_consumer_lag_messages").build())).doesNotThrowAnyException();
+ void rejectsConsumerGroupOnNonGroupScopedMetric() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.disk.usage_ratio")
+ .operator(">")
+ .threshold(0.8)
+ .duration("5m")
+ .instanceId("inst-1")
+ .consumerGroup("cg-1")
+ .build();
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("consumerGroup is not supported
for metric"));
}
@Test
- void rejectsUnsupportedNotificationChannelsOutsideTheHttpApiTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
-
"rocketmq_consumer_lag_messages").channels(List.of("webhook")).build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("Unsupported
notification channel");
+ void rejectsTopicOnNonTopicScopedMetric() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.CLUSTER)
+ .metric("broker.disk.usage_ratio")
+ .operator(">")
+ .threshold(0.8)
+ .duration("5m")
+ .instanceId("inst-1")
+ .topic("order-topic")
+ .build();
+
+ BusinessException ex = rejectionOf(rule);
+
+ assertTrue(ex.getMessage().contains("topic is not supported for
metric"));
}
@Test
- void acceptsNotificationChannelsIndependentlyOfTheDefaultLocaleTest() {
- Locale previous = Locale.getDefault();
- try {
- Locale.setDefault(Locale.forLanguageTag("tr-TR"));
- assertThatCode(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.BUSINESS,
- "rocketmq_consumer_lag_messages").channels(List.of("
DINGTALK ")).build()))
- .doesNotThrowAnyException();
- } finally {
- Locale.setDefault(previous);
- }
+ void acceptsConsumerLagRuleWithGroupScopedChecks() {
+ AlertRuleVO rule = AlertRuleVO.builder()
+ .domain(AlertDomain.BUSINESS)
+ .metric("consumer.lag.total")
+ .operator(">")
+ .threshold(1000)
+ .duration("5m")
+ .instanceId("inst-1")
+ .consumerGroup("cg-1")
+ .build();
+
+ assertDoesNotThrow(() -> NativeAlertRulePolicy.validate(rule));
}
@Test
- void rejectsOverflowingNativeRuleDurationsBeforePersistenceTest() {
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "broker.availability")
- .instanceId("local").duration("9223372036854775807y").build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("Invalid alert
duration");
- assertThatThrownBy(() ->
NativeAlertRulePolicy.validate(rule(AlertDomain.CLUSTER, "broker.availability")
-
.instanceId("local").reminderInterval("9223372036854775807y").build()))
-
.isInstanceOf(BusinessException.class).hasMessageContaining("Invalid alert
duration");
- }
+ void rejectsMalformedDuration() {
+ AlertRuleVO rule = validNativeRule();
Review Comment:
Coverage regression: no remaining test sets reminderInterval, so the parse
call at NativeAlertRulePolicy.java:78 is now uncovered. The old suite validated
reminderInterval("9223372036854775807y") alongside duration. Please add a
reminderInterval rejection case (e.g. invalid or overflowing value).
--
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]
