test(streamable-http): add missing integration tests for pagination, bad request, and logging by jskjw157 · Pull Request #613 · modelcontextprotocol/kotlin-sdk

jskjw157 · 2026-03-22T07:32:24Z

This PR adds the remaining non-security integration tests for the Streamable HTTP transport as outlined in #183 and left over from #486.

Changes included:

Pagination: Added cursor-based pagination tests for Prompts, Resources, and Tools to Abstract*IntegrationTest.
Bad Request: Added tests to verify that invalid cursor parameters correctly throw an McpException with INTERNAL_ERROR.
Logging: Added LoggingIntegrationTestStreamableHttp to specifically test setLevel and logging message notifications over Streamable HTTP.

Note: Security (Allow/Deny) tests are intentionally excluded as discussed in #486, pending further server auth model implementation.

Copilot

Pull request overview

Adds missing Streamable HTTP integration coverage to close remaining non-security scenarios for listing pagination, invalid cursor handling, and logging message delivery/filtering.

Changes:

Add cursor-based pagination integration tests for prompts/resources/tools list endpoints.
Add invalid cursor (“bad request”) tests asserting server exceptions propagate as McpException with INTERNAL_ERROR.
Add Streamable HTTP–specific logging integration tests for setLoggingLevel and log message notifications.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/streamablehttp/LoggingIntegrationTestStreamableHttp.kt	New Streamable HTTP logging integration tests (notifications + level filtering).
integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt	Adds tools list pagination + invalid cursor integration coverage.
integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt	Adds resources list pagination + invalid cursor integration coverage.
integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractPromptIntegrationTest.kt	Adds prompts list pagination + invalid cursor integration coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-22T07:35:19Z

.../kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt

+                val all = server.tools.values.map { it.tool }
+                val cursor = request.cursor?.toIntOrNull() ?: 0
+                val pageSize = 2
+                val page = all.drop(cursor).take(pageSize)
+                val next = if (cursor + page.size < all.size) (cursor + page.size).toString() else null
+                ListToolsResult(tools = page, nextCursor = next)
+            }
+        }
+
+        val first = client.listTools()
+        assertTrue(first.tools.isNotEmpty())
+        val next = first.nextCursor
+        assertNotNull(next)
+
+        val second = client.listTools(ListToolsRequest(PaginatedRequestParams(cursor = next)))
+        assertTrue(second.tools.isNotEmpty())
+
+        val combinedNames = (first.tools + second.tools).map { it.name }
+        assertTrue(combinedNames.any { it.startsWith(prefix) })
+    }


The pagination handler builds all from server.tools.values, which comes from an immutable persistent map (no guaranteed iteration order). Because the test only fetches two pages, the newly added paginated-tool-* entries may not appear in those pages, making this test potentially flaky. Consider sorting all by tool name and/or filtering to the prefix tools (or paging until nextCursor == null) so the assertions are deterministic.

Copilot · 2026-03-22T07:35:20Z

.../kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt

