Skip to content

Improve code coverage for HttpTransport#45

Merged
james2037 merged 1 commit into
masterfrom
feature/http-transport-coverage
Jun 5, 2025
Merged

Improve code coverage for HttpTransport#45
james2037 merged 1 commit into
masterfrom
feature/http-transport-coverage

Conversation

@james2037

Copy link
Copy Markdown
Owner

This commit addresses code coverage gaps in src/Transport/HttpTransport.php.

Key changes include:

  • receive method: I added tests to cover invalid item structures within batch JSON-RPC requests. I ensured that if an item in a batch is not a valid JSON object (e.g., a string or a list), a TransportException is thrown.
  • send method: I investigated an else branch previously thought to be reachable. I determined that PHP's runtime type hinting for method parameters (JsonRpcMessage|array|null) makes this branch effectively unreachable. I removed the unreachable else block.
  • isStreamOpen method: I added a test to confirm it correctly returns false, as the simplified transport does not support streaming.
  • isClosed method: I added tests to verify its state:
    • Returns false immediately after transport instantiation.
    • Returns true after the send() method has been called.
  • getResponse method: I added a test using reflection to cover a fallback scenario where the internal $this->response property might be null. This test ensures the method generates a proper error response in such a case.

All new tests pass, and existing tests continue to pass. Linters (PHPCS, PHPStan) also pass with these changes. The overall goal was to increase test coverage for the specified methods and conditions.

This commit addresses code coverage gaps in `src/Transport/HttpTransport.php`.

Key changes include:

- **receive method**: I added tests to cover invalid item structures within batch JSON-RPC requests. I ensured that if an item in a batch is not a valid JSON object (e.g., a string or a list), a `TransportException` is thrown.
- **send method**: I investigated an `else` branch previously thought to be reachable. I determined that PHP's runtime type hinting for method parameters (`JsonRpcMessage|array|null`) makes this branch effectively unreachable. I removed the unreachable `else` block.
- **isStreamOpen method**: I added a test to confirm it correctly returns `false`, as the simplified transport does not support streaming.
- **isClosed method**: I added tests to verify its state:
    - Returns `false` immediately after transport instantiation.
    - Returns `true` after the `send()` method has been called.
- **getResponse method**: I added a test using reflection to cover a fallback scenario where the internal `$this->response` property might be `null`. This test ensures the method generates a proper error response in such a case.

All new tests pass, and existing tests continue to pass. Linters (PHPCS, PHPStan) also pass with these changes. The overall goal was to increase test coverage for the specified methods and conditions.
@github-actions

github-actions Bot commented Jun 5, 2025

Copy link
Copy Markdown

🧪 Test Results Summary

PHPUnit Tests

✅ All tests passed!

Details: OK (238 tests, 954 assertions)

  • Total Tests: 238
  • Total Assertions: 954

Code Quality

  • PHPCS: ✅ No issues
  • PHPStan: ✅ No errors found

📁 Detailed Reports

  • Coverage Report: Download the test-outputs artifact for detailed HTML coverage report
  • Test Results: Check the "Tests" tab above for detailed test results
  • Raw Logs: View the "Actions" tab for complete output logs

This comment will update automatically when you push new commits.

Code Coverage

Code Coverage

Package Line Rate Health
Capability/CapabilityInterface.php 0%
Capability/ResourcesCapability.php 100%
Capability/ToolsCapability.php 100%
Exception/InvalidParamsException.php 0%
Exception/InvalidRequestException.php 0%
Exception/MethodNotSupportedException.php 0%
Exception/TransportException.php 0%
Message/JsonRpcMessage.php 100%
Registry/Registry.php 97%
Resource/Attribute/ResourceUri.php 100%
Resource/BlobResourceContents.php 100%
Resource/Resource.php 100%
Resource/ResourceContents.php 100%
Resource/ResourceRegistry.php 100%
Resource/TextResourceContents.php 100%
Server.php 69%
Tool/Attribute/Parameter.php 100%
Tool/Attribute/Tool.php 100%
Tool/Attribute/ToolAnnotations.php 100%
Tool/Content/AbstractContent.php 100%
Tool/Content/AbstractMediaContent.php 100%
Tool/Content/Annotations.php 100%
Tool/Content/AudioContent.php 100%
Tool/Content/ContentItemInterface.php 0%
Tool/Content/EmbeddedResource.php 100%
Tool/Content/ImageContent.php 100%
Tool/Content/TextContent.php 100%
Tool/Tool.php 100%
Tool/ToolRegistry.php 100%
Transport/HttpTransport.php 100%
Transport/StdioTransport.php 96%
Transport/TransportInterface.php 0%
Summary 91% (870 / 955)

@codecov

codecov Bot commented Jun 5, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@james2037 james2037 merged commit 7f36906 into master Jun 5, 2025
3 checks passed
@james2037 james2037 deleted the feature/http-transport-coverage branch June 5, 2025 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant