Squashed 0944d8a (check_timeout) into one commit #35

Closed
wants to merge 21 commits into from

3 participants

@ianheggie

Hi I have squashed it all into one commit - let me know which you prefer and I'll kill the other (I prefer this one if you don't care).

Once one or either is merged, then I have some fixes for the bugs that where found to submit.

Squashed 0944d8a (check_timeout) into one commit:

Simplify and expand tests, adding tests for:

  • timeout conditions
  • unreachable hosts and routes
  • open and unused ports

Added test/setup_ubuntu.bash to setup unreachable host, route,
and confirm we can still use telstra.com as a black hole
for pings and tests for ports 7, 22 and 1001

Refactored tests to line up with existing example in external

Add extra rvm versions;
Add IGNORE_HTTP_PING_DOES_NOT_WAITS_FOR_RESPONSE_BUG and
IGNORE_HTTP_PING_DOES_NOT_TIMEOUT_BUG to test those bugs with
failure allowed;

Reorganise .travis.yml to make intent clearer

Speed up timeout tests

Give more details on failing tests

and others added some commits Feb 25, 2014
@djberg96 Commented out a test that usually failed. 3ccaee9
@ianheggie ianheggie Add travis tests for various ruby versions, with 1.8.7 and jruby mark…
…ed as allowed failures
e391c27
@ianheggie ianheggie Commented out jruby 18mode and 19mode till it is fixed so
we don't repeat three copies of it freezing for 10 minutes
1c0a6f7
@djberg96 Merge pull request #29 from ianheggie/add_travis_tests
Add travis tests for various ruby versions, with 1.8.7 and jruby marked ...
55d00af
@ianheggie ianheggie Add ICMP tests to travis-ci (refactored exclusions to work
with rvmsudo);
Drop ruby 1.8.7 test;
Fix ping of 192.168.0.1 to use 127.0.0.1 instead;
Move handling of EXCLUDE_ICMP_TEST to the same place as other excludes;
9a0748f
@ianheggie ianheggie Add to Known Issues in README based on testing results 6c59c70
@djberg96 Merge pull request #31 from ianheggie/add_icmp_tests_to_travis
Add icmp tests to travis, drop ruby 1.8.7, note known issues discovered in testing
c699a8a
@bcandrea bcandrea Fixed timeouts in Net::Ping::External
Using Timeout.timeout with Open3 is not an option with Ruby > 1.9.3
(see https://bugs.ruby-lang.org/issues/5487). Implemented a workaround
based on the code at https://gist.github.com/lpar/1032297
77fd65a
@bcandrea bcandrea Update travis.yml to trigger CI run
This update should trigger a Travis CI build on different Rubies.
6ae1b88
@djberg96 Merge pull request #32 from bcandrea/develop
Fixed timeouts in Net::Ping::External
61cd0a4
@djberg96 Don't use read_nonblock on Windows. bb83bc9
@bcandrea bcandrea Merge pull request #1 from djberg96/master
Merge back changes from djberg96
c034875
@bcandrea bcandrea Fix a typo in the Net::Ping::External test
This typo had the effect of setting the expected
ping duration to 5.5s instead of 1.5s. Since in Windows
the defaul ping timeout is 4s, the test always passed.
ef80ced
@bcandrea bcandrea Avoid implementing timeout on Windows
The -w option of ping on Windows has the same effect
of an external timeout.
9fe37ec
@bcandrea bcandrea Merge branch 'feature/fix-win32-external-ping' into develop c805658
@djberg96 Merge pull request #33 from bcandrea/develop
Fix a typo in the unit test for Net::Ping::External and change the implementation for Windows
a4ae2bd
@djberg96 Remove comment about win32-open3, add note about minimum Ruby version. 666474e
@ianheggie ianheggie Squashed 0944d8a (check_timeout) into one commit:
coSimplify and expand tests, adding tests for:
* timeout conditions
* unreachable hosts and routes
* open and unused ports

Added test/setup_ubuntu.bash to setup unreachable host, route,
and confirm we can still use telstra.com as a black hole
for pings and tests for ports 7, 22 and 1001

Refactored tests to line up with existing example in external

Add extra rvm versions;
Add IGNORE_HTTP_PING_DOES_NOT_WAITS_FOR_RESPONSE_BUG and
IGNORE_HTTP_PING_DOES_NOT_TIMEOUT_BUG to test those bugs with
failure allowed;

Reorganise .travis.yml to make intent clearer

Speed up timeout tests

Give more details on failing tests
9fdfc69
@djberg96
Owner

Upon further reflection, I've decided to go back to just using the command line arguments. I think I lost sight of the purpose of the External class, which is to just wrap an external timeout. If folks need more granular timeout control, I think they shouldn't use an external ping to begin with.

I don't want you to think I don't appreciate your contribution - I do! And you will still get credit for spotting the useless Timeout wrapper and providing a solution.

ianheggie added some commits Apr 2, 2014
@ianheggie ianheggie Change UNREACHABLE_HOST to using a script to try and find an unreacha…
…ble host (using iptables doesn't work for host unreachable);

Don't enforce that icmp pings have to wait if they discover network unreachable early;
8453f54
@ianheggie ianheggie Merge remote-tracking branch 'upstream/master'
Conflicts: (resolved in favour of upstream changes):
	README
	lib/net/ping/external.rb
	test/test_net_ping_external.rb
f6bd725
@ianheggie

Just looked at your change to remove time-out from external - I like what you did. I note that you didn't keep the (timeout*1000).to_i.to_s for windows - I suspect that was to make sure there where no fractional milliseconds in the command line arg.

@ianheggie ianheggie Merge branch 'master' (and upstream) into extra_timeout_and_unreachab…
…le_tests

Conflicts:
	test/test_net_ping_external.rb
a599135
@ianheggie ianheggie referenced this pull request Apr 2, 2014
Closed

Fix icmp enetunreach bug #36

@djberg96 djberg96 closed this Apr 2, 2014
@ianheggie ianheggie deleted the ianheggie:extra_timeout_and_unreachable_tests branch Apr 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment