Skip to content

Commit 611d03f

Browse files
authored
Fix builds paging when models param is used (#163)
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.
1 parent 379a050 commit 611d03f

File tree

3 files changed

+67
-8
lines changed

3 files changed

+67
-8
lines changed

library/build.gradle.kts

+4
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,10 @@ tasks.named("check") {
227227
dependsOn("integrationTest")
228228
}
229229

230+
tasks.named<Test>("integrationTest") {
231+
jvmArgs("-Xmx512m")
232+
}
233+
230234
java {
231235
consistentResolution {
232236
useRuntimeClasspathVersions()

library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensionsIntegrationTest.kt

+62-8
Original file line numberDiff line numberDiff line change
@@ -2,32 +2,86 @@ package com.gabrielfeo.gradle.enterprise.api.extension
22

33
import com.gabrielfeo.gradle.enterprise.api.*
44
import com.gabrielfeo.gradle.enterprise.api.internal.*
5+
import com.gabrielfeo.gradle.enterprise.api.model.BuildModelName
56
import kotlinx.coroutines.flow.collect
67
import kotlinx.coroutines.flow.take
78
import kotlinx.coroutines.test.runTest
9+
import okhttp3.Request
810
import org.junit.jupiter.api.*
911
import org.junit.jupiter.api.Assertions.*
12+
import kotlin.test.AfterTest
13+
import kotlin.time.Duration.Companion.minutes
1014

1115
class BuildsApiExtensionsIntegrationTest {
1216

13-
@Test
14-
fun getBuildsFlowUsesQueryInAllRequests() = runTest {
17+
init {
1518
env = RealEnv
1619
keychain = RealKeychain(RealSystemProperties)
17-
val recorder = RequestRecorder()
18-
val api = buildApi(recorder)
19-
api.buildsApi.getBuildsFlow(since = 0, query = "user:*").take(2000).collect()
20-
recorder.requests.forEachIndexed { i, request ->
21-
assertEquals("user:*", request.url.queryParameter("query"), "[$i]")
22-
}
20+
}
21+
22+
private val recorder = RequestRecorder()
23+
private val api = buildApi(recorder)
24+
25+
@AfterTest
26+
fun setup() {
2327
api.shutdown()
2428
}
2529

30+
@Test
31+
fun getBuildsFlowPreservesParamsAcrossRequests() = runTest(timeout = 3.minutes) {
32+
api.buildsApi.getBuildsFlow(
33+
since = 0,
34+
query = "user:*",
35+
models = listOf(BuildModelName.gradleAttributes),
36+
reverse = true,
37+
).take(2000).collect()
38+
recorder.requests.forEach {
39+
assertUrlParam(it, "query", "user:*")
40+
assertUrlParam(it, "models", "gradle-attributes")
41+
assertUrlParam(it, "reverse", "true")
42+
}
43+
}
44+
45+
@Test
46+
fun getBuildsFlowReplacesSinceForFromBuildAfterFirstRequest() = runTest {
47+
api.buildsApi.getBuildsFlow(since = 1).take(2000).collect()
48+
assertReplacedForFromBuildAfterFirstRequest(param = "since" to "1")
49+
}
50+
51+
@Test
52+
fun getBuildsFlowReplacesFromInstantForFromBuildAfterFirstRequest() = runTest {
53+
api.buildsApi.getBuildsFlow(fromInstant = 1).take(2000).collect()
54+
assertReplacedForFromBuildAfterFirstRequest(param = "fromInstant" to "1")
55+
}
56+
57+
private fun assertReplacedForFromBuildAfterFirstRequest(param: Pair<String, String>) {
58+
with(recorder.requests) {
59+
val (key, value) = param
60+
first().let {
61+
assertUrlParam(it, key, value)
62+
assertUrlParam(it, "fromBuild", null)
63+
}
64+
(this - first()).forEach {
65+
assertUrlParam(it, key, null)
66+
assertUrlParamNotNull(it, "fromBuild")
67+
}
68+
}
69+
}
70+
2671
private fun buildApi(recorder: RequestRecorder) =
2772
GradleEnterpriseApi.newInstance(
2873
config = Config(
2974
clientBuilder = recorder.clientBuilder(),
3075
cacheConfig = Config.CacheConfig(cacheEnabled = false),
3176
)
3277
)
78+
79+
private fun assertUrlParam(request: Request, key: String, expected: String?) {
80+
val actual = request.url.queryParameter(key)
81+
assertEquals(expected, actual, "Expected '$key='$expected', but was '$key=$actual' (${request.url})")
82+
}
83+
84+
private fun assertUrlParamNotNull(request: Request, key: String) {
85+
assertNotNull(request.url.queryParameter(key), "Expected param $key, but was null (${request.url})")
86+
}
3387
}

library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensions.kt

+1
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ fun BuildsApi.getBuildsFlow(
5454
reverse = reverse,
5555
maxWaitSecs = maxWaitSecs,
5656
maxBuilds = buildsPerPage,
57+
models = models,
5758
)
5859
emitAll(builds.asFlow())
5960
}

0 commit comments

Comments
 (0)