rspec DSL is globally exposed in development environment

[tmertens]

The default behavior of expose_dsl_globally was fixed in https://github.com/rspec/rspec-core/pull/1933 so that it works as expected when the configuration object is not instantiated by the tests. However, this had the side effect in rspec 3.3 that when requiring rspec-rails in the Gemfile for the :development and :test environments (for the loading of rake tasks), it causes the dsl to be injected globally into our development environment. This in turn results in conflicts with running our development environment where there are some calls to a context method which in certain cases ends up calling rspec's context method which then result in our application crashing when run in development mode.

I should additionally note that in the test environment, this is not a problem because we set expose_dsl_globally = false.

Workaround

In case anyone else runs into this, I did this as a workaround:

In Rakefile:

require File.expand_path('../config/application', __FILE__)

MyApp::Application.load_tasks

# This must come after the load_tasks call above.
if defined? RSpec
  RSpec.configuration.expose_dsl_globally = false
end

EDIT: The above workaround only fixes it for rake tasks. A more complete workaround is to put it in config/application.rb after Bundler.require for non-production environments:

  Bundler.require(:default, :assets, Rails.env)

  if defined? RSpec
    RSpec.configuration.expose_dsl_globally = false
  end

[myronmarston]

Thanks for the bug report. In RSpec 4 we plan to extract all the monkey patching to an external gem such that RSpec proper has no monkey patching whatsoever. Once that's done, this won't be a problem anymore.

Your work around looks reasonable, but here's another potential solution: add require: false to the rspec-rails entry in your Gemfile. Unless I'm missing something, it seems like you don't need rspec required in your dev environment at all, right?

[tmertens]

it seems like you don't need rspec required in your dev environment at all, right?

See the first paragraph: https://github.com/rspec/rspec-rails#installation

[myronmarston]

I know about that, but my (perhaps mistaken understanding) was that the rspec-rails gem had to be available to be loaded in the dev environment (in order to allow use from generators and such) but that it doesn't necessarily need to always be loaded.

@cupakromer knows more here. I'm not a rails user.

[tmertens]

Ah, I suppose we could load it only in the Rakefile; however, there are still issues there as well, as this actually breaks some data seed tasks for us in rake tasks, so that is unfortunately not a complete solution - we would still need to explicitly disable the global DSL as a workaround.

[myronmarston]

I don't think you need to load RSpec as a whole in the Rakefile. In rspec-core, the rspec/core/rake_task is designed to be loaded individually in a rake file and it doesn't load any parts of RSpec -- it just creates a shell command that runs RSpec when needed. In general, in my apps, I don't load any gems globally in my Rakefile -- I have each task require the files and libraries that it needs to work. That way, you can run rake -T and get the list really fast and each task loads only what it uses. It keeps things nice and snappy.

I'm not sure about the rspec-rails stuff but I think you could get it to work there as well.

[cupakromer]

in order to allow use from generators and such

Yes. That is the main reason. You can see everything we do in lib/rspec-rails.rb. Currently this boils down to:

• hook into generators
• load our Rake task
• shim the ActionMailer setup

as this actually breaks some data seed tasks for us in rake tasks

@tmertens could you elaborate more on this. This is the first I've heard of the RSpec patches braking seeds. :frowning: I'm very sorry this is happening in your environment. I'd like to better understand why so we can evaluate further possibilities.

[tmertens]

@cupakromer

I don't fully understand the part of our codebase where this was a problem, there is a lot of metaprogramming / duck typing involved here, but the basic problem is that context is being called at the class level if it is defined on the class:

class MyClass
  def self.do_something(thing)
    foo = self.context(thing) if self.respond_to?(:context)
    # do more things
  end
end

thing.context doesn't resolve if it is an object instance which doesn't implement the context method; however, in this case thing is a class/module, so both self.context(thing) and self.respond_to?(:context) will resolve to the method defined on Kernel by RSpec if the class doesn't provide its own implementation. This happens deep in some code run by an observer which is triggered when saving/updating certain records, hence it causes failures when saving/updating records (seeding the DB) and/or loading pages and performing certain actions when the app is running as well in development. This wasn't caught by our test suite, because we explicitly disable the global dsl in the test environment.

You can observe this behavior by opening a rails console in development with rspec-rails in your gemfile per the Readme, then run:

[5] pry(main)> class Foo; end
=> nil
[6] pry(main)> Foo.context
=> RSpec::ExampleGroups::Anonymous
[7] pry(main)> context
=> RSpec::ExampleGroups::Anonymous_2
[8] pry(main)> Foo.new.context
NoMethodError: undefined method `context' for #<Foo:0x007f821f31c010>
from (pry):9:in `<main>'
[9] pry(main)> Foo.respond_to? :context
=> true

[cupakromer]

@tmertens thanks for that thorough explanation. That really helps out. I'm mulling over some ideas, but unfortunately the gemspec options for dependencies are minimal; compared to what Bundler allows you to use in the Gemfile.

I don't think lib/rspec-rails.rb needs to load the rest of RSpec; well perhaps part of it for the Rake task. However, due to how Rails uses Bundler.require by default and how bundler handles dependencies listed in gem specs our options seem fairly limited.

[att14]

I guess this ticket is specifically about the development environment, but we had a problem with this in the test environment. We use collectiveidea/interactor pretty extensively, which has a context instance method. A pattern some of the teams use is to define a delegate at the top of an Interactor to show what are valid values that can be passed in as context since context is just an OStruct that can have any key and any nonexistent key returns nil.

We had a situation where a delegate was defined,

delegate :foo, to: context

Since RSpec injects context into Kernel this delegate does not cause a NameError at import time. Unfortunately the delegate was never used in the code so none of the tests failed. The issue wasn't discovered until deployment.

In this particular service we are running rails (4.2.10), rspec (3.7.0) and rspec-rails (3.7.2). We have both config.disable_monkey_patching! and config.expose_dsl_globally = false set.

There is another unrelated issue that arose while looking into this that might help some other people. IRB actually inserts the method context into the REPL so this issue did not manifest itself in rails c either.

[JonRowe]

@att14 it's the same issue, you need to control the load order of RSpec to prevent it, hopefully when RSpec 4 is released the issue will go away

[d4rky-pl]

Hello from the future 👋 It's been 6 years, RSpec 4 does not seem to have materialized but this problem still exists.

Maybe rspec-rails should disable the monkeypatch by default when in development?

[JonRowe]

The next major version of rspec-rails could turn this off by default and let people enable it themselves if you'd like to submit a PR adding that to the railtie.

[pirj]

Also RSpec 4 is cooking, just no promises on the release date.

[d4rky-pl]

@JonRowe sounds like a plan, I'll take a look sometime this weekend

@pirj good to know! I kind of assumed without checking that the big rewrite has been dropped in favour of continuous support and improvement of 3.x instead

[pirj]

No rewrite, just a cleanup. The existing code is good already.