Skip to content

fix(test): gate cancelled response tests on stdio features - #1365

Open
LucaCiucci wants to merge 1 commit into
modelcontextprotocol:mainfrom
BUGSENG:main
Open

LucaCiucci wants to merge 1 commit into
modelcontextprotocol:mainfrom
BUGSENG:main

Conversation

@LucaCiucci

Copy link
Copy Markdown

Declare server and transport-io as required features for test_cancelled_response.

Motivation and Context

The test calls transport::stdio(), but transport-io is not enabled by default. Without the feature requirement, default-feature test builds fail to compile. Cargo now skips this test when its required features are unavailable.

How Has This Been Tested?

  • A bare cargo test does not fail anymore.
  • Confirmed Cargo reports the required features when explicitly selecting the test without transport-io.
  • Ran cargo test -p rmcp --test test_cancelled_response --features transport-io: both tests pass.

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

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • (N/A) I have added appropriate error handling
  • (N/A) I have added or updated documentation as needed

Additional context

N/A

The cancellation response test starts a helper server with
`transport::stdio()`, but transport-io is not enabled by default.

Declare the server and transport-io requirements so Cargo skips the
test when those features are unavailable instead of failing to compile
it.
@LucaCiucci
LucaCiucci requested a review from a team as a code owner October 15, 2026 10:29
@github-actions github-actions Bot added T-dependencies Dependencies related changes T-config Configuration file changes labels Oct 15, 2026
@github-actions
github-actions Bot requested a review from bolinfest October 15, 2026 10:30
@github-actions

Copy link
Copy Markdown
Contributor

@bolinfest is on triage rotation this week and is the reviewer for this one.

Labels come from the PR labeler. Please review the change.

Automated triage. Labels are a first pass and the assigned maintainer has the final say.

@LucaCiucci

Copy link
Copy Markdown
Author

I think the automated labels are wrong. T-dependencies doesn't sound right to me.

@DaleSeo DaleSeo added T-test Testing related changes and removed T-dependencies Dependencies related changes labels Oct 15, 2026
Comment thread crates/rmcp/Cargo.toml
Comment on lines +492 to +495
[[test]]
name = "test_cancelled_response"
path = "tests/test_cancelled_response.rs"
required-features = ["server", "transport-io"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
[[test]]
name = "test_cancelled_response"
path = "tests/test_cancelled_response.rs"
required-features = ["server", "transport-io"]
[[test]]
name = "test_cancelled_response"
required-features = ["server"]
path = "tests/test_cancelled_response.rs"

Gating the entire target on transport-io also skips the three tests here gated on client. They only use duplex and mpsc, and never touch stdio.

// tests/test_cancelled_response.rs
+#[cfg(feature = "transport-io")]
 #[tokio::test(flavor = "multi_thread", worker_threads = 4)]
 async fn cancelled_request_receives_no_response() -> anyhow::Result<()> {
 ...
+#[cfg(feature = "transport-io")]
 #[tokio::test]
 // the HELPER_ENV entry point, serve_helper_stdio, spawn_helper

@DaleSeo

DaleSeo commented Oct 15, 2026

Copy link
Copy Markdown
Member

I think the automated labels are wrong. T-dependencies doesn't sound right to me.

#1368 will fix that

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-config Configuration file changes T-test Testing related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants