Skip to content

chore: consolidate repeated rmcp tests - #931

Merged
DaleSeo merged 1 commit into
mainfrom
chore/rstest-lint-cleanup
Jun 26, 2026
Merged

chore: consolidate repeated rmcp tests#931
DaleSeo merged 1 commit into
mainfrom
chore/rstest-lint-cleanup

Conversation

@DaleSeo

@DaleSeoDaleSeo commented Jun 26, 2026

Copy link
Copy Markdown
Member

Motivation and Context

This introduces rstest for repeated rmcp unit-test cases so near-duplicate test clusters can stay individually reported without hand-written loops. It also cleans up existing test-target lint failures by making private test fixtures private, simplifying manual future-returning test handlers to async fns, and keeping macro-only router fields explicitly expected. Before this change, strict clippy on the rmcp test targets failed on clippy::exhaustive_structs, clippy::manual_async_fn, and dead_code; after the cleanup, cargo clippy -p rmcp --all-targets --all-features --locked -- -D warnings passes.

How Has This Been Tested?

Before:

❯ cargo clippy -p rmcp --features "client auth elicitation transport-streamable-http-client-reqwest" --lib --tests -- -D warnings
Checking rmcp v1.8.0 (/Users/dale.seo/dale/rust-sdk/crates/rmcp)
error: unused imports: `ListPromptsResult`, `PaginatedRequestParams`, `RoleServer`, and `service::RequestContext`
--> crates/rmcp/tests/test_prompt_macros.rs:7:20
|
7 | ClientHandler, RoleServer, ServerHandler, ServiceExt,
| ^^^^^^^^^^
...
10 | ClientInfo, ContentBlock, GetPromptRequestParams, GetPromptResult, ListPromptsResult,
| ^^^^^^^^^^^^^^^^^
11 | PaginatedRequestParams, PromptMessage, Role,
| ^^^^^^^^^^^^^^^^^^^^^^
...
14 | service::RequestContext,
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D unused-imports` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(unused_imports)]`
error: this function can be simplified using the `async fn` syntax
--> crates/rmcp/tests/test_tool_disable_notification.rs:48:5
|
48 | / fn call_tool(
49 | | &self,
50 | | request: rmcp::model::CallToolRequestParams,
51 | | context: rmcp::service::RequestContext<RoleServer>,
52 | | ) -> impl std::future::Future<Output = Result<CallToolResult, rmcp::ErrorData>> + MaybeSendFuture + '_
| |__________________________________________________________________________________________________________^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#manual_async_fn
= note: `-D clippy::manual-async-fn` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_async_fn)]`
error: this function can be simplified using the `async fn` syntax
--> crates/rmcp/tests/test_tool_disable_notification.rs:61:5
|
61 | / fn list_tools(
62 | | &self,
63 | | _request: Option<rmcp::model::PaginatedRequestParams>,
64 | | _context: rmcp::service::RequestContext<RoleServer>,
65 | | ) -> impl std::future::Future<Output = Result<rmcp::model::ListToolsResult, rmcp::ErrorData>>
66 | | + MaybeSendFuture
67 | | + '_ {
| |________^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#manual_async_fn
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_notification.rs:15:1
|
15 | pub struct Server {}
| ^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
= note: `-D clippy::exhaustive-structs` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::exhaustive_structs)]`
help: try adding #[non_exhaustive]
|
15 + #[non_exhaustive]
16 | pub struct Server {}
|
error: could not compile `rmcp` (test "test_tool_disable_notification") due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error: could not compile `rmcp` (test "test_notification") due to 1 previous error
error: field `tool_router` is never read
--> crates/rmcp/tests/test_task_support_validation.rs:21:5
|
20 | pub struct TaskSupportTestServer {
| --------------------- field in this struct
21 | tool_router: ToolRouter<Self>,
| ^^^^^^^^^^^
|
= note: `TaskSupportTestServer` has derived impls for the traits `Clone` and `Debug`, but these are intentionally ignored during dead code analysis
= note: `-D dead-code` implied by `-D warnings`
= help: to override `-D warnings` add `#[expect(dead_code)]` or `#[allow(dead_code)]`
error: could not compile `rmcp` (test "test_task_support_validation") due to 1 previous error
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_prompt_macros.rs:20:1
|
20 | / pub struct CodeReviewRequest {
21 | | pub file_path: String,
22 | | pub language: String,
23 | | }
| |_^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
= note: `-D clippy::exhaustive-structs` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::exhaustive_structs)]`
help: try adding #[non_exhaustive]
|
20 + #[non_exhaustive]
21 | pub struct CodeReviewRequest {
|
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_prompt_macros.rs:197:1
|
197 | / pub struct OptionalFieldTestSchema {
198 | | #[schemars(description = "An optional description field")]
199 | | pub description: Option<String>,
200 | | }
| |_^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
help: try adding #[non_exhaustive]
|
197 + #[non_exhaustive]
198 | pub struct OptionalFieldTestSchema {
|
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_prompt_macros.rs:204:1
|
204 | / pub struct OptionalI64TestSchema {
205 | | #[schemars(description = "An optional i64 field")]
206 | | pub count: Option<i64>,
207 | | pub mandatory_field: String, // Added to ensure non-empty object schema
208 | | }
| |_^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
help: try adding #[non_exhaustive]
|
204 + #[non_exhaustive]
205 | pub struct OptionalI64TestSchema {
|
error: could not compile `rmcp` (test "test_prompt_macros") due to 4 previous errors
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_prompt_routers.rs:12:1
|
12 | / pub struct TestHandler<T: 'static = ()> {
13 | | pub _marker: std::marker::PhantomData<fn(*const T)>,
14 | | }
| |_^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
= note: `-D clippy::exhaustive-structs` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::exhaustive_structs)]`
help: try adding #[non_exhaustive]
|
12 + #[non_exhaustive]
13 | pub struct TestHandler<T: 'static = ()> {
|
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_prompt_routers.rs:19:1
|
19 | / pub struct Request {
20 | | pub fields: HashMap<String, String>,
21 | | }
| |_^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
help: try adding #[non_exhaustive]
|
19 + #[non_exhaustive]
20 | pub struct Request {
|
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_prompt_routers.rs:24:1
|
24 | / pub struct Sum {
25 | | pub a: i32,
26 | | pub b: i32,
27 | | }
| |_^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
help: try adding #[non_exhaustive]
|
24 + #[non_exhaustive]
25 | pub struct Sum {
|
error: could not compile `rmcp` (test "test_prompt_routers") due to 3 previous errors
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_tool_macros.rs:21:1
|
21 | / pub struct GetWeatherRequest {
22 | | /// City of interest.
23 | | pub city: String,
24 | | /// Date of interest.
25 | | pub date: String,
26 | | }
| |_^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
= note: `-D clippy::exhaustive-structs` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::exhaustive_structs)]`
help: try adding #[non_exhaustive]
|
21 + #[non_exhaustive]
22 | pub struct GetWeatherRequest {
|
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_tool_macros.rs:165:1
|
165 | / pub struct OptionalFieldTestSchema {
166 | | /// Field description.
167 | | #[schemars(description = "An optional description field")]
168 | | pub description: Option<String>,
169 | | }
| |_^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
help: try adding #[non_exhaustive]
|
165 + #[non_exhaustive]
166 | pub struct OptionalFieldTestSchema {
|
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_tool_macros.rs:173:1
|
173 | / pub struct OptionalI64TestSchema {
174 | | /// Optional count field.
175 | | #[schemars(description = "An optional i64 field")]
176 | | pub count: Option<i64>,
... |
179 | | pub mandatory_field: String,
180 | | }
| |_^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
help: try adding #[non_exhaustive]
|
173 + #[non_exhaustive]
174 | pub struct OptionalI64TestSchema {
|
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_tool_macros.rs:373:1
|
373 | pub struct MinimalServer;
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
help: try adding #[non_exhaustive]
|
373 + #[non_exhaustive]
374 | pub struct MinimalServer;
|
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_tool_macros.rs:455:1
|
455 | pub struct ElidedToolHandlerServer;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
help: try adding #[non_exhaustive]
|
455 + #[non_exhaustive]
456 | pub struct ElidedToolHandlerServer;
|
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_tool_macros.rs:512:1
|
512 | pub struct CustomInfoServer;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
help: try adding #[non_exhaustive]
|
512 + #[non_exhaustive]
513 | pub struct CustomInfoServer;
|
error: exported structs should not be exhaustive
--> crates/rmcp/tests/test_tool_macros.rs:542:1
|
542 | pub struct ManualInfoServer;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#exhaustive_structs
help: try adding #[non_exhaustive]
|
542 + #[non_exhaustive]
543 | pub struct ManualInfoServer;
|
error: could not compile `rmcp` (test "test_tool_macros") due to 7 previous errors

After:

❯ cargo clippy -p rmcp --features "client auth elicitation transport-streamable-http-client-reqwest" --lib --tests -- -D warnings
Checking rmcp v1.8.0 (/Users/dale.seo/dale/rust-sdk/crates/rmcp)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 5.18s

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

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

@github-actionsgithub-actionsBot added T-dependencies Dependencies related changes T-test Testing related changes T-config Configuration file changes T-core Core library changes T-handler Handler implementation changes T-model Model/data structure changes T-transport Transport layer changes labels Jun 26, 2026
@DaleSeo
DaleSeoforce-pushed the chore/rstest-lint-cleanup branch from fb0ff47 to 2f62089CompareJune 26, 2026 18:47
@DaleSeoDaleSeo self-assigned this Jun 26, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review June 26, 2026 18:51
@DaleSeo
DaleSeo requested a review from a team as a code ownerJune 26, 2026 18:51
use tokio::sync::{Mutex, Notify};
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};

pub struct Server {}

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

With pub, Clippy asks for #[non_exhaustive].

@DaleSeo
DaleSeo merged commit e1af378 into mainJun 26, 2026
19 checks passed
@DaleSeo
DaleSeo deleted the chore/rstest-lint-cleanup branch June 26, 2026 20:15
@github-actionsgithub-actionsBot mentioned this pull request Jun 26, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-dependenciesDependencies related changesT-handlerHandler implementation changesT-modelModel/data structure changesT-testTesting related changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@DaleSeo@alexhancock