Reset webmock after all after(:each/example) hooks

[drews256]

I was recently working on mocking out some requests and ran into an issue where I was using webmock to forward mocked requests to a rack app.

I had wrapped a bunch of specs like:

Rspec.configure do |config|
  config.around(:each) do |example|
    some_mocking_setup
    example.run  # <- makes a bunch of requests to the mocked endpoints
    some_mocking_teardown # <- uses some of my mocked requests to tell the rack service to reset 
  end
end

And was surprised when some_mocking_teardown was throwing errors about me attempting my mocking teardown.

I am able to successfully accomplish what I need by doing:

Rspec.configure do |config|
  config.before(:each) do |example|
    some_mocking_setup
    example.run  # <- makes a bunch of requests to the mocked endpoints
  end

  config.after(:each) do |example|
    some_mocking_teardown # <- uses some of my mocked requests to tell the rack service to reset 
  end
end

I would expect either option to work.

I have included a proposed pr to only reset webmock after the config.around(:each) in the webmock configuration.

Thoughts on this?

[bblimke]

@drews256 thank you for pointing that. I agree around hook makes sense. My only concern is if it's possible that this change will break some of the existing WebMock setups that people have already created.

[drews256]

Definitely agreed. Any thoughts on some more specs to see if we can make it break. The cool thing about around is rspec (according to the docs) treats the spec like a block and should encapsulate the spec. But, I have not looked at the intricacies of how rspec handles the difference between these two.

Could we toggle this as some sort of setting and deprecate the old way? Or maybe just leave it as some sort of setting?

I feel like at the minimum it should probably be documented? It didn't take too long to figure out what was going on, but might be nice to mention.

[bblimke]

Nobody has raised that issue before, therefore I don't think it would affect many users. I don't think that many people set expectations in after or a suffix of the around block, therefore I'm even keen on releasing a minor version with that change, and treat previous implementation with after hook as a flaw.

I'd like to avoid configuration toggles as it only leads to complexity.

[drews256]

Perfect. Well, let me know what the next steps are.

[davidstosik]

We've had issues recently caused by the order in which webmock/rspec and capybara/rspec are required during the spec initialization process.

The reason was that if WebMock.reset! happens before Capybara.reset_sessions!, then there is a chance the browser (eg. headless Chrome) is still running and responsible for extra requests that will land in WebMock's registry for the next spec.

I have identified a few different ways to fix that:

• fix the require order, by adding require "webmock/rspec" in spec/spec_helper.rb, as recommended by WebMock's documentation, instead of later, in spec/rails_helper.rb.
• add my own config.append_after(:each) { WebMock.reset! }
• in the WebMock gem, use append_after or before instead of after:

webmock/lib/webmock/rspec.rb

Lines 36 to 38 in 382d84c

	config.after(:each) do
	WebMock.reset!
	end

It seems that using around would have a similar beneficial impact, as the "late part" of an around block would get executed after any after block!
This would only move the problem somewhere else, as WebMock would now be subject to order interference with other gems that add an around block.

[bblimke]

@davidstosik thank you for the useful feedback.

@drews256 ok, let's give it a try. I see it as an improvements, but we can rollback this change in case we get any complaints.

[bblimke]

This is now released as version 3.9.0