+        val exception = org.junit.jupiter.api.assertThrows<io.modelcontextprotocol.kotlin.sdk.types.McpException> {
+            runBlocking {
+                client.listTools(ListToolsRequest(PaginatedRequestParams(cursor = "bad")))
+            }


This test is already running inside runBlocking(Dispatchers.IO); wrapping the suspending client.listTools(...) call in an additional runBlocking { ... } inside assertThrows is redundant and can make failures/hangs harder to diagnose. Prefer using a suspending assertion (e.g., assertFailsWith) inside the existing runBlocking, or drop the outer runBlocking and keep a single runBlocking inside assertThrows.

Copilot · 2026-03-22T07:35:20Z

...lin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt

+        val first = client.listResources()
+        assertTrue(first.resources.isNotEmpty())
+        val next = first.nextCursor
+        assertNotNull(next)
+
+        val second = client.listResources(ListResourcesRequest(PaginatedRequestParams(cursor = next)))
+        assertTrue(second.resources.isNotEmpty())
+
+        val combinedUris = (first.resources + second.resources).map { it.uri }


The pagination handler uses server.resources.values (backed by a persistent map) as the source list. Iteration order isn’t guaranteed, and since the test only fetches two pages, the paginated-resource-* entries might not land in those pages, which can make the test flaky. Sort/filter the resources list (e.g., by uri/name) and/or page until nextCursor == null before asserting.

Suggested change

val first = client.listResources()

assertTrue(first.resources.isNotEmpty())

val next = first.nextCursor

assertNotNull(next)

val second = client.listResources(ListResourcesRequest(PaginatedRequestParams(cursor = next)))

assertTrue(second.resources.isNotEmpty())

val combinedUris = (first.resources + second.resources).map { it.uri }

val combinedUris = mutableListOf<String>()

var cursor: String? = null

do {

val request = if (cursor == null) {

ListResourcesRequest()

} else {

ListResourcesRequest(PaginatedRequestParams(cursor = cursor))

}

val response = client.listResources(request)

combinedUris += response.resources.map { it.uri }

cursor = response.nextCursor

} while (cursor != null)

assertTrue(combinedUris.isNotEmpty())

Copilot · 2026-03-22T07:35:20Z

...lin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt

+        val exception = org.junit.jupiter.api.assertThrows<McpException> {
+            runBlocking {
+                client.listResources(ListResourcesRequest(PaginatedRequestParams(cursor = "bad")))
+            }


This test runs in runBlocking(Dispatchers.IO) already; the additional nested runBlocking { client.listResources(...) } inside assertThrows is unnecessary. Consider using a suspending assertion within the current runBlocking scope, or remove the outer runBlocking and keep a single runBlocking around the suspending call.

Copilot · 2026-03-22T07:35:20Z

...otlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractPromptIntegrationTest.kt

+        val nextCursor = first.nextCursor
+        assertNotNull(nextCursor)
+
+        val second = client.listPrompts(ListPromptsRequest(PaginatedRequestParams(cursor = nextCursor)))
+        assertTrue(second.prompts.isNotEmpty())
+
+        val combined = first.prompts + second.prompts
+        assertTrue(combined.any { it.name.startsWith(pagePrefix) })


all is built from server.prompts.values (persistent map iteration order is not guaranteed). Because the test only fetches two pages, the added paginated-prompt-* prompts may not be present in first + second, making this test potentially flaky. Sort/filter the prompts list (e.g., by name) and/or iterate through pages until nextCursor == null before asserting.

Suggested change

val nextCursor = first.nextCursor

assertNotNull(nextCursor)

val second = client.listPrompts(ListPromptsRequest(PaginatedRequestParams(cursor = nextCursor)))

assertTrue(second.prompts.isNotEmpty())

val combined = first.prompts + second.prompts

assertTrue(combined.any { it.name.startsWith(pagePrefix) })

val allPrompts = mutableListOf<io.modelcontextprotocol.kotlin.sdk.types.Prompt>()

allPrompts.addAll(first.prompts)

var cursor = first.nextCursor

while (cursor != null) {

val page = client.listPrompts(ListPromptsRequest(PaginatedRequestParams(cursor = cursor)))

allPrompts.addAll(page.prompts)

cursor = page.nextCursor

}

assertTrue(allPrompts.any { it.name.startsWith(pagePrefix) })

Copilot · 2026-03-22T07:35:21Z

...otlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractPromptIntegrationTest.kt

+        val exception = org.junit.jupiter.api.assertThrows<McpException> {
+            runBlocking {
+                client.listPrompts(ListPromptsRequest(PaginatedRequestParams(cursor = "not-a-number")))
+            }


This test already uses runBlocking(Dispatchers.IO); the nested runBlocking { client.listPrompts(...) } inside assertThrows is redundant. Prefer a single runBlocking (either outer or inner) and a suspending assertion so the test structure is simpler and avoids blocking an extra thread.

Copilot · 2026-03-22T07:35:21Z

...rotocol/kotlin/sdk/integration/kotlin/streamablehttp/LoggingIntegrationTestStreamableHttp.kt

+        }
+
+        server.addTool(name = "test-logging-level", description = "test") { request ->
+            LoggingLevel.values().forEach { level ->


LoggingLevel.values() allocates a new array each call; elsewhere in the codebase tests use LoggingLevel.entries, which is allocation-free and the preferred Kotlin API. Consider switching to entries for consistency and to avoid unnecessary allocations.

Suggested change

LoggingLevel.values().forEach { level ->

LoggingLevel.entries.forEach { level ->

codecov-commenter · 2026-03-26T10:40:54Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
3062	1	3061	50

View the top 1 failed test(s) by shortest run time

io.modelcontextprotocol.kotlin.sdk.testing.ChannelTransportTest::should connect and list resources()[jvm]

Stack Traces | 120s run time

java.util.concurrent.TimeoutException: should connect and list resources() timed out after 2 minutes
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:387)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.tryRemoveAndExec(ForkJoinPool.java:1351)
	at java.base/java.util.concurrent.ForkJoinTask.awaitDone(ForkJoinTask.java:422)
	at java.base/java.util.concurrent.ForkJoinTask.join(ForkJoinTask.java:651)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:387)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1312)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1843)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1808)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:188)
	Suppressed: java.lang.InterruptedException
		at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:98)
		at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:70)
		at kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source)
		at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:48)
		at kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source)
		at io.modelcontextprotocol.kotlin.sdk.testing.ChannelTransportTest.should connect and list resources(ChannelTransportTest.kt:29)
		at java.base/java.lang.reflect.Method.invoke(Method.java:580)
		... 9 more

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

