Skip to content

test: remove mokksy dependency#754

Open
devcrocod wants to merge 1 commit into
mainfrom
chore/remove-mokksy
Open

test: remove mokksy dependency#754
devcrocod wants to merge 1 commit into
mainfrom
chore/remove-mokksy

Conversation

@devcrocod
Copy link
Copy Markdown
Contributor

Dropsdev.mokksy:mokksy dependency

Motivation and Context

mokksy is a niche, single-maintainer library with very limited external adoption, introduced by its own author. Keeping a test-only dependency on it is unnecessary supply-chain exposure given that the two patterns already used in this repo, MockEngine for transport-level tests and embeddedServer for integration-level tests, cover everything it provided.

Breaking Changes

none

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the dev.mokksy:mokksy test dependency and replaces the affected Streamable HTTP client tests with a combination of MockEngine-based common tests and embedded-server integration tests, reducing supply-chain exposure while preserving coverage for key behaviors.

Changes:

  • Removed Mokksy-based JVM tests and their supporting mock server utilities.
  • Added a commonTest coverage replacement for “skip empty SSE” using MockEngine and inline SSE parsing.
  • Added new JVM integration tests for Streamable HTTP client transport using embeddedServer + CIO.

Reviewed changes

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

Show a summary per file
File Description
kotlin-sdk-client/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/StreamableHttpClientTest.kt Removed Mokksy-based Streamable HTTP client tests.
kotlin-sdk-client/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/MockMcp.kt Removed Mokksy-based mock MCP server implementation.
kotlin-sdk-client/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/AbstractStreamableHttpClientTest.kt Removed Mokksy-backed shared test harness.
kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/streamable/http/StreamableHttpClientTransportTest.kt Added MockEngine-based test to ensure empty/whitespace SSE events are skipped.
kotlin-sdk-client/build.gradle.kts Dropped the Mokksy dependency from JVM tests.
integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/streamablehttp/StreamableHttpClientTransportIntegrationTest.kt Added embedded-server integration tests for GET SSE notifications, termination, and non-SSE GET behaviors.
gradle/libs.versions.toml Removed the Mokksy version and library entry from the catalog.

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

awaitCancellation()
}
}
delete(MCP_PATH) {
internal class StreamableHttpClientTransportIntegrationTest {

@Test
fun `client receives progress notifications via GET SSE and lists tools`(): Unit = runBlocking(Dispatchers.IO) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants