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

Fix rspec failure when using webmock v2.2.0 or above #65

Merged

Conversation

yujideveloper
Copy link
Contributor

@yujideveloper yujideveloper commented Mar 10, 2017

This is a temporary fix to make rspec work well.

Webmock is now raising Net::OpenTimeout in to_timeout since v2.2.0.
However, faraday is raising Faraday::Error::ConnectionFailed when it rescued Net::OpenTimeout on v0.11.0

@yujideveloper yujideveloper changed the title Fix rspec fail when using webmock v2.2.0 or above Fix rspec failure when using webmock v2.2.0 or above Mar 10, 2017
@coveralls
Copy link

coveralls commented Mar 10, 2017

Coverage Status

Coverage increased (+0.07%) to 99.113% when pulling d9544c3 on yujideveloper:temporary-fix-for-rspec into 523414d on flexirest:master.

@execjosh
Copy link
Contributor

Sorry, I can't commit from where I'm at; so, I'm writing a comment.

Couldn't you check the Exception#cause like this?

--- a/lib/flexirest/connection.rb
+++ b/lib/flexirest/connection.rb
@@ -26,11 +26,17 @@ module Flexirest
       block.call
     rescue Faraday::Error::TimeoutError
       raise Flexirest::TimeoutException.new("Timed out getting #{full_url(path)}")
-    rescue Faraday::Error::ConnectionFailed
+    rescue Faraday::Error::ConnectionFailed => err
+      if err.cause.is_a?(Net::OpenTimeout)
+        raise Flexirest::TimeoutException.new("Timed out getting #{full_url(path)}")
+      end
       begin
         reconnect
         block.call
-      rescue Faraday::Error::ConnectionFailed
+      rescue Faraday::Error::ConnectionFailed => err
+        if err.cause.is_a?(Net::OpenTimeout)
+          raise Flexirest::TimeoutException.new("Timed out getting #{full_url(path)}")
+        end
         raise Flexirest::ConnectionFailedException.new("Unable to connect to #{full_url(path)}")
       end
     end

@andyjeffries
Copy link
Collaborator

Unfortunately Exception#cause is "new" to Ruby 2.1 and we support Ruby versions before that.

@execjosh
Copy link
Contributor

execjosh commented Mar 31, 2017

I can't figure out why bundle fails for ruby 1.9.3 on travis. I am unable to reproduce locally, even when using the same versions of everything. There is just no mention of mime-types-data gem anywhere...

Maybe some gem has been updated and the test just needs to be run again?

image
https://travis-ci.org/flexirest/flexirest/jobs/209645887#L158

root@f223efae985c:/flexirest/flexirest# ruby --version
ruby 1.9.3p551 (2014-11-13 revision 48407) [x86_64-linux]
root@f223efae985c:/flexirest/flexirest# rvm --version
rvm 1.26.10 (1.26.10) by Wayne E. Seguin <wayneeseguin@gmail.com>, Michal Papis <mpapis@gmail.com> [https://rvm.io/]
root@f223efae985c:/flexirest/flexirest# bundle --version
Bundler version 1.7.6
root@f223efae985c:/flexirest/flexirest# gem --version
2.4.5
root@f223efae985c:/flexirest/flexirest# bundle install --jobs=3 --retry=3 --path=vendor/bundle --binstubs=vendor/bin
Don't run Bundler as root. Bundler can ask for sudo if it is needed, and installing your bundle as root will break this
application for all non-root users on this machine.
Fetching gem metadata from https://rubygems.org/...........
Resolving dependencies...
Installing i18n 0.8.1
Installing rake 12.0.0
Installing minitest 5.10.1
Installing thread_safe 0.3.6
Installing addressable 2.4.0
Installing api-auth 2.1.0
Using bundler 1.7.6
Installing builder 3.2.3
Installing docile 1.1.5
Installing simplecov-html 0.10.0
Installing json 1.8.6
Installing tins 1.6.0
Installing thor 0.19.4
Installing safe_yaml 1.0.4
Installing diff-lcs 1.3
Installing multipart-post 2.0.0
Installing multi_json 1.12.1
Installing public_suffix 1.4.6
Installing hashdiff 0.3.2
Installing rspec-support 3.5.0
Installing tzinfo 1.2.3
Installing simplecov 0.14.1
Installing term-ansicolor 1.3.2
Installing crack 0.4.3
Installing ffi 1.9.18
Installing faraday 0.11.0
Installing rspec-core 3.5.4
Installing rspec-expectations 3.5.0
Installing rspec-mocks 3.5.0
Installing simplecov-rcov 0.2.3
Installing activesupport 4.2.8
Installing coveralls 0.8.20
Installing webmock 2.1.0
Installing ethon 0.10.1
Installing rspec 3.5.0
Installing rspec_junit_formatter 0.2.3
Using flexirest 1.3.33 from source at .
Installing typhoeus 1.1.2
Your bundle is complete!
It was installed into ./vendor/bundle
Post-install message from webmock:

  WebMock 2.0 has some breaking changes. Please check the CHANGELOG: https://goo.gl/piDGLu

@coveralls
Copy link

coveralls commented Apr 10, 2017

Coverage Status

Coverage increased (+0.1%) to 99.113% when pulling 4940be4 on yujideveloper:temporary-fix-for-rspec into 74ddf93 on flexirest:master.

Webmock is now raising `Net::OpenTimeout` in `to_timeout` since v2.2.0.
However, faraday is raising `Faraday::Error::ConnectionFailed` when it rescued
`Net::OpenTimeout` on v0.11.0

* https://github.com/bblimke/webmock/blob/v2.3.2/CHANGELOG.md#220
* bblimke/webmock#659
* https://github.com/lostisland/faraday/blob/v0.11.0/lib/faraday/adapter/net_http.rb#L29-L57
@coveralls
Copy link

coveralls commented Apr 18, 2017

Coverage Status

Coverage increased (+0.003%) to 98.998% when pulling 92d9a5b on yujideveloper:temporary-fix-for-rspec into 74ddf93 on flexirest:master.

@yujideveloper
Copy link
Contributor Author

yujideveloper commented Apr 18, 2017

Hi, @andyjeffries and @execjosh

Thank you for your review.
I updated this pr. Could you re-review this?

Copy link
Contributor

@execjosh execjosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

@andyjeffries
Copy link
Collaborator

Me too, thanks for the PR.

@andyjeffries andyjeffries merged commit 845ffb5 into flexirest:master Apr 21, 2017
@yujideveloper yujideveloper deleted the temporary-fix-for-rspec branch April 21, 2017 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants