Skip to content

test(server): add more tests for SSEServerTransport class #391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 26, 2025

Conversation

christian-bromann
Copy link
Contributor

Motivation and Context

I've been skimming through the code and discovered that there is an opportunity to add more tests to the SSEServerTransport class. That's it!

Note: the package-lock.json change is not intended but it seems to be missing the recent update. Happy to revert though.

How Has This Been Tested?

I added unit tests.

Breaking Changes

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

Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

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

Thank you for improving SDK. Sorry about the delay in PR reviews. Those files have conflicts now, please can you merge main and please revert package-lock changes

Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

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

Thank you

Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

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

oh, looks like test "should return 202 if message has a valid schema" is failing in CI

Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

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

🙏

@ihrpr ihrpr merged commit be845dd into modelcontextprotocol:main Jun 26, 2025
2 checks passed
@christian-bromann christian-bromann deleted the cb/sse-tests branch June 26, 2025 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants