disable_monkey_patching not preventing global alteration of describe

[tibbon]

I'm using Minitest 5.8.4 for some of my older tests. This includes minitest/spec that gives me rspec-like describe in my tests.

When bundle the gem rspec-rails 3.5.1 (which of course brings in rspec-core 3.5.1) all of the describe code in my minitests no longer work, and I get the error of undefined local variable or method 'describe' for MyTest:Class (where this is a subclass of ActionController::TestCase)

To attempt to remedy this, I've tried to disable RSpec's monkey patching, but this isn't seeming to work either.

RSpec.configure do |config|
  config.disable_monkey_patching!
end

I believe (although I'm missing a few things), that this is because when you disable_monkey_patching, it isn't preventing it, but rather attempting to undo the prior changes. Related bits of code from rspec-core below:

RSpec.configure do |config|
  config.disable_monkey_patching!
end

## disable_monkey_patching essentially does this and a few other things
RSpec.configure do |config|
  config.expose_dsl_globally = false
end

def expose_dsl_globally=(value)
  if value
    Core::DSL.expose_globally!
    Core::SharedExampleGroup::TopLevelDSL.expose_globally!
  else
    Core::DSL.remove_globally!
    Core::SharedExampleGroup::TopLevelDSL.remove_globally!
  end
end

# Removes the describe method from Module and the top level binding.
# @api private
def self.remove_globally!
  return unless exposed_globally?

  example_group_aliases.each do |method_name|
    change_global_dsl { undef_method method_name }
  end

  @exposed_globally = false
end

def self.change_global_dsl(&changes)
  (class << top_level; self; end).class_exec(&changes)
  Module.class_exec(&changes)
end

I think the change that needs to happen, is to prevent describe from ever attaching to top level binding to begin with, and not overriding it if it might have been defined elsewhere. Help/thoughts?

[tibbon]

Some more relevant code for what minitest/spec is doing can be found here:
https://github.com/seattlerb/minitest/blob/master/lib/minitest/spec.rb

[myronmarston]

I believe (although I'm missing a few things), that this is because when you disable_monkey_patching, it isn't preventing it, but rather attempting to undo the prior changes.

Yep, for backwards compatibility reasons we were forced to do it this way. Historically, calling describe on main has always worked, and for RSpec 3 we made the decision to allow people to opt-out of monkey patching but to keep things working as-is for folks who don't reconfigure it. AFAIK, there's no way to achieve that w/o defining the monkey patches by default and providing a config API to undo it, like we've done. If you can think of a way to improve this while maintaining backwards compatibility, please let us know!

In RSpec 4, we plan to extract all monkey patching from RSpec and move it into a separate gem, so that monkey patching is opt-in instead of opt-out and users have to explicitly install and load a gem to get it. That should solve the issue for you but as a breaking change we can't do it until RSpec 4 given our commitment to SemVer.

In the meantime, is there any reason you are loading both rspec-core and minitest in the same process? IMO, it doesn't make sense to load both. If you are using both, I'd expect you to run them as separate processes and not be both loaded in the same process at the same time. If you're using Bundler.require, just set :require => false on rspec-rails and rspec-core so it's not loaded by default. (Or consider moving away form Bundler.require entirely). If you avoid loading rspec-core in your minitest process, it should solve the issues you are seeing.

[tibbon]

Thank you so much. I totally forgot that I can just set :require => false, as you're correct that they don't need to be loaded in the same process.

But yea, I'm totally willing to put some time into a PR for this that's targetted at being merged into RSpec 4.

[myronmarston]

But yea, I'm totally willing to put some time into a PR for this that's targetted at being merged into RSpec 4.

We haven't started on RSpec 4 yet and have no plans to start in the near future, so such a PR would likely just sit stagnant and create extra work to resolve the merge conflicts in the future when we do start on RSpec 4.