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
fix: correct connect timeout setting for ApacheHttpRequest #803
fix: correct connect timeout setting for ApacheHttpRequest #803
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test?
Same comment as in #804: The Apache HttpClient implementation does a great job and making itself uninspectable once initialized - we would need to test its behavior (or use reflection to look at internals). For these transport adapters, we will need to set up an integration test that somehow ensures the desired behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one initially seemed harder to test, but I think @chanseokoh had a really good idea in the bug report that will enable us to test this one by setting a low timeout and connecting to a non-existent site.
I'm not sure why this fails on windows, but it seems to be within the Apache adapter. We should be able to skip this test on windows. |
@@ -183,6 +187,23 @@ public void process(HttpRequest request, HttpContext context) | |||
assertTrue("Expected to have called our test interceptor", interceptorCalled.get()); | |||
} | |||
|
|||
@Test(timeout = 10_000L) | |||
public void testConnectTimeout() { | |||
// Apache HttpClient doesn't appear to behave correctly on windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a bug filed against Apache we could reference here?
Fixes #799