fix conformance tests (#483)
# Fix conformance tests. 1. fix node version to workaround https://github.com/nodejs/node/issues/56645 2. use `@argfile` to handle long classpaths 3. specify extension for npx for windows 4. Install `@modelcontextprotocol/conformance@0.1.8` GitHub Actions workflow. 5. Centralize logging for process streams in conformance tests: Add `startLogging` utility for InputStream logging with `KotlinLogging`. Replace inline process stream handling with `startLogging` calls Note: Currently tests pass using `@modelcontextprotocol/conformance@0.1.8`/ But changing to 0.1.9 will fail client scenarios (it is not related to this PR). ## Motivation and Context Before fix conformance tests are broken on Windows ## How Has This Been Tested? Run test locally and in github actions ## Breaking Changes No ## Types of changes - [x] 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 <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [x] I have added appropriate error handling - [x] I have added or updated documentation as needed ## Additional context May be we should update `CONTRIBUTING.md/Building/Requirements`. But currently it does not mention at all: node, android, etc. --------- Co-authored-by: f.bobin <f.bobin@vk.team> Co-authored-by: Konstantin Pavlov <1517853+kpavlov@users.noreply.github.com>
This commit is contained in:
Fedor Bobin
GitHub
parent
1ee43f07b0
commit
c73507b96a
7 changed files with 119 additions and 52 deletions
2
.github/workflows/build.yml
vendored
2
.github/workflows/build.yml
vendored
|
|
@ -62,7 +62,7 @@ jobs:
|
|||
if: matrix.os == 'ubuntu-latest'
|
||||
uses: actions/setup-node@v6
|
||||
with:
|
||||
node-version: 'lts/*'
|
||||
node-version: '22' # increase only after https://github.com/nodejs/node/issues/56645 will be fixed
|
||||
|
||||
- name: Set up Node Packages
|
||||
if: matrix.os == 'ubuntu-latest'
|
||||
|
|
|
|||
28
.github/workflows/conformance.yml
vendored
28
.github/workflows/conformance.yml
vendored
|
|
@ -14,11 +14,23 @@ concurrency:
|
|||
|
||||
jobs:
|
||||
run-conformance:
|
||||
runs-on: macos-latest
|
||||
name: Run Conformance Tests
|
||||
runs-on: ${{ matrix.os }}
|
||||
name: Run Conformance Tests on ${{ matrix.os }}
|
||||
timeout-minutes: 20
|
||||
env:
|
||||
JAVA_OPTS: "-Xmx8g -Dfile.encoding=UTF-8 -Djava.awt.headless=true -Dkotlin.daemon.jvm.options=-Xmx6g"
|
||||
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
include:
|
||||
- os: ubuntu-latest
|
||||
max-workers: 3
|
||||
- os: windows-latest
|
||||
max-workers: 3
|
||||
- os: macos-latest
|
||||
max-workers: 2
|
||||
|
||||
steps:
|
||||
- uses: actions/checkout@v6
|
||||
|
||||
|
|
@ -31,7 +43,12 @@ jobs:
|
|||
- name: Setup Node.js
|
||||
uses: actions/setup-node@v6
|
||||
with:
|
||||
node-version: 'lts/*'
|
||||
node-version: '22' # increase only after https://github.com/nodejs/node/issues/56645 will be fixed
|
||||
|
||||
- name: Setup Conformance Tests
|
||||
working-directory: conformance-test
|
||||
run: |-
|
||||
npm install -g @modelcontextprotocol/conformance@0.1.8
|
||||
|
||||
- name: Setup Gradle
|
||||
uses: gradle/actions/setup-gradle@v5
|
||||
|
|
@ -45,11 +62,12 @@ jobs:
|
|||
../.konan/**
|
||||
|
||||
- name: Run Conformance Tests
|
||||
run: ./gradlew --no-daemon :conformance-test:test
|
||||
run: |-
|
||||
./gradlew :conformance-test:test --no-daemon --max-workers ${{ matrix.max-workers }}
|
||||
|
||||
- name: Upload Conformance Results
|
||||
if: always()
|
||||
uses: actions/upload-artifact@v6
|
||||
with:
|
||||
name: conformance-results
|
||||
name: conformance-results-${{ matrix.os }}
|
||||
path: conformance-test/results/
|
||||
|
|
|
|||
|
|
@ -1,7 +1,9 @@
|
|||
package io.modelcontextprotocol.kotlin.sdk.conformance
|
||||
|
||||
import io.github.oshai.kotlinlogging.KotlinLogging
|
||||
import io.modelcontextprotocol.kotlin.test.utils.NPX
|
||||
import io.modelcontextprotocol.kotlin.test.utils.findFreePort
|
||||
import io.modelcontextprotocol.kotlin.test.utils.startLogging
|
||||
import org.junit.jupiter.api.AfterAll
|
||||
import org.junit.jupiter.api.BeforeAll
|
||||
import org.junit.jupiter.api.DynamicTest
|
||||
|
|
@ -13,10 +15,14 @@ import java.lang.management.ManagementFactory
|
|||
import java.net.HttpURLConnection
|
||||
import java.net.URI
|
||||
import java.util.concurrent.TimeUnit
|
||||
import kotlin.io.path.createTempFile
|
||||
import kotlin.properties.Delegates
|
||||
|
||||
private val logger = KotlinLogging.logger {}
|
||||
|
||||
val processStderrLogger = KotlinLogging.logger(name = "stderr")
|
||||
val processStdoutLogger = KotlinLogging.logger(name = "stdout")
|
||||
|
||||
private const val CONFORMANCE_VERSION = "0.1.8"
|
||||
|
||||
enum class TransportType {
|
||||
|
|
@ -210,79 +216,75 @@ class ConformanceTest {
|
|||
}
|
||||
|
||||
private fun runServerConformanceTest(scenario: String, transportType: TransportType) {
|
||||
val processBuilder = when (transportType) {
|
||||
val serverUrl = when (transportType) {
|
||||
TransportType.SSE -> {
|
||||
val serverUrl = "http://127.0.0.1:$serverPort/mcp"
|
||||
ProcessBuilder(
|
||||
"npx",
|
||||
"@modelcontextprotocol/conformance@$CONFORMANCE_VERSION",
|
||||
"server",
|
||||
"--url",
|
||||
serverUrl,
|
||||
"--scenario",
|
||||
scenario,
|
||||
).apply {
|
||||
inheritIO()
|
||||
}
|
||||
"http://127.0.0.1:$serverPort/mcp"
|
||||
}
|
||||
|
||||
TransportType.WEBSOCKET -> {
|
||||
val serverUrl = "ws://127.0.0.1:$serverPort/ws"
|
||||
ProcessBuilder(
|
||||
"npx",
|
||||
"@modelcontextprotocol/conformance@$CONFORMANCE_VERSION",
|
||||
"server",
|
||||
"--url",
|
||||
serverUrl,
|
||||
"--scenario",
|
||||
scenario,
|
||||
).apply {
|
||||
inheritIO()
|
||||
}
|
||||
"ws://127.0.0.1:$serverPort/ws"
|
||||
}
|
||||
}
|
||||
|
||||
val processBuilder = ProcessBuilder(
|
||||
NPX,
|
||||
"@modelcontextprotocol/conformance@$CONFORMANCE_VERSION",
|
||||
"server",
|
||||
"--url",
|
||||
serverUrl,
|
||||
"--scenario",
|
||||
scenario,
|
||||
)
|
||||
|
||||
runConformanceTest("server", scenario, processBuilder, transportType)
|
||||
}
|
||||
|
||||
private fun runClientConformanceTest(scenario: String, transportType: TransportType) {
|
||||
val testClasspath = getTestClasspath()
|
||||
|
||||
val clientCommand = when (transportType) {
|
||||
// Create an argfile to avoid Windows command line length limits
|
||||
val argFile = createTempFile(suffix = ".args").toFile()
|
||||
argFile.deleteOnExit()
|
||||
|
||||
val mainClass = when (transportType) {
|
||||
TransportType.SSE -> {
|
||||
val serverUrl = "http://127.0.0.1:$serverPort/mcp"
|
||||
listOf(
|
||||
"java",
|
||||
"-cp",
|
||||
testClasspath,
|
||||
"io.modelcontextprotocol.kotlin.sdk.conformance.ConformanceClientKt",
|
||||
serverUrl,
|
||||
argFile.writeText(
|
||||
buildString {
|
||||
appendLine("-cp")
|
||||
appendLine(testClasspath)
|
||||
appendLine("io.modelcontextprotocol.kotlin.sdk.conformance.ConformanceClientKt")
|
||||
},
|
||||
)
|
||||
"http://127.0.0.1:$serverPort/mcp"
|
||||
}
|
||||
|
||||
TransportType.WEBSOCKET -> {
|
||||
val serverUrl = "ws://127.0.0.1:$serverPort/ws"
|
||||
listOf(
|
||||
"java",
|
||||
"-cp",
|
||||
testClasspath,
|
||||
"io.modelcontextprotocol.kotlin.sdk.conformance.WebSocketConformanceClientKt",
|
||||
serverUrl,
|
||||
argFile.writeText(
|
||||
buildString {
|
||||
appendLine("-cp")
|
||||
appendLine(testClasspath)
|
||||
appendLine("io.modelcontextprotocol.kotlin.sdk.conformance.WebSocketConformanceClientKt")
|
||||
},
|
||||
)
|
||||
"ws://127.0.0.1:$serverPort/ws"
|
||||
}
|
||||
}
|
||||
|
||||
val clientCommand = listOf(
|
||||
"java",
|
||||
"@${argFile.absolutePath}",
|
||||
mainClass,
|
||||
)
|
||||
|
||||
val processBuilder = ProcessBuilder(
|
||||
"npx",
|
||||
NPX,
|
||||
"@modelcontextprotocol/conformance@$CONFORMANCE_VERSION",
|
||||
"client",
|
||||
"--command",
|
||||
clientCommand.joinToString(" "),
|
||||
"--scenario",
|
||||
scenario,
|
||||
).apply {
|
||||
inheritIO()
|
||||
}
|
||||
)
|
||||
|
||||
runConformanceTest("client", scenario, processBuilder, transportType)
|
||||
}
|
||||
|
|
@ -296,10 +298,20 @@ class ConformanceTest {
|
|||
val capitalizedType = type.replaceFirstChar { it.uppercase() }
|
||||
logger.info { "Running $type conformance test [$transportType]: $scenario" }
|
||||
|
||||
val timeoutSeconds = System.getenv("CONFORMANCE_TEST_TIMEOUT_SECONDS")?.toLongOrNull()
|
||||
?: DEFAULT_TEST_TIMEOUT_SECONDS
|
||||
val timeoutSeconds =
|
||||
System.getenv("CONFORMANCE_TEST_TIMEOUT_SECONDS")?.toLongOrNull() ?: DEFAULT_TEST_TIMEOUT_SECONDS
|
||||
|
||||
val process = processBuilder.start()
|
||||
|
||||
process.errorStream.startLogging(
|
||||
logger = processStderrLogger,
|
||||
name = "test(PID=${process.pid()})",
|
||||
)
|
||||
process.inputStream.startLogging(
|
||||
logger = processStdoutLogger,
|
||||
name = "test(PID=${process.pid()})",
|
||||
)
|
||||
|
||||
val completed = process.waitFor(timeoutSeconds, TimeUnit.SECONDS)
|
||||
|
||||
if (!completed) {
|
||||
|
|
|
|||
|
|
@ -5,4 +5,6 @@ org.slf4j.simpleLogger.showDateTime=false
|
|||
|
||||
# Log level for specific packages or classes
|
||||
org.slf4j.simpleLogger.log.io.ktor.server=DEBUG
|
||||
org.slf4j.simpleLogger.log.stdout=TRACE
|
||||
org.slf4j.simpleLogger.log.stderr=TRACE
|
||||
org.slf4j.simpleLogger.log.io.modelcontextprotocol=DEBUG
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ kotlin {
|
|||
api(libs.mockk)
|
||||
api(libs.awaitility)
|
||||
api(libs.junit.jupiter.params)
|
||||
api(libs.kotlin.logging)
|
||||
runtimeOnly(libs.slf4j.simple)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
package io.modelcontextprotocol.kotlin.test.utils
|
||||
|
||||
public val isWindows: Boolean = System.getProperty("os.name").lowercase().contains("windows")
|
||||
|
||||
public val NPX: String = if (isWindows) "npx.cmd" else "npx"
|
||||
|
|
|
|||
|
|
@ -1,5 +1,10 @@
|
|||
package io.modelcontextprotocol.kotlin.test.utils
|
||||
|
||||
import io.github.oshai.kotlinlogging.KLogger
|
||||
import java.io.BufferedReader
|
||||
import java.io.InputStream
|
||||
import java.io.InputStreamReader
|
||||
|
||||
public fun createTeeProcessBuilder(): ProcessBuilder = if (isWindows) {
|
||||
ProcessBuilder("cmd", "/c", "findstr", "^")
|
||||
} else {
|
||||
|
|
@ -11,3 +16,30 @@ public fun createSleepyProcessBuilder(): ProcessBuilder = if (isWindows) {
|
|||
} else {
|
||||
ProcessBuilder("sh", "-c", "sleep 1 && echo 'simulated error' >&2 && exit 1")
|
||||
}
|
||||
|
||||
/**
|
||||
* Starts logging the lines from this InputStream to a specified logger.
|
||||
* Each line from the InputStream is read and logged using the provided logger
|
||||
* at the debug level, optionally with an additional prefix string appended
|
||||
* to each logged line. The method runs the logging process in a separate daemon thread.
|
||||
*
|
||||
* @param logger The logger used to log the lines from the InputStream.
|
||||
* @param name The name to assign to the daemon thread handling the logging process.
|
||||
*/
|
||||
public fun InputStream.startLogging(logger: KLogger, name: String) {
|
||||
val stream = this
|
||||
Thread {
|
||||
try {
|
||||
BufferedReader(InputStreamReader(stream)).use { reader ->
|
||||
reader.lineSequence().forEach { line ->
|
||||
logger.debug { line }
|
||||
}
|
||||
}
|
||||
} catch (e: Exception) {
|
||||
logger.trace(e) { "Error reading server test stdout" }
|
||||
}
|
||||
}.apply {
|
||||
this.name = name
|
||||
isDaemon = true
|
||||
}.start()
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue