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

Automatically disable WebMock after rspec suite #731

Merged

Conversation

d4rky-pl
Copy link
Contributor

When using ruby-codacy-coverage we were forced to whitelist Codacy by hand, even though the code coverage was being sent long after the entire test suite has run (in at_exit block in simplecov).

This was a surprising behaviour as the point of requiring webmock/rspec is to have a seamless integration that doesn't interfere with other tooling.

This PR changes the default behaviour of webmock/rspec so it does not by default interfere with test configuration.

@bblimke
Copy link
Owner

bblimke commented Nov 30, 2017

@d4rky-pl thanks for the research and the pull request. That looks useful!

I'm trying to think if this change is sufficiently backwards compatible and if it can break an existing setup for anyone.

I believe this could only break if someone does the following currently:

require 'webmock/rspec'
WebMock.disable!

The other option is that someone takes advantage of WebMock being enabled,
before the before(:suite) callbacks are invoked.

Both scenarios are fairly unlikely, don't you think?

@d4rky-pl
Copy link
Contributor Author

I was hoping to start a discussion around this. Right now the current setup also makes sure that no network calls are called for the setup stage, duration of entire suite, end then some. For some setups it definitely makes sense to avoid any possible network calls (testing in total isolation, CI with no network adapter, making sure your tests are impervious to network issues etc). On the other hand, require "webmock/rspec" can make a person assume that this is only the rspec integration and not "disable everything for as long as process is running".

I was frankly more worried about the exact opposite case, which would be not being able to enable webmock for both test suite and additional tooling (for ex. for the previously mentioned testing in total isolation) - even turning webmock manually on will still disable it after running a full suite so to enable it you'd have to require just webmock and include webmock helpers manualy (or add some extra way of disabling the after callback / reenabling webmock)

I don't think there's a good or bad solution in here but my PR do change the default behaviour. It might be a good idea to either merge it on full minor/major version, add extra step to make it work the way I'm proposing (require + Webmock.enable_rspec!? disable_after_suite?) or at the very least add a disclaimer in README that says the rspec integration only works for the duration of test suite and not for setup files and after suite callbacks.

Choices, choices...

@bblimke
Copy link
Owner

bblimke commented Dec 4, 2017

@d4rky-pl the chance of someone being affected by that change seems so low to me that I'm tempted to do a minor version release, since it's an improvement over how webmock/rspec worked so far, and is also a bug fix in a way.

@d4rky-pl
Copy link
Contributor Author

d4rky-pl commented Dec 4, 2017

Yeah, after thinking this through I agree

@bblimke bblimke merged commit ba69c1d into bblimke:master Dec 6, 2017
davidstosik added a commit to davidstosik/rspec_api_documentation that referenced this pull request Oct 2, 2020
3.2.0 introduced another breaking change that had to be addressed:

[Automatically disable WebMock after rspec suite](bblimke/webmock#731)

As the example app_spec.rb in features/oauth2_mac_client.feature is supposed to be a stand-alone spec file, it made sense, to me, to `require "webmock/rspec"`.
Does that make any sense?
jakehow pushed a commit to zipmark/rspec_api_documentation that referenced this pull request Oct 2, 2020
* Test more recent versions of Ruby on Travis

Added:
- 2.4.9 (reached end-of-life but may temporarily help in incremental debugging)
- 2.5.8
- 2.6.6

Did not add 2.7.x yet because it introduces *a lot* of warnings.

Did not remove any past version yet because until decided otherwise,
they should be supported by the gem. (I would recommend a major version
bump when those versions get dropped.)

The idea is that developments from now on should be future-proof, and
changes should be tested on current versions of Ruby.

* Update WebMock to latest 2.x version

Updating to WebMock 2.x means that:

> require 'webmock' does not enable WebMock anymore. gem 'webmock' can
now be safely added to a Gemfile and no http client libs will be
modified when it's loaded. Call WebMock.enable! to enable WebMock.
>
> Please note that require 'webmock/rspec', require 'webmock/test_unit',
require 'webmock/minitest' and require 'webmock/cucumber' still do
enable WebMock.

Source: [WebMock's
CHANGELOG.md](https://github.com/bblimke/webmock/blob/master/CHANGELOG.md#200).

As rspec_api_documentation is very much tied to RSpec, I saw no problem
with replacing all instances of `require 'webmock'` with `require
'webmock/rspec'`.

* Update WebMock dependency to 3.2.0

3.2.0 introduced another breaking change that had to be addressed:

[Automatically disable WebMock after rspec suite](bblimke/webmock#731)

As the example app_spec.rb in features/oauth2_mac_client.feature is supposed to be a stand-alone spec file, it made sense, to me, to `require "webmock/rspec"`.
Does that make any sense?

* Bump WebMock dev dependency to 3.5.0

This introduces Ruby 2.6 support and should get the test suite to run on that version of Ruby.

* Try using WebMock 3.8.3

Let's see how this goes on Travis.
E1337Kat pushed a commit to E1337Kat/rspec_api_documentation that referenced this pull request Mar 5, 2024
* Test more recent versions of Ruby on Travis

Added:
- 2.4.9 (reached end-of-life but may temporarily help in incremental debugging)
- 2.5.8
- 2.6.6

Did not add 2.7.x yet because it introduces *a lot* of warnings.

Did not remove any past version yet because until decided otherwise,
they should be supported by the gem. (I would recommend a major version
bump when those versions get dropped.)

The idea is that developments from now on should be future-proof, and
changes should be tested on current versions of Ruby.

* Update WebMock to latest 2.x version

Updating to WebMock 2.x means that:

> require 'webmock' does not enable WebMock anymore. gem 'webmock' can
now be safely added to a Gemfile and no http client libs will be
modified when it's loaded. Call WebMock.enable! to enable WebMock.
>
> Please note that require 'webmock/rspec', require 'webmock/test_unit',
require 'webmock/minitest' and require 'webmock/cucumber' still do
enable WebMock.

Source: [WebMock's
CHANGELOG.md](https://github.com/bblimke/webmock/blob/master/CHANGELOG.md#200).

As rspec_api_documentation is very much tied to RSpec, I saw no problem
with replacing all instances of `require 'webmock'` with `require
'webmock/rspec'`.

* Update WebMock dependency to 3.2.0

3.2.0 introduced another breaking change that had to be addressed:

[Automatically disable WebMock after rspec suite](bblimke/webmock#731)

As the example app_spec.rb in features/oauth2_mac_client.feature is supposed to be a stand-alone spec file, it made sense, to me, to `require "webmock/rspec"`.
Does that make any sense?

* Bump WebMock dev dependency to 3.5.0

This introduces Ruby 2.6 support and should get the test suite to run on that version of Ruby.

* Try using WebMock 3.8.3

Let's see how this goes on Travis.
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.

2 participants