Skip to content

Conversation

@devcrocod
Copy link
Contributor

  • ktor 3.2.3
  • Tests are ignored because Ktor 3.2.3 has issues with wasm in the test environment. However, it works outside of tests. All other targets pass these tests successfully.

Motivation and Context

compatibility with older versions of Ktor

How Has This Been Tested?

CI

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

@devcrocod devcrocod marked this pull request as ready for review October 24, 2025 12:24
Copilot AI review requested due to automatic review settings October 24, 2025 12:24
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 downgrades Ktor from version 3.3.1 to 3.2.3 to maintain compatibility with older versions. Due to known issues with Ktor 3.2.3's wasm/js support in test environments, several integration and transport tests have been temporarily disabled with @Ignore annotations. The tests continue to pass on all other target platforms.

Key Changes

  • Ktor dependency downgraded from 3.3.1 to 3.2.3 in the version catalog
  • Six tests across three test files marked as ignored with explanatory comments

Reviewed Changes

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

File Description
gradle/libs.versions.toml Downgrades Ktor version from 3.3.1 to 3.2.3
kotlin-sdk-test/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/WebSocketIntegrationTest.kt Adds @Ignore annotations to three WebSocket integration tests due to wasm/js incompatibility
kotlin-sdk-test/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/SseIntegrationTest.kt Adds @Ignore annotations to three SSE integration tests due to wasm/js incompatibility
kotlin-sdk-test/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseTransportTest.kt Adds @Ignore annotation to one SSE transport test due to wasm/js incompatibility

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@devcrocod devcrocod requested a review from tiginamaria October 24, 2025 12:27
Copy link
Contributor

@tiginamaria tiginamaria left a comment

Choose a reason for hiding this comment

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

As I get the comment right, tests with lower ktor are failing with wasm/js, but do work with jvm. Maybe we can instead of ignoring move tests under jvm target?
Other then that looks good!

@devcrocod
Copy link
Contributor Author

@tiginamaria Yes, I’m going to do that in the next PRs.
The WebSocket will be tested only with the JVM.

I’ll look into why sse fails with wasm and try to fix it for wasm. Otherwise, I’ll move it under the JVM as well.
At the moment, I didn’t want this to be a blocker for the release

@devcrocod devcrocod merged commit ebd5a27 into main Oct 24, 2025
4 checks passed
@devcrocod devcrocod deleted the devcrocod/downgrade-ktor branch October 24, 2025 12:40
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