Refactor MockTransport

[kpavlov]

Refactor MockTransport

This replaces bespoke test transport with a reusable, configurable MockTransport in a shared testing package.

The new MockTransport supports registering handlers for specific JSON-RPC methods (success and error), records sent/received messages, and provides an awaitMessage helper with polling and timeouts. Tests were added to validate handler registration, auto-responses, error responses, concurrency, and message awaiting behavior. Existing client tests were updated to configure MockTransport via lambdas instead of hardcoded logic. Additionally, coroutine test utilities were added to the core test dependencies.

Motivation and Context

To provide better reusable test utilities

How Has This Been Tested?

• Unit tests

Breaking Changes

No

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
• Test improvement

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

Additional context

#289 follow-up cc: @maeryo

[maeryo]

The _sentMessages and _receivedMessages lists grow unbounded, which can cause memory issues in long-running or high-volume tests.

Suggestion

private val maxMessages: Int = 1000  // configurable
private val _sentMessages = ArrayDeque<JSONRPCMessage>()
private val _receivedMessages = ArrayDeque<JSONRPCMessage>()

override suspend fun send(message: JSONRPCMessage) {
    mutex.withLock {
        _sentMessages.add(message)
        if (_sentMessages.size > maxMessages) {
            _sentMessages.removeFirst()
        }
    }
    // ...
}

[maeryo]

While ConcurrentSet is used for requestHandlers, the iteration in send() method is not protected by the mutex, which could lead to race conditions when handlers are added/removed concurrently.

[maeryo]

The parameter name poolInterval appears to be a typo. The correct terminology is "pollInterval" (as in "polling"), not "pool".

[maeryo]

The parameter name poolInterval appears to be a typo and should be pollInterval (consistent with the polling pattern). This is the same issue found in MockTransport.kt

[maeryo]

I noticed a few areas that could enhance test maintainability and reliability (MockTransportTest.kt) :

1. Code Duplication (Lines 338-339, 374, 432, 472, 514, 549)

The same transport setup pattern is repeated across multiple tests. Consider extracting a helper:

private fun createCustomTransport(configure: MockTransport.() -> Unit = {}): MockTransport {
    return MockTransport(configure).apply { onMessage { } }
}

2. Test Isolation (Lines 29-48)

All tests share the same transport instance. Since MockTransport accumulates messages internally, this could cause test interference. Consider either:

• Adding a clearMessages() method and calling it in @BeforeTest, or
• Creating fresh transport instances per test when isolation is needed

// Remove shared transport property and create in each test
@Test
fun `some test`() = runTest {
    val transport = MockTransport {
        onMessageReplyResult(Method.Defined.Initialize) { ... }
    }
    transport.onMessage { }
    // test logic
}

3. Redundant Assertions (Throughout)

The pattern assertNotNull(x) + assertTrue(x is Type) is redundant in Kotlin. The type check already ensures non-null:

// Current
assertNotNull(message)
assertTrue(message is JSONRPCResponse)

// Suggested
assertTrue(message is JSONRPCResponse)  // Smart cast handles both checks

These are minor improvements but would help keep the test suite cleaner and more maintainable. WDYT?

[devcrocod]

Please consider keeping MockTransport within the test code rather than exposing it in the public API

[devcrocod]

I’m not sure why MockTransport needs to be added to the core module. It makes the core module more complex, and such things don’t really belong there.
In my opinion, there should be a separate module for mocks that’s used exclusively for testing

[kpavlov]

@devcrocod feel free to continue and move MockTransport to test classes and finish or drop this PR