Skip to content

Skip flaky test on Mono where stderr callbacks are unsupported#980

Merged
stephentoub merged 4 commits intomainfrom
copilot/fix-flaky-createasync-test
Nov 20, 2025
Merged

Skip flaky test on Mono where stderr callbacks are unsupported#980
stephentoub merged 4 commits intomainfrom
copilot/fix-flaky-createasync-test

Conversation

Copy link
Contributor

Copilot AI commented Nov 18, 2025 •
edited
Loading

Fix flaky test: CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked

Summary

Fixed the flaky test by properly skipping it on Mono runtime where Process.ErrorDataReceived callbacks are not reliably supported.

Root Cause

The test CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked relies on Process.ErrorDataReceived event to capture stderr output from a subprocess. This event is not reliably fired on Mono runtime, which is a known platform limitation.

The test already had platform detection logic (IsStdErrCallbackSupported => !PlatformDetection.IsMonoRuntime) to identify this incompatibility, but the skip attribute was commented out, allowing the test to run (and fail) on Mono.

The test failed on Mono because:

  1. The subprocess executes and writes to stderr
  2. The ErrorDataReceived event callback is never invoked on Mono
  3. Test assertions check that the callback was called (count >= 1)
  4. The assertion fails with count still at 0

Solution

Uncommented the [Fact(Skip = ..., SkipUnless = nameof(IsStdErrCallbackSupported))] attribute to properly skip the test on Mono platforms where it cannot succeed.

Previous attempts to fix this by modifying StdioClientSessionTransport.cs with additional WaitForExit() calls were incorrect and have been reverted, as they did not address the root cause (Mono not firing the event at all).

Changes

  • tests/ModelContextProtocol.Tests/Transport/StdioClientTransportTests.cs: Uncommented the skip attribute to skip test on Mono
  • src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs: Reverted to original code (no changes)

Testing & Verification

  • Test runs and passes on .NET 8.0, 9.0, and 10.0 (non-Mono) - verified 10 consecutive runs
  • All 40 StdioClientTransportTests pass
  • Test is properly skipped on Mono platforms where stderr callbacks don't work
  • Build completes with no warnings or errors

Security Summary

No security vulnerabilities were introduced or discovered. No code changes to production code.

Original prompt

This section details on the original issue you should resolve

<issue_title>CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked test is very flaky</issue_title>
<issue_description>It's failing frequently in CI.</issue_description>

<agent_instructions>Please investigate why this test is flaky and fix it.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix flaky CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked test Fix race condition in stderr reading for stdio transport Nov 18, 2025
Copilot AI requested a review from stephentoub November 18, 2025 19:21
@stephentoub stephentoub marked this pull request as ready for review November 18, 2025 19:42
Copilot AI requested a review from stephentoub November 18, 2025 19:50
@stephentoub
Copy link
Contributor

stephentoub commented Nov 19, 2025

@copilot, this change does not address the issue, as evidenced by CI failing with the problem:

  Assert.InRange() Failure: Value not in range
  Range:  (1 - 2147483647)
  Actual: 0
      at ModelContextProtocol.Tests.Transport.StdioClientTransportTests.CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked () [0x001c8] in <69cc817959bb4cc3885cc3ba22a5c8d7>:0 
  Standard output
    | [2025-11-19T13:20:43] ModelContextProtocol.Client.StdioClientTransport Information: stdio-sh connecting.
    | [2025-11-19T13:20:43] ModelContextProtocol.Client.StdioClientTransport Information: stdio-sh starting server process. Command: 'sh'.
    | [2025-11-19T13:20:43] ModelContextProtocol.Client.StdioClientTransport Information: stdio-sh started server process with PID 8519.
    | [2025-11-19T13:20:43] ModelContextProtocol.Client.StdioClientSessionTransport Information: stdio-sh transport reading messages.
    | [2025-11-19T13:20:43] ModelContextProtocol.Client.StdioClientSessionTransport Information: stdio-sh transport completed reading messages.
    | [2025-11-19T13:20:43] ModelContextProtocol.Client.StdioClientSessionTransport Information: stdio-sh shutting down.
    | [2025-11-19T13:20:43] ModelContextProtocol.Client.StdioClientSessionTransport Information: stdio-sh shut down.
    | [2025-11-19T13:20:43] ModelContextProtocol.Client.McpClient Error: Client (ModelContextProtocol.Tests 1.0.0.0) client initialization error.
    System.IO.IOException: The server shut down unexpectedly.
      at System.Threading.Tasks.TaskExtensions.WaitAsync (System.Threading.Tasks.Task task, System.TimeSpan timeout, System.Threading.CancellationToken cancellationToken) [0x00265] in <1fc557a7312b4427a8acfe69c788c8e6>:0 
      at System.Threading.Tasks.TaskExtensions.WaitAsync[T] (System.Threading.Tasks.Task`1[TResult] task, System.TimeSpan timeout, System.Threading.CancellationToken cancellationToken) [0x0007f] in <1fc557a7312b4427a8acfe69c788c8e6>:0 
      at ModelContextProtocol.McpSessionHandler.SendRequestAsync (ModelContextProtocol.Protocol.JsonRpcRequest request, System.Threading.CancellationToken cancellationToken) [0x0038c] in <1fc557a7312b4427a8acfe69c788c8e6>:0 
      at ModelContextProtocol.McpSession.SendRequestAsync[TParameters,TResult] (System.String method, TParameters parameters, System.Text.Json.Serialization.Metadata.JsonTypeInfo`1[T] parametersTypeInfo, System.Text.Json.Serialization.Metadata.JsonTypeInfo`1[T] resultTypeInfo, ModelContextProtocol.Protocol.RequestId requestId, System.Threading.CancellationToken cancellationToken) [0x000f5] in <1fc557a7312b4427a8acfe69c788c8e6>:0 
      at ModelContextProtocol.Client.McpClientImpl.ConnectAsync (System.Threading.CancellationToken cancellationToken) [0x001e1] in <1fc557a7312b4427a8acfe69c788c8e6>:0

Copy link
Contributor Author

Copilot AI commented Nov 19, 2025

@copilot, this change does not address the issue, as evidenced by CI failing with the problem:

  Assert.InRange() Failure: Value not in range

...

You're correct - my previous fix didn't address the issue. In 688a602, I've reverted those changes and implemented the proper fix: skipping the test on Mono where Process.ErrorDataReceived callbacks don't work reliably. The test already had IsStdErrCallbackSupported => !PlatformDetection.IsMonoRuntime but the skip attribute was commented out.

Copilot AI changed the title Fix race condition in stderr reading for stdio transport Skip flaky test on Mono where stderr callbacks are unsupported Nov 19, 2025
Copilot AI requested a review from stephentoub November 19, 2025 22:04
@stephentoub stephentoub merged commit 3f39ba7 into main Nov 20, 2025
10 checks passed
@stephentoub stephentoub deleted the copilot/fix-flaky-createasync-test branch November 20, 2025 03:34
Copilot AI mentioned this pull request Feb 15, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eiriktsarpalis eiriktsarpalis eiriktsarpalis left review comments

@halter73 halter73 Awaiting requested review from halter73

@stephentoub stephentoub Awaiting requested review from stephentoub

Assignees

@stephentoub stephentoub

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked test is very flaky

3 participants

@stephentoub @eiriktsarpalis