diff --git a/buildSrc/src/main/kotlin/Dependencies.kt b/buildSrc/src/main/kotlin/Dependencies.kt index f5dc443c9..c996d0b56 100644 --- a/buildSrc/src/main/kotlin/Dependencies.kt +++ b/buildSrc/src/main/kotlin/Dependencies.kt @@ -1,6 +1,6 @@ object Dependencies { val apacheCommonsLang3 = "org.apache.commons:commons-lang3:3.11" - val assertj = "org.assertj:assertj-core:3.16.1" + val assertj = "org.assertj:assertj-core:3.20.2" val awsDynamodb = "com.amazonaws:aws-java-sdk-dynamodb:1.11.774" val flywayGradleBuildscriptDep = "gradle.plugin.com.boxfuse.client:flyway-release:5.0.2" val guava = "com.google.guava:guava:30.0-jre" diff --git a/service/src/test/kotlin/app/cash/backfila/TestTools.kt b/service/src/test/kotlin/app/cash/backfila/TestTools.kt index ae3648c2c..7a2403751 100644 --- a/service/src/test/kotlin/app/cash/backfila/TestTools.kt +++ b/service/src/test/kotlin/app/cash/backfila/TestTools.kt @@ -1,8 +1,13 @@ package app.cash.backfila +import app.cash.backfila.dashboard.UiEventLog +import app.cash.backfila.service.persistence.DbEventLog import misk.MiskCaller import misk.inject.keyOf import misk.scope.ActionScope +import org.assertj.core.api.Assertions +import org.assertj.core.api.Condition +import org.assertj.core.api.SoftAssertions fun ActionScope.fakeCaller( service: String? = null, @@ -12,3 +17,24 @@ fun ActionScope.fakeCaller( return enter(mapOf(keyOf() to MiskCaller(service = service, user = user))) .use { function() } } + +// For useful soft assertions without forgetting to call assertAll() +fun softAssert(assertions: SoftAssertions.() -> Unit) { + with(SoftAssertions()) { + assertions() + assertAll() + } +} + +// Makes asserting on UiEventLog entries easy. +fun uiEventLogWith( + type: DbEventLog.Type, + user: String?, + message: String +): Condition { + return Assertions.allOf( + Condition({ it.type == type }, "uiEventLog.type"), + Condition({ it.user == user }, "uiEventLog.user"), + Condition({ it.message == message }, "uiEventLog.message") + ) +} diff --git a/service/src/test/kotlin/app/cash/backfila/actions/StartStopBackfillActionTest.kt b/service/src/test/kotlin/app/cash/backfila/actions/StartStopBackfillActionTest.kt index f616910d5..416b7dab7 100644 --- a/service/src/test/kotlin/app/cash/backfila/actions/StartStopBackfillActionTest.kt +++ b/service/src/test/kotlin/app/cash/backfila/actions/StartStopBackfillActionTest.kt @@ -3,6 +3,7 @@ package app.cash.backfila.actions import app.cash.backfila.BackfilaTestingModule import app.cash.backfila.api.ConfigureServiceAction import app.cash.backfila.client.Connectors +import app.cash.backfila.client.FakeBackfilaClientServiceClient import app.cash.backfila.dashboard.CreateBackfillAction import app.cash.backfila.dashboard.GetBackfillRunsAction import app.cash.backfila.dashboard.GetBackfillStatusAction @@ -17,9 +18,10 @@ import app.cash.backfila.service.persistence.BackfilaDb import app.cash.backfila.service.persistence.BackfillPartitionState import app.cash.backfila.service.persistence.BackfillState import app.cash.backfila.service.persistence.DbBackfillRun +import app.cash.backfila.service.persistence.DbEventLog +import app.cash.backfila.softAssert +import app.cash.backfila.uiEventLogWith import com.google.inject.Module -import javax.inject.Inject -import kotlin.test.assertNotNull import misk.exceptions.BadRequestException import misk.hibernate.Id import misk.hibernate.Query @@ -28,128 +30,162 @@ import misk.hibernate.load import misk.scope.ActionScope import misk.testing.MiskTest import misk.testing.MiskTestModule +import org.assertj.core.api.Assertions import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test +import javax.inject.Inject +import kotlin.test.assertNotNull @MiskTest(startService = true) class StartStopBackfillActionTest { @Suppress("unused") - @MiskTestModule - val module: Module = BackfilaTestingModule() + @MiskTestModule val module: Module = BackfilaTestingModule() - @Inject - lateinit var configureServiceAction: ConfigureServiceAction + @Inject lateinit var configureServiceAction: ConfigureServiceAction - @Inject - lateinit var createBackfillAction: CreateBackfillAction + @Inject lateinit var createBackfillAction: CreateBackfillAction - @Inject - lateinit var startBackfillAction: StartBackfillAction + @Inject lateinit var startBackfillAction: StartBackfillAction - @Inject - lateinit var stopBackfillAction: StopBackfillAction + @Inject lateinit var stopBackfillAction: StopBackfillAction - @Inject - lateinit var getBackfillRunsAction: GetBackfillRunsAction + @Inject lateinit var getBackfillRunsAction: GetBackfillRunsAction - @Inject - lateinit var getBackfillStatusAction: GetBackfillStatusAction + @Inject lateinit var getBackfillStatusAction: GetBackfillStatusAction - @Inject - lateinit var queryFactory: Query.Factory + @Inject lateinit var queryFactory: Query.Factory - @Inject - lateinit var scope: ActionScope + @Inject lateinit var scope: ActionScope - @Inject - @BackfilaDb - lateinit var transacter: Transacter + @Inject @BackfilaDb lateinit var transacter: Transacter - @Test - fun startAndStop() { + @Inject lateinit var fakeBackfilaClientServiceClient: FakeBackfilaClientServiceClient + + @BeforeEach + fun setup() { + // Always configure the backfills before any test scope.fakeCaller(service = "deep-fryer") { configureServiceAction.configureService( ConfigureServiceRequest.Builder() .backfills( listOf( - ConfigureServiceRequest.BackfillData( - "ChickenSandwich", "Description", listOf(), null, - null, false - ) + ConfigureServiceRequest.BackfillData.Builder() + .name("ChickenSandwich") + .description("Description") + .build(), + ConfigureServiceRequest.BackfillData.Builder() + .name("BeefSandwich") + .description("Description") + .build() ) ) .connector_type(Connectors.ENVOY) .build() ) } + } + + @Test fun `Backfila starts with no backfills`() { scope.fakeCaller(user = "molly") { var backfillRuns = getBackfillRunsAction.backfillRuns("deep-fryer") - assertThat(backfillRuns.paused_backfills).hasSize(0) - assertThat(backfillRuns.running_backfills).hasSize(0) + softAssert { + assertThat(backfillRuns.paused_backfills).hasSize(0) + assertThat(backfillRuns.running_backfills).hasSize(0) + } + } + } - val response = createBackfillAction.create( - "deep-fryer", - CreateBackfillRequest.Builder() - .backfill_name("ChickenSandwich") - .build() - ) + @Nested inner class `After a backfill is created` { + var createdBackfillRunId: Long = 0 // lateinit won't work on primitives - backfillRuns = getBackfillRunsAction.backfillRuns("deep-fryer") - assertThat(backfillRuns.paused_backfills).hasSize(1) - assertThat(backfillRuns.running_backfills).hasSize(0) + @BeforeEach fun createBackfill() { + scope.fakeCaller(user = "molly") { + val response = createBackfillAction.create( + "deep-fryer", + CreateBackfillRequest.Builder() + .backfill_name("ChickenSandwich") + .build() + ) + createdBackfillRunId = response.backfill_run_id + } + } - val id = response.backfill_run_id - assertThat(backfillRuns.paused_backfills[0].id).isEqualTo(id.toString()) - startBackfillAction.start(id, StartBackfillRequest()) + @Test fun `The backfill starts out paused`() { + scope.fakeCaller(user = "molly") { + var backfillRuns = getBackfillRunsAction.backfillRuns("deep-fryer") + softAssert { + assertThat(backfillRuns.paused_backfills).hasSize(1) + assertThat(backfillRuns.running_backfills).hasSize(0) + assertThat(backfillRuns.paused_backfills).singleElement().extracting { it.id } + .isEqualTo(createdBackfillRunId.toString()) + } + } + } + + @Nested inner class `And then the backfill is started` { + @BeforeEach fun startBackfill() { + scope.fakeCaller(user = "molly") { + startBackfillAction.start(createdBackfillRunId, StartBackfillRequest()) + } + } + + @Test fun `the Backfill is running`() { + softAssert { + var status = getBackfillStatusAction.status(createdBackfillRunId) + assertThat(status.state).isEqualTo(BackfillState.RUNNING) + assertThat(status.partitions).extracting { it.state } + .containsOnly(BackfillPartitionState.RUNNING) + + assertThat(status.event_logs).first().`is`( + uiEventLogWith( + type = DbEventLog.Type.STATE_CHANGE, + user = "molly", + message = "backfill started" + ) + ) + + val backfillRuns = getBackfillRunsAction.backfillRuns("deep-fryer") + assertThat(backfillRuns.paused_backfills).hasSize(0) + assertThat(backfillRuns.running_backfills).hasSize(1) + } + } + + @Nested inner class `And then the backfill is stopped` { + @BeforeEach fun stopBackfill() { + scope.fakeCaller(user = "molly") { + stopBackfillAction.stop(createdBackfillRunId, StopBackfillRequest()) + } + } - var status = getBackfillStatusAction.status(id) - assertThat(status.state).isEqualTo(BackfillState.RUNNING) - assertThat(status.partitions.map { it.state }) - .containsOnly(BackfillPartitionState.RUNNING) - assertThat(status.event_logs[0].message).isEqualTo("backfill started") - assertThat(status.event_logs[0].user).isEqualTo("molly") - - backfillRuns = getBackfillRunsAction.backfillRuns("deep-fryer") - assertThat(backfillRuns.paused_backfills).hasSize(0) - assertThat(backfillRuns.running_backfills).hasSize(1) - - stopBackfillAction.stop(id, StopBackfillRequest()) - - status = getBackfillStatusAction.status(id) - assertThat(status.state).isEqualTo(BackfillState.PAUSED) - assertThat(status.partitions.map { it.state }) - .containsOnly(BackfillPartitionState.PAUSED) - assertThat(status.event_logs[0].message).isEqualTo("backfill stopped") - assertThat(status.event_logs[0].user).isEqualTo("molly") - - backfillRuns = getBackfillRunsAction.backfillRuns("deep-fryer") - assertThat(backfillRuns.paused_backfills).hasSize(1) - assertThat(backfillRuns.running_backfills).hasSize(0) + @Test fun `the Backfill is stopped`() { + softAssert { + val status = getBackfillStatusAction.status(createdBackfillRunId) + assertThat(status.state).describedAs("state").isEqualTo(BackfillState.PAUSED) + assertThat(status.partitions.map { it.state }) + .containsOnly(BackfillPartitionState.PAUSED) + + Assertions.assertThat(status.event_logs).first().`is`( + uiEventLogWith( + type = DbEventLog.Type.STATE_CHANGE, + user = "molly", + message = "backfill stopped" + ) + ) + + val backfillRuns = getBackfillRunsAction.backfillRuns("deep-fryer") + assertThat(backfillRuns.paused_backfills).hasSize(1) + assertThat(backfillRuns.running_backfills).hasSize(0) + } + } + } } } @Test fun pagination() { - scope.fakeCaller(service = "deep-fryer") { - configureServiceAction.configureService( - ConfigureServiceRequest.Builder() - .backfills( - listOf( - ConfigureServiceRequest.BackfillData.Builder() - .name("ChickenSandwich") - .description("Description") - .build(), - ConfigureServiceRequest.BackfillData.Builder() - .name("BeefSandwich") - .description("Description") - .build() - ) - ) - .connector_type(Connectors.ENVOY) - .build() - ) - } scope.fakeCaller(user = "molly") { repeat(15) { createBackfillAction.create( @@ -165,34 +201,19 @@ class StartStopBackfillActionTest { .build() ) } - val backfillRuns = getBackfillRunsAction.backfillRuns("deep-fryer") - assertThat(backfillRuns.paused_backfills).hasSize(20) + val backfillRunsPage1 = getBackfillRunsAction.backfillRuns("deep-fryer") + assertThat(backfillRunsPage1.paused_backfills).hasSize(20) val backfillRunsPage2 = getBackfillRunsAction.backfillRuns( "deep-fryer", - pagination_token = backfillRuns.next_pagination_token + pagination_token = backfillRunsPage1.next_pagination_token ) assertThat(backfillRunsPage2.paused_backfills).hasSize(10) } } @Test - fun backfillDoesntExist() { - scope.fakeCaller(service = "deep-fryer") { - configureServiceAction.configureService( - ConfigureServiceRequest.Builder() - .backfills( - listOf( - ConfigureServiceRequest.BackfillData( - "ChickenSandwich", "Description", listOf(), null, - null, false - ) - ) - ) - .connector_type(Connectors.ENVOY) - .build() - ) - } + fun `an incorrect backfill does not exist`() { scope.fakeCaller(user = "molly") { val response = createBackfillAction.create( "deep-fryer", @@ -200,31 +221,16 @@ class StartStopBackfillActionTest { .backfill_name("ChickenSandwich") .build() ) - val id = response.backfill_run_id + val incorrectId = response.backfill_run_id + 1 assertThatThrownBy { - startBackfillAction.start(id + 1, StartBackfillRequest()) + startBackfillAction.start(incorrectId, StartBackfillRequest()) }.isInstanceOf(BadRequestException::class.java) } } @Test fun cantStartRunningBackfill() { - scope.fakeCaller(service = "deep-fryer") { - configureServiceAction.configureService( - ConfigureServiceRequest.Builder() - .backfills( - listOf( - ConfigureServiceRequest.BackfillData( - "ChickenSandwich", "Description", listOf(), null, - null, false - ) - ) - ) - .connector_type(Connectors.ENVOY) - .build() - ) - } scope.fakeCaller(user = "molly") { val response = createBackfillAction.create( "deep-fryer", @@ -249,21 +255,6 @@ class StartStopBackfillActionTest { @Test fun cantStopPausedBackfill() { - scope.fakeCaller(service = "deep-fryer") { - configureServiceAction.configureService( - ConfigureServiceRequest.Builder() - .backfills( - listOf( - ConfigureServiceRequest.BackfillData( - "ChickenSandwich", "Description", listOf(), null, - null, false - ) - ) - ) - .connector_type(Connectors.ENVOY) - .build() - ) - } scope.fakeCaller(user = "molly") { val response = createBackfillAction.create( "deep-fryer", @@ -280,21 +271,6 @@ class StartStopBackfillActionTest { @Test fun cantToggleCompletedBackfill() { - scope.fakeCaller(service = "deep-fryer") { - configureServiceAction.configureService( - ConfigureServiceRequest.Builder() - .backfills( - listOf( - ConfigureServiceRequest.BackfillData( - "ChickenSandwich", "Description", listOf(), null, - null, false - ) - ) - ) - .connector_type(Connectors.ENVOY) - .build() - ) - } scope.fakeCaller(user = "molly") { val response = createBackfillAction.create( "deep-fryer", diff --git a/service/src/test/kotlin/app/cash/backfila/client/FakeBackfilaClientServiceClient.kt b/service/src/test/kotlin/app/cash/backfila/client/FakeBackfilaClientServiceClient.kt index c3de99509..807f82dab 100644 --- a/service/src/test/kotlin/app/cash/backfila/client/FakeBackfilaClientServiceClient.kt +++ b/service/src/test/kotlin/app/cash/backfila/client/FakeBackfilaClientServiceClient.kt @@ -1,5 +1,8 @@ package app.cash.backfila.client +import app.cash.backfila.client.FakeBackfilaClientServiceClient.Companion.ResponseBehaviour.FAILURE +import app.cash.backfila.client.FakeBackfilaClientServiceClient.Companion.ResponseBehaviour.SUCCESS +import app.cash.backfila.client.FakeBackfilaClientServiceClient.Companion.ResponseBehaviour.TO_CHANNEL import app.cash.backfila.protos.clientservice.GetNextBatchRangeRequest import app.cash.backfila.protos.clientservice.GetNextBatchRangeResponse import app.cash.backfila.protos.clientservice.KeyRange @@ -11,6 +14,7 @@ import java.util.LinkedList import javax.inject.Inject import javax.inject.Singleton import kotlinx.coroutines.channels.Channel +import misk.exceptions.BadRequestException import okio.ByteString.Companion.encodeUtf8 @Singleton @@ -25,12 +29,30 @@ class FakeBackfilaClientServiceClient @Inject constructor() : BackfilaClientServ /** Send responses or exceptions here to return them to the runner. */ val runBatchResponses = Channel>() + // Storing our response behavior for our two partitions and the default. This allows one partition + // to make progress ahead of the other. + val eightyDashBehaviour = PartitionBehaviour(TO_CHANNEL, TO_CHANNEL) + val dashEightyBehaviour = PartitionBehaviour(TO_CHANNEL, TO_CHANNEL) + val defaultBehaviour = PartitionBehaviour(TO_CHANNEL, TO_CHANNEL) + + private fun getPartitionBehaviour(partitionName: String): PartitionBehaviour { + return when (partitionName) { + DASH_EIGHTY -> dashEightyBehaviour + EIGHTY_DASH -> eightyDashBehaviour + else -> defaultBehaviour + } + } + fun dontBlockGetNextBatch() { - getNextBatchRangeRequests.close() + eightyDashBehaviour.nextBatch = SUCCESS + dashEightyBehaviour.nextBatch = SUCCESS + defaultBehaviour.nextBatch = SUCCESS } fun dontBlockRunBatch() { - runBatchRequests.close() + eightyDashBehaviour.runBatch = SUCCESS + dashEightyBehaviour.runBatch = SUCCESS + defaultBehaviour.runBatch = SUCCESS } override fun prepareBackfill(request: PrepareBackfillRequest): PrepareBackfillResponse { @@ -41,12 +63,12 @@ class FakeBackfilaClientServiceClient @Inject constructor() : BackfilaClientServ .partitions( listOf( PrepareBackfillResponse.Partition( - "-80", + DASH_EIGHTY, KeyRange("0".encodeUtf8(), "1000".encodeUtf8()), 1_000_000L ), PrepareBackfillResponse.Partition( - "80-", + EIGHTY_DASH, KeyRange("0".encodeUtf8(), "1000".encodeUtf8()), null ) @@ -57,38 +79,69 @@ class FakeBackfilaClientServiceClient @Inject constructor() : BackfilaClientServ override suspend fun getNextBatchRange(request: GetNextBatchRangeRequest): GetNextBatchRangeResponse { - if (!getNextBatchRangeRequests.isClosedForSend) { - getNextBatchRangeRequests.send(request) - return getNextBatchRangeResponses.receive().getOrThrow() + when (getPartitionBehaviour(request.partition_name).nextBatch) { + TO_CHANNEL -> { + getNextBatchRangeRequests.send(request) + return getNextBatchRangeResponses.receive().getOrThrow() + } + FAILURE -> { + throw BadRequestException("Forced Test GetNextBatchRange Failure") + } + SUCCESS -> { + val nextStart = if (request.previous_end_key != null) { + request.previous_end_key.utf8().toLong() + 1 + } else { + request.backfill_range.start.utf8().toLong() + } + var nextEnd = nextStart + request.batch_size - 1 + if (nextEnd > request.backfill_range.end.utf8().toLong()) { + nextEnd = request.backfill_range.end.utf8().toLong() + } + if (nextStart > request.backfill_range.end.utf8().toLong()) { + return GetNextBatchRangeResponse(listOf()) + } + return GetNextBatchRangeResponse( + listOf( + GetNextBatchRangeResponse.Batch( + KeyRange(nextStart.toString().encodeUtf8(), nextEnd.toString().encodeUtf8()), + nextEnd - nextStart + 1, + nextEnd - nextStart + 1 + ) + ) + ) + } } - val nextStart = if (request.previous_end_key != null) { - request.previous_end_key.utf8().toLong() + 1 - } else { - request.backfill_range.start.utf8().toLong() + } + + override suspend fun runBatch(request: RunBatchRequest): RunBatchResponse { + when (getPartitionBehaviour(request.partition_name).runBatch) { + TO_CHANNEL -> { + runBatchRequests.send(request) + return runBatchResponses.receive().getOrThrow() } - var nextEnd = nextStart + request.batch_size - 1 - if (nextEnd > request.backfill_range.end.utf8().toLong()) { - nextEnd = request.backfill_range.end.utf8().toLong() + FAILURE -> { + throw BadRequestException("Forced Test RunBatch Failure") } - if (nextStart > request.backfill_range.end.utf8().toLong()) { - return GetNextBatchRangeResponse(listOf()) + SUCCESS -> { + return RunBatchResponse.Builder().build() } - return GetNextBatchRangeResponse( - listOf( - GetNextBatchRangeResponse.Batch( - KeyRange(nextStart.toString().encodeUtf8(), nextEnd.toString().encodeUtf8()), - nextEnd - nextStart + 1, - nextEnd - nextStart + 1 - ) - ) - ) } + } - override suspend fun runBatch(request: RunBatchRequest): RunBatchResponse { - if (runBatchRequests.isClosedForSend) { - return RunBatchResponse.Builder().build() + companion object { + // Partition constants + val DASH_EIGHTY = "-80" + val EIGHTY_DASH = "80-" + + data class PartitionBehaviour( + var nextBatch: ResponseBehaviour, + var runBatch: ResponseBehaviour + ) + + enum class ResponseBehaviour { + TO_CHANNEL, + SUCCESS, + FAILURE } - runBatchRequests.send(request) - return runBatchResponses.receive().getOrThrow() } }