fix(sql): Fixing tests (spinnaker#2535)

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,6 @@
+#Thu Nov 29 10:16:52 PST 2018
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.2-bin.zip
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
+distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.2-all.zip

orca-core-tck/src/main/groovy/com/netflix/spinnaker/orca/pipeline/persistence/ExecutionRepositoryTck.groovy

@@ -224,7 +224,7 @@ abstract class ExecutionRepositoryTck<T extends ExecutionRepository> extends Spe
     )
 
     then:
-    orchestrations*.id == ['not-started-2', 'not-started-1', 'not-too-old', 'pretty-new']
+    orchestrations*.id == ['not-started-2', 'not-started-1', 'pretty-new', 'not-too-old']
 
     where:
     daysOfExecutionHistory = 14

orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/persistence/ExecutionRepository.java

@@ -100,7 +100,7 @@ List<Execution> retrieveOrchestrationsForApplication(@Nonnull String application
   List<String> retrieveAllApplicationNames(@Nullable ExecutionType executionType, int minExecutions);
 
   boolean hasExecution(@Nonnull ExecutionType type, @Nonnull String id);
-  
+
   List<String> retrieveAllExecutionIds(@Nonnull ExecutionType type);
 
   final class ExecutionCriteria {
@@ -178,13 +178,23 @@ public int compare(Execution a, Execution b) {
       }
     },
 
+    /**
+     * Sort executions nulls first, then by startTime descending, breaking ties by lexicographically descending IDs.
+     */
     START_TIME_OR_ID {
       @Override
       public int compare(Execution a, Execution b) {
-        Long aStartTime = Optional.ofNullable(a.getStartTime()).orElse(0L);
-        Long bStartTime = Optional.ofNullable(b.getStartTime()).orElse(0L);
+        Long aStartTime = a.getStartTime();
+        Long bStartTime = b.getStartTime();
+
+        if (aStartTime == null) {
+          return -1;
+        }
+        if (bStartTime == null) {
+          return 0;
+        }
 
-        int startCompare = aStartTime.compareTo(bStartTime);
+        int startCompare = bStartTime.compareTo(aStartTime);
         if (startCompare == 0) {
           return b.getId().compareTo(a.getId());
         }

orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/persistence/ExecutionComparatorSpec.groovy

@@ -53,7 +53,7 @@ class ExecutionComparatorSpec extends Specification {
     executions.sort(START_TIME_OR_ID)
 
     then:
-    assert executions*.id == ["4", "3", "1", "2"]
+    assert executions*.id == ["4", "2", "3", "1"]
   }
 
   def "sort by REVERSE_BUILD_TIME"() {

orca-sql/orca-sql.gradle

@@ -15,11 +15,8 @@
  */
 
 apply from: "$rootDir/gradle/kotlin.gradle"
-apply from: "$rootDir/gradle/spek.gradle"
 apply from: "$rootDir/gradle/spock.gradle"
 
-apply plugin: "groovy"
-
 dependencies {
   compile project(":orca-core")
 
@@ -35,4 +32,20 @@ dependencies {
   testCompile "com.h2database:h2:1.4.197"
   testCompile project(":orca-core-tck")
   testCompile project(":orca-test-groovy")
+
+  testCompile "io.strikt:strikt-core:0.11.5"
+  testCompile "org.assertj:assertj-core:3.9.0"
+  testCompile "org.junit.jupiter:junit-jupiter-api:$jupiterVersion"
+  testCompile "com.oneeyedmen:minutest:+"
+  testCompile "com.nhaarman:mockito-kotlin:1.5.0"
+
+  testRuntime "org.junit.jupiter:junit-jupiter-engine:$jupiterVersion"
+  testRuntime "org.junit.platform:junit-platform-launcher:$junitVersion"
+  testRuntime "org.junit.vintage:junit-vintage-engine:$junitLegacyVersion"
+}
+
+test {
+  useJUnitPlatform {
+    includeEngines "junit-vintage", "junit-jupiter"
+  }
 }

orca-sql/src/main/kotlin/com/netflix/spinnaker/orca/sql/pipeline/persistence/SqlExecutionRepository.kt

@@ -51,7 +51,10 @@ import org.jooq.SelectWhereStep
 import org.jooq.Table
 import org.jooq.exception.SQLDialectNotSupportedException
 import org.jooq.impl.DSL
-import org.jooq.impl.DSL.*
+import org.jooq.impl.DSL.count
+import org.jooq.impl.DSL.field
+import org.jooq.impl.DSL.name
+import org.jooq.impl.DSL.table
 import org.slf4j.LoggerFactory
 import rx.Observable
 import java.lang.System.currentTimeMillis
@@ -282,18 +285,19 @@ class SqlExecutionRepository(
 
         val startTime = criteria.startTimeCutoff
         if (startTime != null) {
-          // This may look like a bug, but it isn't. Start time isn't always set (NOT_STARTED status). We
-          // don't want to exclude Executions that haven't started, but we also want to still reduce the result set.
           where
-            .and(field("build_time").greaterThan(startTime.toEpochMilli()))
+            .and(
+              field("start_time").greaterThan(startTime.toEpochMilli())
+                .or(field("start_time").isNull)
+            )
             .statusIn(criteria.statuses)
         } else {
           where.statusIn(criteria.statuses)
         }
       },
       seek = {
         val ordered = when (sorter) {
-          START_TIME_OR_ID -> it.orderBy(field("start_time").desc(), field("id").desc())
+          START_TIME_OR_ID -> it.orderBy(field("start_time").desc().nullsFirst(), field("id").desc())
           REVERSE_BUILD_TIME -> it.orderBy(field("build_time").asc(), field("id").asc())
           else -> it.orderBy(field("id").desc())
         }

orca-sql/src/test/groovy/com/netflix/spinnaker/orca/sql/cleanup/OldPipelineCleanupPollingNotificationAgentSpec.groovy

@@ -64,7 +64,7 @@ class OldPipelineCleanupPollingNotificationAgentSpec extends Specification {
 
   def setupSpec() {
     currentDatabase = initDatabase()
-    executionRepository = new SqlExecutionRepository("test", currentDatabase.context, mapper, new TransactionRetryProperties())
+    executionRepository = new SqlExecutionRepository("test", currentDatabase.context, mapper, new TransactionRetryProperties(), 10)
   }
 
   def "should preserve the most recent 5 executions when cleaning up old pipeline executions"() {

orca-sql/src/test/kotlin/com/netflix/spinnaker/orca/sql/SqlHealthcheckQueueActivatorSpec.kt → orca-sql/src/test/kotlin/com/netflix/spinnaker/orca/sql/SqlHealthcheckQueueActivatorTest.kt

@@ -16,62 +16,59 @@
 package com.netflix.spinnaker.orca.sql
 
 import com.netflix.spectator.api.NoopRegistry
-import com.nhaarman.mockito_kotlin.*
-import org.jetbrains.spek.api.Spek
-import org.jetbrains.spek.api.dsl.describe
-import org.jetbrains.spek.api.dsl.given
-import org.jetbrains.spek.api.dsl.it
-import org.jetbrains.spek.api.dsl.on
+import com.nhaarman.mockito_kotlin.doReturn
+import com.nhaarman.mockito_kotlin.doThrow
+import com.nhaarman.mockito_kotlin.isA
+import com.nhaarman.mockito_kotlin.mock
+import com.nhaarman.mockito_kotlin.reset
+import com.nhaarman.mockito_kotlin.whenever
+import com.oneeyedmen.minutest.junit.JupiterTests
+import com.oneeyedmen.minutest.junit.context
 import org.jooq.DSLContext
 import org.jooq.DeleteWhereStep
 import org.jooq.Table
 import strikt.api.expect
 import strikt.assertions.isEqualTo
 
-object SqlHealthcheckQueueActivatorSpec : Spek({
+class SqlHealthcheckQueueActivatorTest : JupiterTests {
 
-  describe("healthchecking sql") {
+  override val tests = context<Unit> {
 
     val dslContext = mock<DSLContext>()
     val query = mock<DeleteWhereStep<*>>()
 
-    given("a healthy current state") {
+    after {
+      reset(dslContext, query)
+    }
+
+    context("a healthy current state") {
       val subject = SqlHealthcheckActivator(dslContext, NoopRegistry(), unhealthyThreshold = 1).apply {
         _enabled.set(true)
       }
 
-      afterEachTest { reset(dslContext, query) }
-
-      on("successive write failures") {
+      test("successive write failures") {
         whenever(dslContext.delete(isA<Table<*>>())) doThrow RuntimeException("oh no")
 
         subject.performWrite()
         subject.performWrite()
 
-        it("deactivates its enabled flag") {
-          expect(subject.enabled).isEqualTo(false)
-        }
+        expect(subject.enabled).isEqualTo(false)
       }
     }
 
-    given("an unhealthy sql connection") {
+    context("an unhealthy sql connection") {
       val subject = SqlHealthcheckActivator(dslContext, NoopRegistry(), healthyThreshold = 1).apply {
         _enabled.set(false)
       }
 
-      afterEachTest { reset(dslContext, query) }
-
-      on("successive write failures") {
+      test("successive write failures") {
         whenever(dslContext.delete(isA<Table<*>>())) doReturn query
 
         subject.performWrite()
         subject.performWrite()
 
-        it("deactivates its enabled flag") {
-          expect(subject.enabled).isEqualTo(true)
-        }
+        expect(subject.enabled).isEqualTo(true)
       }
-
     }
   }
-})
+}