Skip to content

Conversation

@maxisbey
Copy link
Contributor

Fixes flaky test failures in test_sse_security_wildcard_ports by replacing arbitrary sleep with deterministic server readiness polling.

Motivation and Context

The test_sse_security_wildcard_ports test was intermittently failing in CI with:

httpx.ConnectError: All connection attempts failed

The root cause was a race condition: the test used time.sleep(1) to wait for the uvicorn server to start, but this was sometimes insufficient, causing the test to attempt connections before the server was ready.

How Has This Been Tested?

  1. Reproduced the issue: Reduced the sleep time to 0.01s to reliably trigger the race condition and confirmed the same connection error
  2. Verified the fix: Ran the specific test 10 times consecutively - all passed
  3. Regression testing: Ran all 8 SSE security tests - all passed
  4. Performance: Tests now complete in similar or faster time since they don't wait unnecessarily

Breaking Changes

None. This is purely an internal test improvement.

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

The new wait_for_server() function:

  • Actively polls the server port by attempting TCP connections
  • Returns immediately when the server accepts connections (faster than fixed sleep)
  • Uses small 0.01s intervals between attempts to avoid CPU spinning
  • Has a clear 5-second timeout that raises TimeoutError on true failures
  • Makes tests deterministic instead of probabilistic

This follows the principle of avoiding arbitrary sleeps in unit tests that can make CI slower and less reliable.

The test_sse_security_wildcard_ports test was flaky due to a race condition
where the test tried to connect before uvicorn was ready to accept connections.

The original code used time.sleep(1) which was:
- Sometimes too short (causing flaky CI failures)
- Always wasteful when server started quickly
- Non-deterministic

This change replaces the arbitrary sleep with wait_for_server() which:
- Actively polls the server port until it accepts connections
- Returns immediately when ready (faster in most cases)
- Has a clear 5-second timeout for true failures
- Eliminates the race condition entirely

The fix makes the tests deterministic and often faster than before.
@maxisbey maxisbey added bug Something isn't working P3 Nice to haves, rare edge cases labels Oct 28, 2025
Copy link
Contributor

@felixweinberger felixweinberger left a comment

Choose a reason for hiding this comment

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

Nice improvement, thanks!

@felixweinberger felixweinberger merged commit f97f7c4 into main Oct 28, 2025
21 checks passed
@felixweinberger felixweinberger deleted the fix-flaky-sse-security-test branch October 28, 2025 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working P3 Nice to haves, rare edge cases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants