Skip to content
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

Make ConnectTimeout test accept NoRouteToHostException #1526

Merged
merged 2 commits into from Jul 11, 2019

Conversation

@ringerc
Copy link
Member

commented Jul 11, 2019

The ConnectTimeout test expects an exception of type SocketTimeoutException when connecting to the unroutable address 10.255.255.1. But at least on my host (Fedora 30, OpenJDK 1.8.0_212) the connection attempt actually throws java.net.NoRouteToHostException.

That is an entirely reasonable exception to throw in this case, but not the intent of the test.

I think the test is intended to time out while connecting to the unreachable address. But it relies on highly platform-dependent behaviour. In particular if there's any sort of negative caching in place the test may fast-fail:

~/.../2Q/pgjdbc (miscfix *%)$ time ping -c 1 10.255.255.1
PING 10.255.255.1 (10.255.255.1) 56(84) bytes of data.
From 61.245.136.1 icmp_seq=1 Packet filtered

--- 10.255.255.1 ping statistics ---
1 packets transmitted, 0 received, +1 errors, 100% packet loss, time 0ms


real	0m0.122s
user	0m0.003s
sys	0m0.018s

rendering the test useless for the intended purpose.

I propose to treat this as a skipped test, per the associated pull request.

(I've also included a trivial fix for a normally-unreachable NPE I hit during testing here).

ringerc added 2 commits Jul 10, 2019
Skip ConnectTimeout timeout on hosts that throw NoRouteToHostException
If a host throws NoRouteToHostException before the ConnectTimeout, the test
has neither failed nor succeeded.

Github issue #1526

@ringerc ringerc force-pushed the ringerc:miscfix branch from 0290a52 to e08eea8 Jul 11, 2019

@davecramer

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

LGTM

@AppVeyorBot

This comment has been minimized.

Copy link

commented Jul 11, 2019

@AppVeyorBot

This comment has been minimized.

Copy link

commented Jul 11, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Jul 11, 2019

Codecov Report

Merging #1526 into master will increase coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #1526      +/-   ##
============================================
+ Coverage     68.84%   68.84%   +<.01%     
  Complexity     3943     3943              
============================================
  Files           179      179              
  Lines         16478    16480       +2     
  Branches       2675     2676       +1     
============================================
+ Hits          11344    11346       +2     
+ Misses         3888     3887       -1     
- Partials       1246     1247       +1

@davecramer davecramer merged commit 08d8129 into pgjdbc:master Jul 11, 2019

3 checks passed

codecov/project 68.84% (+<.01%) compared to ce8333a
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ringerc

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

Thanks Dave, much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.