chore: Fix broken integrations tests#1202
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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.
|
Closing this PR for now since the flaky tests have auto resolved themselves. If these test begin to flake frequently again in the future we will re-address the value of these tests. |
2 participants
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.