From: Sébastien Lesaint Date: Tue, 23 Jun 2015 11:43:28 +0000 (+0200) Subject: improve coverage of ConditionEvaluator X-Git-Tag: 5.2-RC1~1358 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=d70f025c8f9504c0f9f736a6207ffe954d468828;p=sonarqube.git improve coverage of ConditionEvaluator --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/qualitygate/ConditionEvaluator.java b/server/sonar-server/src/main/java/org/sonar/server/computation/qualitygate/ConditionEvaluator.java index 2bf2c464718..0829833b584 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/qualitygate/ConditionEvaluator.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/qualitygate/ConditionEvaluator.java @@ -20,7 +20,6 @@ package org.sonar.server.computation.qualitygate; import com.google.common.base.Optional; -import com.google.common.base.Preconditions; import javax.annotation.CheckForNull; import org.apache.commons.lang.StringUtils; import org.sonar.server.computation.measure.Measure; @@ -29,7 +28,6 @@ import org.sonar.server.computation.metric.Metric; import static com.google.common.base.Optional.of; import static com.google.common.base.Preconditions.checkArgument; -import static java.util.Objects.requireNonNull; public final class ConditionEvaluator { @@ -51,7 +49,7 @@ public final class ConditionEvaluator { .or(new EvaluationResult(Measure.Level.OK, measureComparable)); } - private Optional evaluateCondition(Condition condition, Comparable measureComparable, Measure.Level alertLevel) { + private static Optional evaluateCondition(Condition condition, Comparable measureComparable, Measure.Level alertLevel) { String conditionValue = getValueToEval(condition, alertLevel); if (StringUtils.isEmpty(conditionValue)) { return Optional.absent(); @@ -88,9 +86,9 @@ public final class ConditionEvaluator { case NOT_EQUALS: return comparison != 0; case GREATER_THAN: - return comparison == 1; + return comparison > 0; case LESS_THAN: - return comparison == -1; + return comparison < 0; default: throw new IllegalArgumentException(String.format("Unsupported operator '%s'", condition.getOperator())); } @@ -186,7 +184,7 @@ public final class ConditionEvaluator { case 5: return variations.hasVariation5() ? of(variations.getVariation5()) : NO_PERIOD_VALUE; default: - throw new IllegalStateException("Following index period is not allowed : " + Double.toString(period)); + throw new IllegalArgumentException("Following index period is not allowed : " + period); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/qualitygate/ConditionEvaluatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/qualitygate/ConditionEvaluatorTest.java index c8aa67fac9c..246aa59975e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/qualitygate/ConditionEvaluatorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/qualitygate/ConditionEvaluatorTest.java @@ -19,16 +19,25 @@ */ package org.sonar.server.computation.qualitygate; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.server.computation.measure.Measure; +import org.sonar.server.computation.measure.MeasureVariations; import org.sonar.server.computation.metric.Metric; import org.sonar.server.computation.metric.MetricImpl; +import static com.google.common.base.Predicates.in; +import static com.google.common.base.Predicates.not; +import static com.google.common.collect.FluentIterable.from; +import static java.util.Arrays.asList; import static org.junit.Assert.fail; import static org.sonar.server.computation.measure.Measure.Level.ERROR; import static org.sonar.server.computation.measure.Measure.Level.OK; +import static org.sonar.server.computation.measure.Measure.newMeasureBuilder; +import static org.sonar.server.computation.metric.Metric.MetricType; import static org.sonar.server.computation.qualitygate.Condition.Operator.EQUALS; import static org.sonar.server.computation.qualitygate.Condition.Operator.GREATER_THAN; import static org.sonar.server.computation.qualitygate.Condition.Operator.LESS_THAN; @@ -44,24 +53,24 @@ public class ConditionEvaluatorTest { @Test public void testInputNumbers() { try { - Metric metric = createMetric(Metric.MetricType.FLOAT); - Measure measure = Measure.newMeasureBuilder().create(10.2d, null); + Metric metric = createMetric(MetricType.FLOAT); + Measure measure = newMeasureBuilder().create(10.2d, null); underTest.evaluate(createErrorCondition(metric, LESS_THAN, "20"), measure); } catch (NumberFormatException ex) { fail(); } try { - Metric metric = createMetric(Metric.MetricType.INT); - Measure measure = Measure.newMeasureBuilder().create(5, null); + Metric metric = createMetric(MetricType.INT); + Measure measure = newMeasureBuilder().create(5, null); underTest.evaluate(createErrorCondition(metric, LESS_THAN, "20.1"), measure); } catch (NumberFormatException ex) { fail(); } try { - Metric metric = createMetric(Metric.MetricType.PERCENT); - Measure measure = Measure.newMeasureBuilder().create(10.2d, null); + Metric metric = createMetric(MetricType.PERCENT); + Measure measure = newMeasureBuilder().create(10.2d, null); underTest.evaluate(createErrorCondition(metric, LESS_THAN, "20.1"), measure); } catch (NumberFormatException ex) { fail(); @@ -72,14 +81,14 @@ public class ConditionEvaluatorTest { return new Condition(metric, operator.getDbValue(), errorThreshold, null, null); } - private static MetricImpl createMetric(Metric.MetricType metricType) { + private static MetricImpl createMetric(MetricType metricType) { return new MetricImpl(1, "key", "name", metricType); } @Test public void testEquals_for_double() { - Metric metric = createMetric(Metric.MetricType.FLOAT); - Measure measure = Measure.newMeasureBuilder().create(10.2d, null); + Metric metric = createMetric(MetricType.FLOAT); + Measure measure = newMeasureBuilder().create(10.2d, null); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(ERROR).hasValue(10.2d); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.1"), measure)).hasLevel(OK).hasValue(10.2d); @@ -87,8 +96,8 @@ public class ConditionEvaluatorTest { @Test public void testEquals_for_String() { - Metric metric = createMetric(Metric.MetricType.STRING); - Measure measure = Measure.newMeasureBuilder().create("TEST"); + Metric metric = createMetric(MetricType.STRING); + Measure measure = newMeasureBuilder().create("TEST"); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "TEST"), measure)).hasLevel(ERROR).hasValue("TEST"); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "TEST2"), measure)).hasLevel(OK).hasValue("TEST"); @@ -96,8 +105,8 @@ public class ConditionEvaluatorTest { @Test public void testNotEquals_for_double() { - Metric metric = createMetric(Metric.MetricType.FLOAT); - Measure measure = Measure.newMeasureBuilder().create(10.2d, null); + Metric metric = createMetric(MetricType.FLOAT); + Measure measure = newMeasureBuilder().create(10.2d, null); assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "10.2"), measure)).hasLevel(OK).hasValue(10.2d); assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "10.1"), measure)).hasLevel(ERROR).hasValue(10.2d); @@ -105,8 +114,8 @@ public class ConditionEvaluatorTest { @Test public void testNotEquals() { - Metric metric = createMetric(Metric.MetricType.STRING); - Measure measure = Measure.newMeasureBuilder().create("TEST"); + Metric metric = createMetric(MetricType.STRING); + Measure measure = newMeasureBuilder().create("TEST"); assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "TEST"), measure)).hasLevel(OK).hasValue("TEST"); assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "TEST2"), measure)).hasLevel(ERROR).hasValue("TEST"); @@ -114,8 +123,8 @@ public class ConditionEvaluatorTest { @Test public void testGreater() { - Metric metric = createMetric(Metric.MetricType.FLOAT); - Measure measure = Measure.newMeasureBuilder().create(10.2d, null); + Metric metric = createMetric(MetricType.FLOAT); + Measure measure = newMeasureBuilder().create(10.2d, null); assertThat(underTest.evaluate(createErrorCondition(metric, GREATER_THAN, "10.1"), measure)).hasLevel(ERROR).hasValue(10.2d); assertThat(underTest.evaluate(createErrorCondition(metric, GREATER_THAN, "10.2"), measure)).hasLevel(OK).hasValue(10.2d); @@ -124,8 +133,8 @@ public class ConditionEvaluatorTest { @Test public void testSmaller() { - Metric metric = createMetric(Metric.MetricType.FLOAT); - Measure measure = Measure.newMeasureBuilder().create(10.2d, null); + Metric metric = createMetric(MetricType.FLOAT); + Measure measure = newMeasureBuilder().create(10.2d, null); assertThat(underTest.evaluate(createErrorCondition(metric, LESS_THAN, "10.1"), measure)).hasLevel(OK).hasValue(10.2d); assertThat(underTest.evaluate(createErrorCondition(metric, LESS_THAN, "10.2"), measure)).hasLevel(OK).hasValue(10.2d); @@ -134,24 +143,24 @@ public class ConditionEvaluatorTest { @Test public void testEquals_Percent() { - Metric metric = createMetric(Metric.MetricType.PERCENT); - Measure measure = Measure.newMeasureBuilder().create(10.2d, null); + Metric metric = createMetric(MetricType.PERCENT); + Measure measure = newMeasureBuilder().create(10.2d, null); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(ERROR).hasValue(10.2d); } @Test public void testEquals_Float() { - Metric metric = createMetric(Metric.MetricType.PERCENT); - Measure measure = Measure.newMeasureBuilder().create(10.2d, null); + Metric metric = createMetric(MetricType.PERCENT); + Measure measure = newMeasureBuilder().create(10.2d, null); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(ERROR).hasValue(10.2d); } @Test public void testEquals_Int() { - Metric metric = createMetric(Metric.MetricType.INT); - Measure measure = Measure.newMeasureBuilder().create(10, null); + Metric metric = createMetric(MetricType.INT); + Measure measure = newMeasureBuilder().create(10, null); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10"), measure)).hasLevel(ERROR).hasValue(10); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(ERROR).hasValue(10); @@ -159,8 +168,8 @@ public class ConditionEvaluatorTest { @Test public void testEquals_Level() { - Metric metric = createMetric(Metric.MetricType.LEVEL); - Measure measure = Measure.newMeasureBuilder().create(ERROR); + Metric metric = createMetric(MetricType.LEVEL); + Measure measure = newMeasureBuilder().create(ERROR); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, ERROR.name()), measure)).hasLevel(ERROR).hasValue(ERROR.name()); @@ -169,16 +178,16 @@ public class ConditionEvaluatorTest { @Test public void testNotEquals_Level() { - Metric metric = createMetric(Metric.MetricType.LEVEL); - Measure measure = Measure.newMeasureBuilder().create(ERROR); + Metric metric = createMetric(MetricType.LEVEL); + Measure measure = newMeasureBuilder().create(ERROR); assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, OK.name()), measure)).hasLevel(ERROR).hasValue(ERROR.name()); } @Test public void testEquals_BOOL() { - Metric metric = createMetric(Metric.MetricType.BOOL); - Measure measure = Measure.newMeasureBuilder().create(false, null); + Metric metric = createMetric(MetricType.BOOL); + Measure measure = newMeasureBuilder().create(false, null); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "1"), measure)).hasLevel(OK).hasValue(false); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "0"), measure)).hasLevel(ERROR).hasValue(false); @@ -186,8 +195,8 @@ public class ConditionEvaluatorTest { @Test public void testNotEquals_BOOL() { - Metric metric = createMetric(Metric.MetricType.BOOL); - Measure measure = Measure.newMeasureBuilder().create(false, null); + Metric metric = createMetric(MetricType.BOOL); + Measure measure = newMeasureBuilder().create(false, null); assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "1"), measure)).hasLevel(ERROR).hasValue(false); assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "0"), measure)).hasLevel(OK).hasValue(false); @@ -195,8 +204,8 @@ public class ConditionEvaluatorTest { @Test public void getLevel_throws_IEA_if_error_threshold_is_not_parsable_boolean() { - Metric metric = createMetric(Metric.MetricType.BOOL); - Measure measure = Measure.newMeasureBuilder().create(false, null); + Metric metric = createMetric(MetricType.BOOL); + Measure measure = newMeasureBuilder().create(false, null); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Quality Gate: Unable to parse value 'polop' to compare against name"); @@ -206,16 +215,16 @@ public class ConditionEvaluatorTest { @Test public void testEquals_work_duration() { - Metric metric = createMetric(Metric.MetricType.WORK_DUR); - Measure measure = Measure.newMeasureBuilder().create(60l, null); + Metric metric = createMetric(MetricType.WORK_DUR); + Measure measure = newMeasureBuilder().create(60l, null); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "60"), measure)).hasLevel(ERROR); } @Test public void getLevel_throws_IEA_if_error_threshold_is_not_parsable_long() { - Metric metric = createMetric(Metric.MetricType.WORK_DUR); - Measure measure = Measure.newMeasureBuilder().create(60l, null); + Metric metric = createMetric(MetricType.WORK_DUR); + Measure measure = newMeasureBuilder().create(60l, null); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Quality Gate: Unable to parse value 'polop' to compare against name"); @@ -225,19 +234,30 @@ public class ConditionEvaluatorTest { @Test public void testErrorAndWarningLevel() { - Metric metric = createMetric(Metric.MetricType.FLOAT); - Measure measure = Measure.newMeasureBuilder().create(10.2d, null); + Metric metric = createMetric(MetricType.FLOAT); + Measure measure = newMeasureBuilder().create(10.2d, null); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(ERROR); assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.1"), measure)).hasLevel(OK); assertThat(underTest.evaluate(new Condition(metric, EQUALS.getDbValue(), "10.3", "10.2", null), measure)).hasLevel(Measure.Level.WARN); + assertThat(underTest.evaluate(new Condition(metric, LESS_THAN.getDbValue(), "10.3", "10.2", null), measure)).hasLevel(Measure.Level.ERROR); + } + + @Test + public void condition_is_always_ok_when_measure_is_noValue() { + for (MetricType metricType : from(asList(MetricType.values())).filter(not(in(ImmutableSet.of(MetricType.DATA, MetricType.LEVEL))))) { + Metric metric = createMetric(metricType); + Measure measure = newMeasureBuilder().createNoValue(); + + assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(OK); + } } @Test public void testUnsupportedType() { - Metric metric = createMetric(Metric.MetricType.DATA); - Measure measure = Measure.newMeasureBuilder().create("3.14159265358"); + Metric metric = createMetric(MetricType.DATA); + Measure measure = newMeasureBuilder().create("3.14159265358"); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Conditions on MetricType DATA are not supported"); @@ -245,4 +265,37 @@ public class ConditionEvaluatorTest { underTest.evaluate(createErrorCondition(metric, EQUALS, "1.60217657"), measure); } + @Test + public void test_condition_on_period() { + for (MetricType metricType : ImmutableList.of(MetricType.FLOAT, MetricType.INT, MetricType.WORK_DUR)) { + Metric metric = createMetric(metricType); + Measure measure = newMeasureBuilder().setVariations(new MeasureVariations(null, 3d, 4d, null, null)).createNoValue(); + + assertThat(underTest.evaluate(new Condition(metric, GREATER_THAN.getDbValue(), "3", null, 2), measure)).hasLevel(OK); + assertThat(underTest.evaluate(new Condition(metric, GREATER_THAN.getDbValue(), "2", null, 2), measure)).hasLevel(ERROR); + assertThat(underTest.evaluate(new Condition(metric, GREATER_THAN.getDbValue(), "4", null, 3), measure)).hasLevel(OK); + assertThat(underTest.evaluate(new Condition(metric, GREATER_THAN.getDbValue(), "3", null, 3), measure)).hasLevel(ERROR); + } + } + + @Test + public void condition_on_period_without_value_is_OK() { + Metric metric = createMetric(MetricType.FLOAT); + Measure measure = newMeasureBuilder().setVariations(new MeasureVariations(null, 3d, 4d, null, null)).createNoValue(); + + assertThat(underTest.evaluate(new Condition(metric, GREATER_THAN.getDbValue(), "3", null, 1), measure)).hasLevel(OK); + assertThat(underTest.evaluate(new Condition(metric, GREATER_THAN.getDbValue(), "3", null, 4), measure)).hasLevel(OK); + assertThat(underTest.evaluate(new Condition(metric, GREATER_THAN.getDbValue(), "3", null, 5), measure)).hasLevel(OK); + } + + @Test + public void evaluate_throws_IAE_if_condition_is_on_invalid_period() { + Metric metric = createMetric(MetricType.FLOAT); + Measure measure = newMeasureBuilder().setVariations(new MeasureVariations(null, 3d, 4d, null, null)).createNoValue(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Following index period is not allowed : 50"); + + underTest.evaluate(new Condition(metric, GREATER_THAN.getDbValue(), "3", null, 50), measure); + } }