Skip to content

Commit

Permalink
Fix builds paging when models param is used (#163)
Browse files Browse the repository at this point in the history
Preserve the `models` param across `/api/builds` requests of
`getBuildsFlow`. Add more integration tests.

When the `models` param is used with `getBuildsFlow`, requests after the
first one (paging) don't send the `models` param, but they should. This
is the same issue faced with `query` (#134), due to this extension being
manually implemented. More integration test coverage should help.
  • Loading branch information
gabrielfeo authored Mar 28, 2024
1 parent 379a050 commit 611d03f
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 8 deletions.
4 changes: 4 additions & 0 deletions library/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,10 @@ tasks.named("check") {
dependsOn("integrationTest")
}

tasks.named<Test>("integrationTest") {
jvmArgs("-Xmx512m")
}

java {
consistentResolution {
useRuntimeClasspathVersions()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,86 @@ package com.gabrielfeo.gradle.enterprise.api.extension

import com.gabrielfeo.gradle.enterprise.api.*
import com.gabrielfeo.gradle.enterprise.api.internal.*
import com.gabrielfeo.gradle.enterprise.api.model.BuildModelName
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.take
import kotlinx.coroutines.test.runTest
import okhttp3.Request
import org.junit.jupiter.api.*
import org.junit.jupiter.api.Assertions.*
import kotlin.test.AfterTest
import kotlin.time.Duration.Companion.minutes

class BuildsApiExtensionsIntegrationTest {

@Test
fun getBuildsFlowUsesQueryInAllRequests() = runTest {
init {
env = RealEnv
keychain = RealKeychain(RealSystemProperties)
val recorder = RequestRecorder()
val api = buildApi(recorder)
api.buildsApi.getBuildsFlow(since = 0, query = "user:*").take(2000).collect()
recorder.requests.forEachIndexed { i, request ->
assertEquals("user:*", request.url.queryParameter("query"), "[$i]")
}
}

private val recorder = RequestRecorder()
private val api = buildApi(recorder)

@AfterTest
fun setup() {
api.shutdown()
}

@Test
fun getBuildsFlowPreservesParamsAcrossRequests() = runTest(timeout = 3.minutes) {
api.buildsApi.getBuildsFlow(
since = 0,
query = "user:*",
models = listOf(BuildModelName.gradleAttributes),
reverse = true,
).take(2000).collect()
recorder.requests.forEach {
assertUrlParam(it, "query", "user:*")
assertUrlParam(it, "models", "gradle-attributes")
assertUrlParam(it, "reverse", "true")
}
}

@Test
fun getBuildsFlowReplacesSinceForFromBuildAfterFirstRequest() = runTest {
api.buildsApi.getBuildsFlow(since = 1).take(2000).collect()
assertReplacedForFromBuildAfterFirstRequest(param = "since" to "1")
}

@Test
fun getBuildsFlowReplacesFromInstantForFromBuildAfterFirstRequest() = runTest {
api.buildsApi.getBuildsFlow(fromInstant = 1).take(2000).collect()
assertReplacedForFromBuildAfterFirstRequest(param = "fromInstant" to "1")
}

private fun assertReplacedForFromBuildAfterFirstRequest(param: Pair<String, String>) {
with(recorder.requests) {
val (key, value) = param
first().let {
assertUrlParam(it, key, value)
assertUrlParam(it, "fromBuild", null)
}
(this - first()).forEach {
assertUrlParam(it, key, null)
assertUrlParamNotNull(it, "fromBuild")
}
}
}

private fun buildApi(recorder: RequestRecorder) =
GradleEnterpriseApi.newInstance(
config = Config(
clientBuilder = recorder.clientBuilder(),
cacheConfig = Config.CacheConfig(cacheEnabled = false),
)
)

private fun assertUrlParam(request: Request, key: String, expected: String?) {
val actual = request.url.queryParameter(key)
assertEquals(expected, actual, "Expected '$key='$expected', but was '$key=$actual' (${request.url})")
}

private fun assertUrlParamNotNull(request: Request, key: String) {
assertNotNull(request.url.queryParameter(key), "Expected param $key, but was null (${request.url})")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ fun BuildsApi.getBuildsFlow(
reverse = reverse,
maxWaitSecs = maxWaitSecs,
maxBuilds = buildsPerPage,
models = models,
)
emitAll(builds.asFlow())
}
Expand Down

0 comments on commit 611d03f

Please sign in to comment.