Spike filter default (#105)

* no default value for spike filter warp properties * minor query tweaks for better usability * DB tag name query should use Option --------- Co-authored-by: tomas mccandless <[email protected]>
Workday · Sep 29, 2023 · 1282d71 · 1282d71
1 parent fcb5235
commit 1282d71
Show file tree

Hide file tree

Showing 9 changed files with 58 additions and 33 deletions.
diff --git a/warp-core/src/main/scala/com/workday/warp/arbiters/ArbiterLike.scala b/warp-core/src/main/scala/com/workday/warp/arbiters/ArbiterLike.scala
@@ -72,7 +72,7 @@ trait ArbiterLike extends PersistenceAware with CanReadHistory {
     * @return a wrapped error with a useful message, or None if the measured test passed its requirement.
     */
   final def voteWithSpikeFilter[T: TestExecutionRowLikeType](ballot: Ballot, testExecution: T): Option[Throwable] = {
-    val (spikeFilterEnabled, alertOnNth) = spikeFilterSettings(testExecution)
+    val (spikeFilterEnabled, alertOnNth) = spikeFilterSettings(testExecution.idTestDefinition)
     voteWithSpikeFilter(ballot, testExecution, spikeFilterEnabled, alertOnNth)
   }
 
@@ -87,8 +87,8 @@ trait ArbiterLike extends PersistenceAware with CanReadHistory {
     *
     * @return spike filtering settings.
     */
-  def spikeFilterSettings[T: TestExecutionRowLikeType](testExecution: T): (Boolean, Int) = {
-    var settings: (Boolean, Int) = this.persistenceUtils.getSpikeFilterSettings(testExecution)
+  def spikeFilterSettings(idTestDefinition: Int): (Boolean, Int) = {
+    var settings: (Boolean, Int) = this.persistenceUtils.getSpikeFilterSettings(idTestDefinition)
       .map(setting => (setting.spikeFilterEnabled, setting.alertOnNth))
       .getOrElse((false, 1))
     // allow individual overrides from properties if they are present

diff --git a/warp-core/src/main/scala/com/workday/warp/arbiters/CanReadHistory.scala b/warp-core/src/main/scala/com/workday/warp/arbiters/CanReadHistory.scala
@@ -59,8 +59,8 @@ trait CanReadHistory extends CorePersistenceAware {
       require(maybePriorExecutions.length <= historySize)
 
       maybePriorExecutions.length == historySize && maybePriorExecutions.forall { execution =>
-        val tag = this.persistenceUtils.getTagName(tagName)
-        this.persistenceUtils.getTestExecutionTagsRowSafe(execution.idTestExecution, tag.idTagName).nonEmpty
+        val maybeTag: Option[TagNameRowLike] = this.persistenceUtils.getTagName(tagName)
+        maybeTag.exists(tag => this.persistenceUtils.getTestExecutionTagsRowSafe(execution.idTestExecution, tag.idTagName).nonEmpty)
       }
     }
   }

diff --git a/warp-core/src/main/scala/com/workday/warp/config/HasCoreWarpProperties.scala b/warp-core/src/main/scala/com/workday/warp/config/HasCoreWarpProperties.scala
@@ -445,21 +445,15 @@ trait HasCoreWarpProperties extends WarpPropertyLike {
     *
     * Required: No
     */
-  val WARP_ARBITER_SPIKE_FILTER_ENABLED: PropertyEntry = PropertyEntry(
-    "wd.warp.arbiter.spike.filter.enabled",
-    isRequired = false,
-    "true"
-  )
+  val WARP_ARBITER_SPIKE_FILTER_ENABLED: PropertyEntry = PropertyEntry("wd.warp.arbiter.spike.filter.enabled")
 
   /**
     * Arbiter flapping number of consecutive failures (depending on arbiter implementation votes)
     * required before failing a build.
     *
     * Required: No
     */
-  val WARP_ARBITER_SPIKE_FILTER_ALERT_ON_NTH: PropertyEntry = PropertyEntry(
-    "wd.warp.arbiter.spike.filter.alert.on.nth", isRequired = false, "1"
-  )
+  val WARP_ARBITER_SPIKE_FILTER_ALERT_ON_NTH: PropertyEntry = PropertyEntry("wd.warp.arbiter.spike.filter.alert.on.nth")
 
   /**
     * Minimum number of recorded measurements necessary for historical arbiters. Applies to all arbiters that extend

diff --git a/warp-core/src/main/scala/com/workday/warp/persistence/AbstractQueries.scala b/warp-core/src/main/scala/com/workday/warp/persistence/AbstractQueries.scala
@@ -357,10 +357,10 @@ trait AbstractQueries {
   /**
     * Creates a [[DBIO]] for reading spike filter settings.
     *
-    * @param testExecution test execution to read spike filter settings for.
+    * @param idTestDefinition test definition to read spike filter settings for.
     * @return a [[DBIO]] (not yet executed) for reading spike filter settings for the given test execution.
     */
-  def getSpikeFilterSettingsQuery[T: TestExecutionRowLikeType](testExecution: T): DBIO[Option[SpikeFilterSettingsRowLike]]
+  def getSpikeFilterSettingsQuery(idTestDefinition: Int): DBIO[Option[SpikeFilterSettingsRowLike]]
 
 
   /**

diff --git a/warp-core/src/main/scala/com/workday/warp/persistence/CorePersistenceAware.scala b/warp-core/src/main/scala/com/workday/warp/persistence/CorePersistenceAware.scala
@@ -364,8 +364,8 @@ trait CorePersistenceAware extends PersistenceAware with WarpLogging {
       * @param name [[String]] name of TagNameRow
       * @return a [[TagNameRowLike]] with the given name
       */
-    override def getTagName(name: String): TagNameRowLike =
-      this.synchronously(this.readTagNameQuery(name)).head
+    override def getTagName(name: String): Option[TagNameRowLike] =
+      this.synchronously(this.readTagNameQuery(name))
 
 
     /**
@@ -403,15 +403,13 @@ trait CorePersistenceAware extends PersistenceAware with WarpLogging {
     }
 
     /**
-      * Reads spike filter settings for the given test execution.
+      * Reads spike filter settings for the given test definition.
       *
-      * @param testExecution execution to look up spike filter settings for.
-      * @tparam T
+      * @param idTEstDefinition test definition to look up spike filter settings for.
       * @return spike filter settings for the given test execution.
       */
-    override def getSpikeFilterSettings[T: TestExecutionRowLikeType](testExecution: T): Option[SpikeFilterSettingsRowLike] = {
-      this.synchronously(getSpikeFilterSettingsQuery(testExecution))
-    }
+    override def getSpikeFilterSettings(idTestDefinition: Int): Option[SpikeFilterSettingsRowLike] =
+      this.synchronously(getSpikeFilterSettingsQuery(idTestDefinition))
 
 
     /**

diff --git a/warp-core/src/main/scala/com/workday/warp/persistence/CoreQueries.scala b/warp-core/src/main/scala/com/workday/warp/persistence/CoreQueries.scala
@@ -585,14 +585,13 @@ trait CoreQueries extends AbstractQueries {
   /**
     * Creates a [[DBIO]] for reading spike filter settings.
     *
-    * @param testExecution test execution to read spike filter settings for.
-    * @tparam T
+    * @param idTestDefinition test definition to read spike filter settings for.
     * @return a [[DBIO]] (not yet executed) for reading spike filter settings for the given test execution.
     */
-  override def getSpikeFilterSettingsQuery[T: TestExecutionRowLikeType](testExecution: T):
+  override def getSpikeFilterSettingsQuery(idTestDefinition: Int):
   DBIO[Option[SpikeFilterSettingsRowLike]] = {
     SpikeFilterSettings
-      .filter(_.idTestDefinition === testExecution.idTestDefinition)
+      .filter(_.idTestDefinition === idTestDefinition)
       .result
       .headOption
   }

diff --git a/warp-core/src/main/scala/com/workday/warp/persistence/PersistenceAware.scala b/warp-core/src/main/scala/com/workday/warp/persistence/PersistenceAware.scala
@@ -364,7 +364,7 @@ trait PersistenceAware extends WarpLogging {
       * @param name [[String]] name of TagNameRow
       * @return a [[TagNameRowLike]] with the given name
       */
-    def getTagName(name: String): TagNameRowLike
+    def getTagName(name: String): Option[TagNameRowLike]
 
 
     /**
@@ -399,11 +399,10 @@ trait PersistenceAware extends WarpLogging {
     /**
       * Reads spike filter settings for the given test execution.
       *
-      * @param testExecution execution to look up spike filter settings for.
-      * @tparam T
+      * @param idTestDefinition test definition to look up spike filter settings for.
       * @return spike filter settings for the given test execution.
       */
-    def getSpikeFilterSettings[T: TestExecutionRowLikeType](testExecution: T): Option[SpikeFilterSettingsRowLike]
+    def getSpikeFilterSettings(idTestDefinition: Int): Option[SpikeFilterSettingsRowLike]
 
 
     /**

diff --git a/warp-core/src/test/scala/com/workday/warp/arbiters/ArbiterLikeSpec.scala b/warp-core/src/test/scala/com/workday/warp/arbiters/ArbiterLikeSpec.scala
@@ -0,0 +1,35 @@
+package com.workday.warp.arbiters
+
+import com.workday.warp.TestId
+import com.workday.warp.junit.{UnitTest, WarpJUnitSpec}
+import com.workday.warp.persistence.Tables._
+import com.workday.warp.persistence.Tables.RowTypeClasses._
+import com.workday.warp.persistence.TablesLike._
+
+import java.time.Instant
+import java.util.UUID
+
+class ArbiterLikeSpec extends WarpJUnitSpec with ArbiterLike {
+
+  @UnitTest
+  def readSpikeFilterSettings(): Unit = {
+    val methodSignature = s"com.workday.warp.arbiters.${UUID.randomUUID.toString}"
+    val testId = TestId.fromString(methodSignature)
+    val testExec: TestExecutionRowLike = this.persistenceUtils.createTestExecution(testId, Instant.now(), 5.0, 6.0)
+
+    val settingsRow = SpikeFilterSettingsRow(testExec.idTestDefinition, false, 10, 10)
+    this.persistenceUtils.writeSpikeFilterSettings(Seq(settingsRow))
+
+    this.spikeFilterSettings(testExec.idTestDefinition) should be (settingsRow.spikeFilterEnabled, settingsRow.alertOnNth)
+  }
+
+  /**
+    * Checks that the measured test passed its performance requirement. If the requirement is failed, constructs an
+    * error with a useful message wrapped in an Option.
+    *
+    * @param ballot        box used to register vote result.
+    * @param testExecution [[TestExecutionRowLikeType]] we are voting on.
+    * @return a wrapped error with a useful message, or None if the measured test passed its requirement.
+    */
+  override def vote[T: TestExecutionRowLikeType](ballot: Ballot, testExecution: T): Option[Throwable] = None
+}
diff --git a/warp-core/src/test/scala/com/workday/warp/arbiters/SmartNumberArbiterSpec.scala b/warp-core/src/test/scala/com/workday/warp/arbiters/SmartNumberArbiterSpec.scala
@@ -221,14 +221,14 @@ class SmartNumberArbiterSpec extends WarpJUnitSpec with CorePersistenceAware wit
     arbiter.voteAndThrow(ballot, testExecution)
 
     // read warp spec test execution tag
-    val tagDescriptionId: Int = this.persistenceUtils.getTagName(CoreConstants.WARP_SPECIFICATION_FIELDS_STRING).idTagName
+    val tagDescriptionId: Int = this.persistenceUtils.getTagName(CoreConstants.WARP_SPECIFICATION_FIELDS_STRING).get.idTagName
     val testExecutionTagId: Int = this.persistenceUtils.getTestExecutionTagsRow(
       testExecution.idTestExecution,
       tagDescriptionId
     ).idTestExecutionTag
 
     // read smart threshold test execution metatag
-    val metaTagDescriptionId: Int = this.persistenceUtils.getTagName(CoreConstants.SMART_THRESHOLD_STRING).idTagName
+    val metaTagDescriptionId: Int = this.persistenceUtils.getTagName(CoreConstants.SMART_THRESHOLD_STRING).get.idTagName
     this.persistenceUtils.synchronously(
       this.persistenceUtils.testExecutionMetaTagQuery(testExecutionTagId, metaTagDescriptionId)
     ).nonEmpty should be (true)