Unit Test Fix - Update concurrency test duration threshold and logging for clarity #878
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
I made two important changes to fix failing tests in the MCP Python SDK project:
The Concurrency Test Fix
Original Problem:
The concurrency test (
test_messages_are_executed_concurrently
) was failing because it had an overly optimistic timing expectation. The test was asserting that 20 concurrent tasks (10 tool calls and 10 resource reads), each with a 0.01-second sleep, would complete in under 0.06 seconds (6 * _sleep_time_seconds
). However, the actual execution time was around 0.11 seconds.Why the fix is needed:
This test was meant to verify that operations happen concurrently, but the threshold was unrealistically strict. The assertion was failing because:
Solution:
I increased the time threshold to a more realistic value (
12 * _sleep_time_seconds
) that still validates the concurrency behavior. The test still confirms that tasks run in parallel (since serial execution would take20 * _sleep_time_seconds
), but it's now more tolerant of real-world execution environments.The Snapshot Test Fix
Original Problem:
The
test_build_metadata
test was failing with aUsageError: snapshot value should not change
error. The test was using inline-snapshot to compare OAuth metadata objects. The error specifically mentioned that the issuer URL value had changed fromhttps://auth.example.com/
tohttps://auth.example.com/v1/mcp
for thewith-path-param
test case.Why the fix is needed:
Snapshot tests are useful for catching unintended changes, but in this case:
Since the test is parametrized, it needs to handle different expected values
Solution:
I replaced the rigid snapshot comparison with direct attribute-by-attribute assertions. This approach:
Both of these changes maintain the original intent of the tests while making them more reliable, especially in CI environments where timing and exact object representation can vary.
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context