Skip to content

Conversation

@devcrocod
Copy link
Contributor

With Ktor 3.2.3, tests don’t work correctly with wasm, so the websocket tests were moved to the jvm target as the most important one

How Has This Been Tested?

local

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
  • Refactoring

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
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 updates the WebSocket integration tests to enable them and use non-suspend Ktor server lifecycle methods. The tests were previously disabled due to compatibility issues with wasm/js in Ktor 3.2.3.

  • Removed @Ignore annotations to re-enable the WebSocket integration tests
  • Changed server lifecycle methods from startSuspend/stopSuspend to start/stop
  • Updated the package name to include the full path hierarchy
Comments suppressed due to low confidence (4)

kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/websocket/WebSocketIntegrationTest.kt:48

  • The stop method is blocking and should not be called from a coroutine context without withContext(Dispatchers.IO). Since this is in a finally block of a runTest, this could block the test dispatcher. Consider wrapping in withContext(Dispatchers.IO) or using a suspend alternative if available.
    kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/websocket/WebSocketIntegrationTest.kt:75
  • The stop method is blocking and should not be called from a coroutine context without withContext(Dispatchers.IO). Since this is in a finally block of a runTest, this could block the test dispatcher. Consider wrapping in withContext(Dispatchers.IO) or using a suspend alternative if available.
    kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/websocket/WebSocketIntegrationTest.kt:111
  • The stop method is blocking and should not be called from a coroutine context without withContext(Dispatchers.IO). Since this is in a finally block of a runTest, this could block the test dispatcher. Consider wrapping in withContext(Dispatchers.IO) or using a suspend alternative if available.
    kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/websocket/WebSocketIntegrationTest.kt:174
  • The start method is blocking and called from a non-suspend function that's invoked within withContext(Dispatchers.Default). This blocking call could impact performance. Consider wrapping in runBlocking or using startSuspend if available to maintain consistency with the asynchronous nature of the test.

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

@devcrocod devcrocod force-pushed the devcrocod/refactor-websocket-tests branch from 6e2c83b to d7401d8 Compare October 30, 2025 11:57
@devcrocod devcrocod merged commit cae9130 into main Oct 30, 2025
4 checks passed
@devcrocod devcrocod deleted the devcrocod/refactor-websocket-tests branch October 30, 2025 14:12
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.

3 participants