devcrocod

left a few comments

devcrocod · 2026-03-26T12:10:35Z

...rotocol/kotlin/sdk/integration/kotlin/streamablehttp/LoggingIntegrationTestStreamableHttp.kt

+        kotlin.test.assertEquals(LoggingLevel.Info, received.params.level)
+        kotlin.test.assertEquals(JsonPrimitive("test-data-sample"), received.params.data)


devcrocod · 2026-03-26T12:12:58Z

...rotocol/kotlin/sdk/integration/kotlin/streamablehttp/LoggingIntegrationTestStreamableHttp.kt

Doesn’t this duplicate ClientConnectionLoggingTest?

devcrocod · 2026-03-26T12:14:04Z

...rotocol/kotlin/sdk/integration/kotlin/streamablehttp/LoggingIntegrationTestStreamableHttp.kt

+        kotlin.test.assertEquals(LoggingLevel.Info, received.params.level)
+        kotlin.test.assertEquals(JsonObject(mapOf("key" to JsonPrimitive("value"))), received.params.data)


devcrocod · 2026-03-26T12:14:16Z

...rotocol/kotlin/sdk/integration/kotlin/streamablehttp/LoggingIntegrationTestStreamableHttp.kt

+        kotlin.test.assertEquals(expectedLevels.size, receivedMessages.size)
+        kotlin.test.assertEquals(expectedLevels.toList(), receivedMessages.map { it.params.level })


devcrocod · 2026-03-26T12:14:25Z

...lin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt

+            }
+        }
+
+        val exception = kotlin.test.assertFailsWith<McpException> {


devcrocod · 2026-03-26T12:14:31Z

.../kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt

+            }
+        }
+
+        val exception = kotlin.test.assertFailsWith<McpException> {


devcrocod · 2026-03-26T12:15:23Z

...otlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractPromptIntegrationTest.kt

+            }
+        }
+
+        val allPrompts = mutableListOf<io.modelcontextprotocol.kotlin.sdk.types.Prompt>()


devcrocod · 2026-03-26T12:16:34Z

...otlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractPromptIntegrationTest.kt

