rspec / rspec-core

RSpec runner and formatters
http://rspec.info
MIT License
1.23k stars 765 forks source link

Add config option to disable all monkey patching #1433

Closed myronmarston closed 10 years ago

myronmarston commented 10 years ago

This is the last thing mentioned in my the plan for rspec 3 blog post that we haven't done. Currently disabling monkey patching involves setting all of this config:

RSpec.configure do |rspec|
  rspec.expose_dsl_globally = false

  rspec.mock_with :rspec do |mocks|
    mocks.syntax = :expect

    # not strictly necessary but listing it for completeness
    mocks.patch_marshal_to_support_partial_doubles = true
  end

  rspec.mock_with :rspec do |expectations|
    expectations.syntax = :expect
  end
end

That's a lot to type. We should expose a single config option that will take care of all of it:

RSpec.configure do |rspec|
  rspec.disable_monkey_patching = true

  # or, if we're not going to support changing it to false:
  rspec.disable_monkey_patching!
end

One gotcha with this config option is that disable_monkey_patching as a method name does not suggest that it will configure rspec to use rspec-mocks and rspec-expectations. I think that it should only configure rspec-mocks and rspec-expectations if the user is using those (either explicitly or implicitly by not setting mock_with or expect_with to anything else).

When the first example group is defined, we default mock_with and expect_with to the rspec defaults if the user hasn't set it to something else:

https://github.com/rspec/rspec-core/blob/0fd9a6c4f63eab72993ae7de977d05049db8e260/lib/rspec/core/example_group.rb#L396-L402

...so I think that this new config options should behave as follows:

It should be noted that if the user uses this option and mock_with :mocha (or similar) that they will still have monkey patching active in their test environment from mocha, but I believe that's the correct behavior. It simply disables monkey patching on whatever pieces of rspec the user is using.

waterlink commented 10 years ago

Hi @myronmarston

I would like to be assigned to this issue. This is my first deep dive into rspec-core though. I've already read relevant specs and code and come to understanding how this should be done.

I prefer to implement this config option as a method.

I have already created this method and made it working through tdd by writing specs. I have put them to additional spec file spec/rspec/core/disable_monkey_patching_spec.rb, so it will be easy to manage them until it is done. These examples should go to configuration_spec.rb and to example_group_spec.rb before it is done.

Next step, as I understand, is to create cucumber bdd/documentation scenarios, where this configuration option is used.

But I require some strong code review, 'cause it is my first contribution to rspec. I already referenced this issue from commit, where I have done this config option already, so you'll be able to give me some feedback.

If it is OK for me to be assigned for this issue, I'll do it gladly. Hope to hear from you soon.

Sincerely, -Alex.

myronmarston commented 10 years ago

Sure. Open a PR and we'll do a code review and get the ball rolling. Thanks for stepping up!

xaviershay commented 10 years ago

Closing in favour of the linked PR.