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 2bf2c464718322ad7713cbc6391f580c592a0c9a..0829833b5847a10cd61887d4e5544e21ae13118a 100644 (file)
--- 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<EvaluationResult> evaluateCondition(Condition condition, Comparable<?> measureComparable, Measure.Level alertLevel) {
+  private static Optional<EvaluationResult> 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 c8aa67fac9cb894ec6fff471f5465771cbfbca00..246aa59975eb9da60d71bedf411e62e3d01b0868 100644 (file)
--- 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);
+  }
 }