+            currentCursor = response.nextCursor
+        } while (currentCursor != null)
+
+        assertTrue(allPrompts.any { it.name.startsWith(pagePrefix) })


why not all?

…bad request, and logging - Add cursor-based pagination tests for Prompts, Resources, and Tools with full page traversal until nextCursor is null - Add invalid cursor tests using assertFailsWith (no nested runBlocking) - Add LoggingIntegrationTestStreamableHttp for setLevel and notification tests - Use LoggingLevel.entries instead of values() for allocation-free iteration

…ests - Replaced `if` checks with `requireNotNull` for argument validation. - Updated formatting for response generation methods to improve clarity. - Sorted tools by name in `listTools` handler for consistent pagination testing. - Replaced `kotlin.test` prefix with direct imports for assertions.

… same thread LoggingIntegrationTestStreamableHttp is not designed for concurrent execution

Copilot AI review requested due to automatic review settings March 22, 2026 07:32

Copilot started reviewing on behalf of jskjw157 March 22, 2026 07:32 View session

Copilot AI reviewed Mar 22, 2026

View reviewed changes

jskjw157 force-pushed the feature/streamable-http-missing-tests branch from 6735788 to af97f37 Compare March 22, 2026 13:42

kpavlov force-pushed the feature/streamable-http-missing-tests branch 2 times, most recently from 84d33e7 to d839b42 Compare March 26, 2026 10:27

kpavlov force-pushed the feature/streamable-http-missing-tests branch 2 times, most recently from fa786e1 to 45a3348 Compare March 26, 2026 11:05

kpavlov requested a review from devcrocod March 26, 2026 11:14

kpavlov added the tests label Mar 26, 2026

kpavlov approved these changes Mar 26, 2026

View reviewed changes

devcrocod reviewed Mar 26, 2026

View reviewed changes

jskjw157 and others added 3 commits March 26, 2026 15:01

refactor(tests): Make LoggingIntegrationTestStreamableHttp run in the…

24a1bcc

… same thread LoggingIntegrationTestStreamableHttp is not designed for concurrent execution

kpavlov force-pushed the feature/streamable-http-missing-tests branch from 45a3348 to 24a1bcc Compare March 26, 2026 13:01

-        val first = client.listResources()
-        assertTrue(first.resources.isNotEmpty())
-        val next = first.nextCursor
-        assertNotNull(next)
-        val second = client.listResources(ListResourcesRequest(PaginatedRequestParams(cursor = next)))
-        assertTrue(second.resources.isNotEmpty())
-        val combinedUris = (first.resources + second.resources).map { it.uri }
+        val combinedUris = mutableListOf<String>()
+        var cursor: String? = null
+        do {
+            val request = if (cursor == null) {
+                ListResourcesRequest()
+            } else {
+                ListResourcesRequest(PaginatedRequestParams(cursor = cursor))
+            }
+            val response = client.listResources(request)
+            combinedUris += response.resources.map { it.uri }
+            cursor = response.nextCursor
+        } while (cursor != null)
+        assertTrue(combinedUris.isNotEmpty())

	LoggingLevel.values().forEach { level ->
	LoggingLevel.entries.forEach { level ->

		kotlin.test.assertEquals(LoggingLevel.Info, received.params.level)
		kotlin.test.assertEquals(JsonPrimitive("test-data-sample"), received.params.data)

		kotlin.test.assertEquals(LoggingLevel.Info, received.params.level)
		kotlin.test.assertEquals(JsonObject(mapOf("key" to JsonPrimitive("value"))), received.params.data)

		kotlin.test.assertEquals(expectedLevels.size, receivedMessages.size)
		kotlin.test.assertEquals(expectedLevels.toList(), receivedMessages.map { it.params.level })

Conversation

jskjw157 commented Mar 22, 2026

Changes included:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 1 Tests Failed:

Uh oh!

devcrocod left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Mar 26, 2026 •

edited

Loading