Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the external service URLs used in integration tests from nghttp2.org to httpbin.org. Feedback suggests replacing these external dependencies with a local mock server to improve test reliability and ensure the tests are hermetic.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the integration tests for ApacheHttp2Transport by replacing external network dependencies with a local FakeServer implementation. This change improves test reliability and execution speed. However, the review feedback identifies a likely copy-paste error in the read timeout tests, where the assertions incorrectly expect a 'Connection Timeout' message instead of a 'Read Timeout' message.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the ApacheHttp2TransportIT integration tests to use a local FakeServer instead of external endpoints, improving test reliability. The feedback suggests simplifying the FakeServer constructor using the ServerBootstrap builder and utilizing the close() method for more idiomatic resource management.
The previous test links caused failures since their targets were moved to a new link. These tests were already flaky due to hitting an external live endpoint and to mitigate this we are converting to using a local server to validate these tests instead. Using a local server makes testing the Read and Write timeouts difficult and as a result these were removed and rely on unit tests for coverage.