RockteMQ-AI commented on code in PR #3578:
URL:
https://github.com/apache/rocketmq-dashboard/pull/3578#discussion_r3941684373
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleEvaluatorTest.java:
##########
@@ -21,71 +21,142 @@
import org.junit.jupiter.api.Test;
import java.time.Instant;
+import java.util.Map;
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class AlertRuleEvaluatorTest {
+
private final AlertRuleEvaluator evaluator = new AlertRuleEvaluator();
+ private AlertRuleVO rule(String metric, AlertDomain domain, String
operator, double threshold) {
+ return AlertRuleVO.builder()
+ .domain(domain)
+ .metric(metric)
+ .operator(operator)
+ .threshold(threshold)
+ .duration("5m")
+ .build();
+ }
+
+ private MetricSample sample(String metricKey, AlertDomain domain, double
value) {
+ return new MetricSample(metricKey, domain, "inst-1", "cluster-1",
Map.of(),
+ value, MetricAvailability.AVAILABLE,
Instant.parse("2026-09-01T08:00:00Z"));
+ }
+
+ private MetricSample unavailableSample(String metricKey, AlertDomain
domain) {
+ return new MetricSample(metricKey, domain, "inst-1", "cluster-1",
Map.of(),
+ null, MetricAvailability.UNAVAILABLE,
Instant.parse("2026-09-01T08:00:00Z"));
+ }
+
@Test
- void triggersMatchingAvailableMetricTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
- .operator(">=").threshold(0.85).enabled(true).build();
- MetricSample sample = sample(MetricAvailability.AVAILABLE, 0.9);
+ void mismatchingMetricDoesNotMatch() {
+ AlertEvaluationResult result = evaluator.evaluate(
+ rule("broker.disk.usage_ratio", AlertDomain.CLUSTER, ">", 0.8),
+ sample("broker.cpu.usage", AlertDomain.CLUSTER, 0.9));
- AlertEvaluationResult result = evaluator.evaluate(rule, sample);
+ assertFalse(result.matches());
+ assertEquals(MetricAvailability.AVAILABLE, result.availability());
+ }
+
+ @Test
+ void mismatchingDomainDoesNotMatch() {
+ AlertEvaluationResult result = evaluator.evaluate(
+ rule("consumer.lag.total", AlertDomain.BUSINESS, ">", 100),
+ sample("consumer.lag.total", AlertDomain.CLUSTER, 500));
- assertThat(result.matches()).isTrue();
- assertThat(result.conditionMet()).isTrue();
- assertThat(result.currentValue()).isEqualTo(0.9);
+ assertFalse(result.matches());
}
@Test
- void evaluatesPercentageThresholdsForNativeRatioMetricsTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
-
.operator(">=").threshold(85).thresholdUnit("%").enabled(true).build();
+ void unavailableSampleOnlyMatchesUnavailableOperator() {
+ AlertEvaluationResult matched = evaluator.evaluate(
+ rule("broker.availability", AlertDomain.CLUSTER,
"UNAVAILABLE", 0),
+ unavailableSample("broker.availability", AlertDomain.CLUSTER));
- AlertEvaluationResult result = evaluator.evaluate(rule,
sample(MetricAvailability.AVAILABLE, 0.9));
+ assertTrue(matched.matches());
+ assertTrue(matched.conditionMet());
+ assertNull(matched.currentValue());
Review Comment:
currentValue() propagation for AVAILABLE samples is no longer asserted
anywhere. The deleted triggersMatchingAvailableMetricTest asserted
currentValue() == 0.9; the only remaining currentValue assertion is assertNull
for the unavailable case. If AlertRuleEvaluator stopped returning the sample
value as currentValue (AlertRuleEvaluator.java:40-42), every test still passes
and alert notifications lose their 'current value' payload. Consider asserting
currentValue in greaterThanComparesAgainstThreshold (e.g. is 0.9 / 0.7 for the
two evaluations).
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleEvaluatorTest.java:
##########
@@ -21,71 +21,142 @@
import org.junit.jupiter.api.Test;
import java.time.Instant;
+import java.util.Map;
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class AlertRuleEvaluatorTest {
+
private final AlertRuleEvaluator evaluator = new AlertRuleEvaluator();
+ private AlertRuleVO rule(String metric, AlertDomain domain, String
operator, double threshold) {
+ return AlertRuleVO.builder()
+ .domain(domain)
+ .metric(metric)
+ .operator(operator)
+ .threshold(threshold)
+ .duration("5m")
+ .build();
+ }
+
+ private MetricSample sample(String metricKey, AlertDomain domain, double
value) {
+ return new MetricSample(metricKey, domain, "inst-1", "cluster-1",
Map.of(),
+ value, MetricAvailability.AVAILABLE,
Instant.parse("2026-09-01T08:00:00Z"));
+ }
+
+ private MetricSample unavailableSample(String metricKey, AlertDomain
domain) {
+ return new MetricSample(metricKey, domain, "inst-1", "cluster-1",
Map.of(),
+ null, MetricAvailability.UNAVAILABLE,
Instant.parse("2026-09-01T08:00:00Z"));
+ }
+
@Test
- void triggersMatchingAvailableMetricTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
- .operator(">=").threshold(0.85).enabled(true).build();
- MetricSample sample = sample(MetricAvailability.AVAILABLE, 0.9);
+ void mismatchingMetricDoesNotMatch() {
+ AlertEvaluationResult result = evaluator.evaluate(
+ rule("broker.disk.usage_ratio", AlertDomain.CLUSTER, ">", 0.8),
+ sample("broker.cpu.usage", AlertDomain.CLUSTER, 0.9));
- AlertEvaluationResult result = evaluator.evaluate(rule, sample);
+ assertFalse(result.matches());
+ assertEquals(MetricAvailability.AVAILABLE, result.availability());
+ }
+
+ @Test
+ void mismatchingDomainDoesNotMatch() {
+ AlertEvaluationResult result = evaluator.evaluate(
+ rule("consumer.lag.total", AlertDomain.BUSINESS, ">", 100),
+ sample("consumer.lag.total", AlertDomain.CLUSTER, 500));
- assertThat(result.matches()).isTrue();
- assertThat(result.conditionMet()).isTrue();
- assertThat(result.currentValue()).isEqualTo(0.9);
+ assertFalse(result.matches());
}
@Test
- void evaluatesPercentageThresholdsForNativeRatioMetricsTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
-
.operator(">=").threshold(85).thresholdUnit("%").enabled(true).build();
+ void unavailableSampleOnlyMatchesUnavailableOperator() {
+ AlertEvaluationResult matched = evaluator.evaluate(
+ rule("broker.availability", AlertDomain.CLUSTER,
"UNAVAILABLE", 0),
+ unavailableSample("broker.availability", AlertDomain.CLUSTER));
- AlertEvaluationResult result = evaluator.evaluate(rule,
sample(MetricAvailability.AVAILABLE, 0.9));
+ assertTrue(matched.matches());
+ assertTrue(matched.conditionMet());
+ assertNull(matched.currentValue());
- assertThat(result.conditionMet()).isTrue();
+ AlertEvaluationResult notMatched = evaluator.evaluate(
+ rule("broker.availability", AlertDomain.CLUSTER, ">", 0),
+ unavailableSample("broker.availability", AlertDomain.CLUSTER));
+
+ assertTrue(notMatched.matches());
+ assertFalse(notMatched.conditionMet());
}
@Test
- void unavailableMetricDoesNotBehaveAsZeroTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
- .operator("<").threshold(0.1).enabled(true).build();
+ void greaterThanComparesAgainstThreshold() {
+ AlertRuleVO rule = rule("broker.disk.usage_ratio",
AlertDomain.CLUSTER, ">", 0.8);
Review Comment:
Coverage regression: the deleted
evaluatesPercentageThresholdsForNativeRatioMetricsTest was the only test
verifying that evaluate() applies
AlertRuleSemanticFingerprint.normalizedThreshold() (AlertRuleEvaluator.java:41)
before comparing. AlertRuleSemanticFingerprintTest only checks fingerprint
identity, not evaluation. After this PR, replacing normalizedThreshold(rule)
with rule.getThreshold() would pass the entire suite, and a rule with threshold
85 + thresholdUnit "%" on broker.disk.usage_ratio would compare 85 against a
0.9 ratio and never fire. Please re-add a test like: rule(metric, domain, ">=",
85) with .thresholdUnit("%") against a 0.9 sample asserting conditionMet() —
the PR description itself says the tests pin comparison 'against the normalized
threshold', so this path should be pinned.
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleEvaluatorTest.java:
##########
@@ -21,71 +21,142 @@
import org.junit.jupiter.api.Test;
import java.time.Instant;
+import java.util.Map;
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class AlertRuleEvaluatorTest {
+
private final AlertRuleEvaluator evaluator = new AlertRuleEvaluator();
+ private AlertRuleVO rule(String metric, AlertDomain domain, String
operator, double threshold) {
+ return AlertRuleVO.builder()
+ .domain(domain)
+ .metric(metric)
+ .operator(operator)
+ .threshold(threshold)
+ .duration("5m")
+ .build();
+ }
+
+ private MetricSample sample(String metricKey, AlertDomain domain, double
value) {
+ return new MetricSample(metricKey, domain, "inst-1", "cluster-1",
Map.of(),
+ value, MetricAvailability.AVAILABLE,
Instant.parse("2026-09-01T08:00:00Z"));
+ }
+
+ private MetricSample unavailableSample(String metricKey, AlertDomain
domain) {
+ return new MetricSample(metricKey, domain, "inst-1", "cluster-1",
Map.of(),
+ null, MetricAvailability.UNAVAILABLE,
Instant.parse("2026-09-01T08:00:00Z"));
+ }
+
@Test
- void triggersMatchingAvailableMetricTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
- .operator(">=").threshold(0.85).enabled(true).build();
- MetricSample sample = sample(MetricAvailability.AVAILABLE, 0.9);
+ void mismatchingMetricDoesNotMatch() {
+ AlertEvaluationResult result = evaluator.evaluate(
+ rule("broker.disk.usage_ratio", AlertDomain.CLUSTER, ">", 0.8),
+ sample("broker.cpu.usage", AlertDomain.CLUSTER, 0.9));
- AlertEvaluationResult result = evaluator.evaluate(rule, sample);
+ assertFalse(result.matches());
+ assertEquals(MetricAvailability.AVAILABLE, result.availability());
+ }
+
+ @Test
+ void mismatchingDomainDoesNotMatch() {
+ AlertEvaluationResult result = evaluator.evaluate(
+ rule("consumer.lag.total", AlertDomain.BUSINESS, ">", 100),
+ sample("consumer.lag.total", AlertDomain.CLUSTER, 500));
- assertThat(result.matches()).isTrue();
- assertThat(result.conditionMet()).isTrue();
- assertThat(result.currentValue()).isEqualTo(0.9);
+ assertFalse(result.matches());
}
@Test
- void evaluatesPercentageThresholdsForNativeRatioMetricsTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
-
.operator(">=").threshold(85).thresholdUnit("%").enabled(true).build();
+ void unavailableSampleOnlyMatchesUnavailableOperator() {
+ AlertEvaluationResult matched = evaluator.evaluate(
+ rule("broker.availability", AlertDomain.CLUSTER,
"UNAVAILABLE", 0),
+ unavailableSample("broker.availability", AlertDomain.CLUSTER));
- AlertEvaluationResult result = evaluator.evaluate(rule,
sample(MetricAvailability.AVAILABLE, 0.9));
+ assertTrue(matched.matches());
+ assertTrue(matched.conditionMet());
+ assertNull(matched.currentValue());
- assertThat(result.conditionMet()).isTrue();
+ AlertEvaluationResult notMatched = evaluator.evaluate(
+ rule("broker.availability", AlertDomain.CLUSTER, ">", 0),
+ unavailableSample("broker.availability", AlertDomain.CLUSTER));
+
+ assertTrue(notMatched.matches());
+ assertFalse(notMatched.conditionMet());
}
@Test
- void unavailableMetricDoesNotBehaveAsZeroTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
- .operator("<").threshold(0.1).enabled(true).build();
+ void greaterThanComparesAgainstThreshold() {
+ AlertRuleVO rule = rule("broker.disk.usage_ratio",
AlertDomain.CLUSTER, ">", 0.8);
- AlertEvaluationResult result = evaluator.evaluate(rule,
sample(MetricAvailability.UNAVAILABLE, null));
+ assertTrue(evaluator.evaluate(rule, sample("broker.disk.usage_ratio",
AlertDomain.CLUSTER, 0.9))
+ .conditionMet());
+ assertFalse(evaluator.evaluate(rule, sample("broker.disk.usage_ratio",
AlertDomain.CLUSTER, 0.7))
+ .conditionMet());
+ }
- assertThat(result.matches()).isTrue();
- assertThat(result.conditionMet()).isFalse();
- assertThat(result.currentValue()).isNull();
+ @Test
+ void equalityOperatorUsesDoubleCompare() {
+ AlertRuleVO rule = rule("broker.disk.usage_ratio",
AlertDomain.CLUSTER, "==", 0.8);
Review Comment:
Operator coverage regression: the old suite exercised ">=" (two tests) and
"<" (one test); the new suite only covers ">", "==", and "!=". Three of the six
numeric cases in AlertRuleEvaluator.compare() (AlertRuleEvaluator.java:54-57:
>=, <, <=) are now untested, so an off-by-one typo like case ">=" -> value >
threshold would go undetected. This also contradicts the PR description's claim
that the tests pin 'every comparison operator'. Please add cases for ">=", "<",
and "<=", ideally including boundary samples (value == threshold).
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleEvaluatorTest.java:
##########
@@ -21,71 +21,142 @@
import org.junit.jupiter.api.Test;
import java.time.Instant;
+import java.util.Map;
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class AlertRuleEvaluatorTest {
Review Comment:
PR description inaccuracy: the summary says this 'adds the first dedicated
unit test suite for AlertRuleEvaluator', but the diff replaces an existing
5-test suite (index b55fa7c1c..e6ef8e388). This is a rewrite, not an addition,
and it nets a loss of unique coverage (see findings above). Please update the
description so maintainers can see this is a coverage swap and judge the
trade-offs.
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleEvaluatorTest.java:
##########
@@ -21,71 +21,142 @@
import org.junit.jupiter.api.Test;
import java.time.Instant;
+import java.util.Map;
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class AlertRuleEvaluatorTest {
+
private final AlertRuleEvaluator evaluator = new AlertRuleEvaluator();
+ private AlertRuleVO rule(String metric, AlertDomain domain, String
operator, double threshold) {
+ return AlertRuleVO.builder()
+ .domain(domain)
+ .metric(metric)
+ .operator(operator)
+ .threshold(threshold)
+ .duration("5m")
+ .build();
+ }
+
+ private MetricSample sample(String metricKey, AlertDomain domain, double
value) {
+ return new MetricSample(metricKey, domain, "inst-1", "cluster-1",
Map.of(),
+ value, MetricAvailability.AVAILABLE,
Instant.parse("2026-09-01T08:00:00Z"));
+ }
+
+ private MetricSample unavailableSample(String metricKey, AlertDomain
domain) {
+ return new MetricSample(metricKey, domain, "inst-1", "cluster-1",
Map.of(),
+ null, MetricAvailability.UNAVAILABLE,
Instant.parse("2026-09-01T08:00:00Z"));
+ }
+
@Test
- void triggersMatchingAvailableMetricTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
- .operator(">=").threshold(0.85).enabled(true).build();
- MetricSample sample = sample(MetricAvailability.AVAILABLE, 0.9);
+ void mismatchingMetricDoesNotMatch() {
+ AlertEvaluationResult result = evaluator.evaluate(
+ rule("broker.disk.usage_ratio", AlertDomain.CLUSTER, ">", 0.8),
+ sample("broker.cpu.usage", AlertDomain.CLUSTER, 0.9));
- AlertEvaluationResult result = evaluator.evaluate(rule, sample);
+ assertFalse(result.matches());
+ assertEquals(MetricAvailability.AVAILABLE, result.availability());
+ }
+
+ @Test
+ void mismatchingDomainDoesNotMatch() {
+ AlertEvaluationResult result = evaluator.evaluate(
+ rule("consumer.lag.total", AlertDomain.BUSINESS, ">", 100),
+ sample("consumer.lag.total", AlertDomain.CLUSTER, 500));
- assertThat(result.matches()).isTrue();
- assertThat(result.conditionMet()).isTrue();
- assertThat(result.currentValue()).isEqualTo(0.9);
+ assertFalse(result.matches());
}
@Test
- void evaluatesPercentageThresholdsForNativeRatioMetricsTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
-
.operator(">=").threshold(85).thresholdUnit("%").enabled(true).build();
+ void unavailableSampleOnlyMatchesUnavailableOperator() {
+ AlertEvaluationResult matched = evaluator.evaluate(
+ rule("broker.availability", AlertDomain.CLUSTER,
"UNAVAILABLE", 0),
+ unavailableSample("broker.availability", AlertDomain.CLUSTER));
- AlertEvaluationResult result = evaluator.evaluate(rule,
sample(MetricAvailability.AVAILABLE, 0.9));
+ assertTrue(matched.matches());
+ assertTrue(matched.conditionMet());
+ assertNull(matched.currentValue());
- assertThat(result.conditionMet()).isTrue();
+ AlertEvaluationResult notMatched = evaluator.evaluate(
+ rule("broker.availability", AlertDomain.CLUSTER, ">", 0),
+ unavailableSample("broker.availability", AlertDomain.CLUSTER));
+
+ assertTrue(notMatched.matches());
+ assertFalse(notMatched.conditionMet());
}
@Test
- void unavailableMetricDoesNotBehaveAsZeroTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
- .operator("<").threshold(0.1).enabled(true).build();
+ void greaterThanComparesAgainstThreshold() {
+ AlertRuleVO rule = rule("broker.disk.usage_ratio",
AlertDomain.CLUSTER, ">", 0.8);
- AlertEvaluationResult result = evaluator.evaluate(rule,
sample(MetricAvailability.UNAVAILABLE, null));
+ assertTrue(evaluator.evaluate(rule, sample("broker.disk.usage_ratio",
AlertDomain.CLUSTER, 0.9))
+ .conditionMet());
+ assertFalse(evaluator.evaluate(rule, sample("broker.disk.usage_ratio",
AlertDomain.CLUSTER, 0.7))
+ .conditionMet());
+ }
- assertThat(result.matches()).isTrue();
- assertThat(result.conditionMet()).isFalse();
- assertThat(result.currentValue()).isNull();
+ @Test
+ void equalityOperatorUsesDoubleCompare() {
+ AlertRuleVO rule = rule("broker.disk.usage_ratio",
AlertDomain.CLUSTER, "==", 0.8);
+
+ assertTrue(evaluator.evaluate(rule, sample("broker.disk.usage_ratio",
AlertDomain.CLUSTER, 0.8))
+ .conditionMet());
+ assertFalse(evaluator.evaluate(rule, sample("broker.disk.usage_ratio",
AlertDomain.CLUSTER, 0.81))
+ .conditionMet());
}
@Test
- void explicitlyTriggersAvailabilityRuleForUnavailableSampleTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.availability")
- .operator("UNAVAILABLE").enabled(true).build();
- MetricSample sample = new MetricSample("broker.availability",
AlertDomain.CLUSTER, "local", null, null,
- null, MetricAvailability.UNAVAILABLE, Instant.now());
+ void notEqualsOperatorDetectsDifference() {
+ AlertRuleVO rule = rule("broker.disk.usage_ratio",
AlertDomain.CLUSTER, "!=", 0.8);
- AlertEvaluationResult result = evaluator.evaluate(rule, sample);
+ assertTrue(evaluator.evaluate(rule, sample("broker.disk.usage_ratio",
AlertDomain.CLUSTER, 0.81))
+ .conditionMet());
+ assertFalse(evaluator.evaluate(rule, sample("broker.disk.usage_ratio",
AlertDomain.CLUSTER, 0.8))
+ .conditionMet());
+ }
- assertThat(result.matches()).isTrue();
- assertThat(result.conditionMet()).isTrue();
- assertThat(result.currentValue()).isNull();
+ @Test
+ void unknownOrNullOperatorNeverMatches() {
+ AlertRuleVO unknown = rule("broker.disk.usage_ratio",
AlertDomain.CLUSTER, "~", 0.8);
+ assertFalse(evaluator.evaluate(unknown,
sample("broker.disk.usage_ratio", AlertDomain.CLUSTER, 0.9))
+ .conditionMet());
+
+ AlertRuleVO nullOp = rule("broker.disk.usage_ratio",
AlertDomain.CLUSTER, null, 0.8);
+ assertFalse(evaluator.evaluate(nullOp,
sample("broker.disk.usage_ratio", AlertDomain.CLUSTER, 0.9))
+ .conditionMet());
}
@Test
- void doesNotMatchOtherRuleDomainTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.BUSINESS).metric("broker.disk.usage_ratio")
- .operator(">=").threshold(0.85).enabled(true).build();
+ void nullRuleOrSampleDoesNotMatch() {
+ AlertEvaluationResult noRule = evaluator.evaluate(null,
+ sample("broker.disk.usage_ratio", AlertDomain.CLUSTER, 0.9));
+
+ assertFalse(noRule.matches());
+ assertEquals(MetricAvailability.AVAILABLE, noRule.availability());
- assertThat(evaluator.evaluate(rule,
sample(MetricAvailability.AVAILABLE, 0.9)).matches()).isFalse();
+ AlertEvaluationResult noSample = evaluator.evaluate(
+ rule("broker.disk.usage_ratio", AlertDomain.CLUSTER, ">",
0.8), null);
+
+ assertFalse(noSample.matches());
+ assertNull(noSample.availability());
}
- private static MetricSample sample(MetricAvailability availability, Double
value) {
- return new MetricSample("broker.disk.usage_ratio",
AlertDomain.CLUSTER, "local", null, null, value,
- availability, Instant.now());
+ @Test
+ void nullDomainDefaultsToBusiness() {
+ AlertRuleVO rule = AlertRuleVO.builder()
Review Comment:
nullDomainDefaultsToBusiness does not exercise the evaluator's null-domain
fallback. AlertRuleVO declares @Builder.Default private AlertDomain domain =
AlertDomain.BUSINESS (AlertRuleVO.java:32-33), so builder().metric(...).build()
produces domain == BUSINESS, not null, and AlertRuleEvaluator.ruleDomain()'s
null branch (AlertRuleEvaluator.java:45-47) is never reached. The test
currently validates the Lombok default, not the evaluator behavior in its name.
Pass the null explicitly, e.g.
AlertRuleVO.builder().domain(null).metric(...)... — or alternatively via new
AlertRuleVO() plus setters with domain left null.
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleEvaluatorTest.java:
##########
@@ -21,71 +21,142 @@
import org.junit.jupiter.api.Test;
import java.time.Instant;
+import java.util.Map;
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
Review Comment:
Style consistency: 24 of the 32 test files in this package use AssertJ
(assertThat(...).isTrue() etc.), and this file previously did too. The rewrite
switches to raw JUnit 5 Assertions, going against the package convention. Not
blocking, but keeping AssertJ would preserve consistency and give better
failure messages (e.g. assertEquals(0.9, result.currentValue()) would print
both values).
##########
server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleEvaluatorTest.java:
##########
@@ -21,71 +21,142 @@
import org.junit.jupiter.api.Test;
import java.time.Instant;
+import java.util.Map;
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class AlertRuleEvaluatorTest {
+
private final AlertRuleEvaluator evaluator = new AlertRuleEvaluator();
+ private AlertRuleVO rule(String metric, AlertDomain domain, String
operator, double threshold) {
+ return AlertRuleVO.builder()
+ .domain(domain)
+ .metric(metric)
+ .operator(operator)
+ .threshold(threshold)
+ .duration("5m")
+ .build();
+ }
+
+ private MetricSample sample(String metricKey, AlertDomain domain, double
value) {
+ return new MetricSample(metricKey, domain, "inst-1", "cluster-1",
Map.of(),
+ value, MetricAvailability.AVAILABLE,
Instant.parse("2026-09-01T08:00:00Z"));
+ }
+
+ private MetricSample unavailableSample(String metricKey, AlertDomain
domain) {
+ return new MetricSample(metricKey, domain, "inst-1", "cluster-1",
Map.of(),
+ null, MetricAvailability.UNAVAILABLE,
Instant.parse("2026-09-01T08:00:00Z"));
+ }
+
@Test
- void triggersMatchingAvailableMetricTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
- .operator(">=").threshold(0.85).enabled(true).build();
- MetricSample sample = sample(MetricAvailability.AVAILABLE, 0.9);
+ void mismatchingMetricDoesNotMatch() {
+ AlertEvaluationResult result = evaluator.evaluate(
+ rule("broker.disk.usage_ratio", AlertDomain.CLUSTER, ">", 0.8),
+ sample("broker.cpu.usage", AlertDomain.CLUSTER, 0.9));
- AlertEvaluationResult result = evaluator.evaluate(rule, sample);
+ assertFalse(result.matches());
+ assertEquals(MetricAvailability.AVAILABLE, result.availability());
+ }
+
+ @Test
+ void mismatchingDomainDoesNotMatch() {
+ AlertEvaluationResult result = evaluator.evaluate(
+ rule("consumer.lag.total", AlertDomain.BUSINESS, ">", 100),
+ sample("consumer.lag.total", AlertDomain.CLUSTER, 500));
- assertThat(result.matches()).isTrue();
- assertThat(result.conditionMet()).isTrue();
- assertThat(result.currentValue()).isEqualTo(0.9);
+ assertFalse(result.matches());
}
@Test
- void evaluatesPercentageThresholdsForNativeRatioMetricsTest() {
- AlertRuleVO rule =
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
-
.operator(">=").threshold(85).thresholdUnit("%").enabled(true).build();
+ void unavailableSampleOnlyMatchesUnavailableOperator() {
+ AlertEvaluationResult matched = evaluator.evaluate(
Review Comment:
Optional: MetricAvailability has four values (AVAILABLE, UNAVAILABLE,
UNSUPPORTED, STALE) and the evaluator routes all non-AVAILABLE samples through
the same branch (AlertRuleEvaluator.java:34-38: matches()=true,
conditionMet()=false unless operator is UNAVAILABLE and availability is
UNAVAILABLE). Only UNAVAILABLE is tested. A one-line case with a STALE or
UNSUPPORTED sample would pin that these also never satisfy a numeric operator —
a pre-existing gap, fine to address here or separately.
--
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]
