Skip to content
This repository

Deprecation warnings when using different requires for spec_helper #371

Closed
jacobat opened this Issue · 21 comments

9 participants

Jacob Atzen Myron Marston David Chelimsky Sebastian Roth Midwire Steven Ringo Gary Bernhardt Corey Haines John Trupiano
Jacob Atzen
jacobat commented

After upgrading a Rails project to 2.6.0 I get the following warning when I run my spec suite with rake spec:

You have set some configuration options after an example group has
already been defined.  In RSpec 3, this will not be allowed.  All
configuration should happen before the first example group is
defined.  The configuration is happening at:

The issue seems to be caused by different ways of requiring spec_helper. One spec has:

require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')

Another has:

require 'spec_helper`

Each spec will run without warning when run individually, but when run together the warning appears. After changing the requires to both use the expanded form the deprecation notice disappears.

Not sure if this is to be considered a bug or just me doing things the wrong way.

David Chelimsky
Owner

You should require spec_helper the same way, and I'd recommend the short version. This is not only to silence this deprecation, but spec_helper is typically used to load other resources and reloading it has lead to some confusing bugs in the past.

I'm going to leave this open as I think this warrants some documentation.

Sebastian Roth
ened commented

Thanks for keeping this open! Just trapped into it. Took me quite a while to find this page.

Midwire

Short version require 'spec_helper' fixed it for us as well. Thanks!

Steven Ringo

You get the same warning if you have a spec file in /spec/lib, e.g. /spec/lib/foo_spec.rb and you don't want to require spec_helper as the class foo is not dependent on rails.

Interestingly, if you rename the lib directory to zlib the problem goes away. I have a feeling this is because of alphabetical load order.

David Chelimsky
Owner

@steveringo - definitely hadn't considered this scenario when we introduced this in rspec/rspec-core@fc70cee. This was there to solve a problem in ruby 1.9: when a module with a method that calls super is included twice in the same runtime object, you get an infinite loop. I think we'll need to find a different way to address that. /cc @myronmarston

Gary Bernhardt

Could that be solved by just detecting that RSpec.configure was called twice? (I'm not sure I fully understand yet... :)

Corey Haines

I think I haven't run into this because I keep two spec directories: spec and spec_no_rails
For my own sensibility and understanding, I like to keep them separate. So, this makes it easier for me to just say rspec spec or rspec spec_no_rails

Steven Ringo

Problem is worked around by creating a simple controller spec. Because controllers come before lib in the alphabet, it gets the require 'spec_helper' that eliminates the warnings. I exposed the issue only because I hadn't got to creating any controller tests yet - was working "past L" in the alphabet on models, requests and routing :-) This is very much an edge case...

Gary Bernhardt

It may be an edge case now, but relying on spec_helper-using specs coming first by coincidence seems a bit fragile to me. :) (I'm also on a personal crusade to get people to stop using spec_helper in most specs. ;)

David Chelimsky
Owner

We need to support n calls to RSpec.configure, as it happens in extensions.

We also need to fix this particular issue - you should be able to require or not require any spec_helper you want from wherever (as long as you don't load the same one twice).

I may be confusing issues. I thought this was related to rspec/rspec-expectations@80e5300, but I'm not certain and I'm having trouble finding any other potential motivation for rspec/rspec-core@fc70cee. Let's wait for @myronmarston to weigh in.

Myron Marston
Owner

There's a bit of a history behind this change...let me see if I can summarize it.

Ruby 1.9 has an obscure bug that triggers infinite recursion (and a SystemStackError) under the following circumstances:

  • A module (let's call it Mod) has an instance method (let's call it foo) that calls super.
  • Mod is included in both a class (class A; end) and one if its subclasses (class B < A; end), but is included in B after it was already included in A.
  • When you create an instance of B and call foo, you will get infinite recursion.

This gist demonstrates the issue. This is only a bug on 1.9; 1.8 does not have this issue. In addition, it is only triggered if the module is included in the superclass after it has already been included in the subclass. I'm guessing that ruby 1.9 uses a linked list to keep track of an object's class hierarchy, and this situation creates a circularly linked list.

In RSpec, we received multiple reports about users getting SystemStackErrors. This issue manifested itself in RSpec due to the following:

  • RSpec::Matchers#method_missing uses super (source), as it should.
  • Prior to RSpec 2.6, rspec-core included RSpec::Matchers in RSpec::Core::ExampleGroup very late. It included it after all examples had been defined, right before running them. This was done so that users could configure RSpec to use either the stdlib assertions or rspec-expectations.
  • Some users had manually included RSpec::Matchers in their example groups--here's on example. All example groups are subclasses of RSpec::Core::ExampleGroup, so this triggered the ruby 1.9 bug when the user had a spec that called an undefined method not handled by RSpec::Matchers#method_missing. It would call super to allow other classes up the class hierarchy to handle it, but instead the user got a confusing SystemStackError.

David and I (and some others on the rspec core team) discussed a few potential solutions for this issue, and decided that the best fix was to change when RSpec::Matchers gets included. To prevent this error, I changed rspec-core so that RSpec::Matchers gets included before any example groups get created. This fixed the infinite recursion issue, but created a new potential issue with configuring RSpec: since we were including the expectation/assertion module before the first example group is defined, we would now be ignoring end-user expect_with configuration if it came after an example group had been defined.

Correct me if I'm wrong, David, but I believe that RSpec configuration was always intended to be done before examples are defined--it just happened to always work fine before now. Looking to the future, this is a very hard feature to support, because the RSpec.configure block sets up how you want RSpec to work, and, naturally, this should happen before you start using RSpec to define your examples. I added the deprecation warning in this commit.

You can still call RSpec.configure as many times as you want. That's not going away. They should just all be before the first example group is defined.

For those of you who are dealing with this warning--is there a way to reorganize your RSpec config files so that they always get loaded first regardless of which specs (or subset of your specs) you are running?

I'm certainly open to suggestions of better ways to balance these issues.

Gary Bernhardt

It sounds like you could just tell people "don't manually include RSpec::Matchers in an example group; it's done for you", even emitting a warning, but I imagine there's more to it than that. :)

We could reorganize our spec directories to work around this by having a file that sorts before all of the others and loads spec_helper. But that's a pretty terrible hack. I think that load order independence should be considered part of RSpec's contract with the user. Calling RSpec.configure is optional, so some spec files will do it and some won't. And the load order is (or, IMO, should be) irrelevant to the user, so RSpec really can't count on configure being called before or after group definition.

David Chelimsky
Owner

It sounds like you could just tell people "don't manually include RSpec::Matchers in an example group; it's done for you", even emitting a warning, but I imagine there's more to it than that. :)

I think telling people stuff is not a reliable way to go. There needs to be something that tells you what's up at runtime, other than "stack level too deep".

We could reorganize our spec directories to work around this ...

I also think constraining directory/file structure like this is an unreasonable burden to put on users.

Here are a couple of alternatives:

  1. RSpec::Matchers could ask the class or module into which it is being included if it is already in the ancestor stack and not redefine things if it's already there.
  2. We narrow this down to the configuration options that might cause trouble if declared post-group-declaration? If the configuration object receives expect_with, for example, after any groups are defined, it raises an error. For the common case, this would never reveal itself. For your case, @garybernhardt (and all your crusaders), if you want to configure expect_with and avoid loading spec/spec_helper.rb in some cases, you'd have to configure expect_with at a higher level. e.g.
# in spec/support/global_configuration.rb
RSpec.configure do |c|
  c.expect_with :stdlib
end

# in spec/spec_helper.rb
require 'support/global_configuration'

# in spec/spec_helper_no_rails.rb
require 'support/global_configuration'

# in a spec that needs rails
require 'spec_helper'

# in a spec that does not need rails
require 'spec_helper_no_rails'

Personally, I like the first alternative :)

Thoughts?

Myron Marston
Owner

RSpec::Matchers could ask the class or module into which it is being included if it is already in the ancestor stack and not redefine things if it's already there.

I actually tried various flavors of fixes to RSpec::Matchers to work around this, but they were hacks, and in the end didn't work like I thought they would. I don't think the specific fix you suggest would work, anyway--if RSpec::Matchers is included in a class after it has been included in one its ancestors, it's not an issue. It's the opposite case that's the problem: when RSpec::Matchers is included in a class after it has been included in one of its subclasses. If ruby provided a Class#subclasses method, it would be possible to detect this at the point that RSpec::Matchers is included...but there is no such method. Individual classes can define inherited and keep track of this, but we can't count on it.

Instead, how does this sound?

  • Remove the deprecation warning for calling RSpec.configure after an example group is defined.
  • Instead, add some code to RSpec::Core::Configuration#expect_with that prints a warning if it is called after an example group has been defined, stating that it is being ignored because RSpec must use that setting very early. Likewise, for any future settings that we determine need to be acted on early, we can put similar warnings.
Myron Marston
Owner

I just realized that I mistated something above:

Mod is included in both a class (class A; end) and one if its subclasses (class B < A; end), but is included in B after it was already included in A.

This should read: "but is included in A after it was already included in B".

David Chelimsky
Owner

Instead, add some code to RSpec::Core::Configuration#expect_with that prints a warning if it is called after an example group has been defined, stating that it is being ignored because RSpec must use that setting very early. Likewise, for any future settings that we determine need to be acted on early, we can put similar warnings.

That's what I was proposing (except as an error, not a warning) as option 2.

I'm good with this option.

Myron Marston
Owner

That's what I was proposing (except as an error, not a warning) as option 2.

I think I misread what you wrote this morning (I had just gotten up...), but yeah, I can see that now :).

Would you prefer an error or a warning?

David Chelimsky
Owner

I think it should be an error. If it's just a warning, then the specs will still run and results might be confusing.

Myron Marston
Owner

Sounds good. I'm really swamped at work right now (the project I've been working on for 4 months is going into production in the next week), so I may not get around to this for a while. Someone else can feel free, or I'll try to get to it when things have settled down at work a bit (and feel free to ping me to remind me if I haven't addressed this in a couple weeks).

Myron Marston
Owner

I just pushed a fix for this to RSpec core.

Myron Marston myronmarston closed this
John Trupiano

I'm having trouble working around this warning. I've got a spec suite that runs cleanly without emitting the warning if run with "rspec spec". But the warning gets emitted when invoked via "rake spec". I think it's clear that Rake is somehow loading the spec_helper twice, but I can't figure out why/how. This behavior can be observed in the twilio-rb gem ( https://github.com/jtrupiano/twilio-rb/tree/rspec ).

When I drop a debugger into spec_helper there I get these two stacktraces before the suite runs:

    from /Users/jtrupiano/projects/twilio-rb/spec/spec_helper.rb:6:in `<top (required)>'
    from /Users/jtrupiano/projects/twilio-rb/spec/account_spec.rb:1:in `require'
    from /Users/jtrupiano/projects/twilio-rb/spec/account_spec.rb:1:in `<top (required)>'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/configuration.rb:419:in `load'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/configuration.rb:419:in `block in load_spec_files'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/configuration.rb:419:in `map'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/configuration.rb:419:in `load_spec_files'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:18:in `run'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:80:in `run_in_process'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:69:in `run'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:11:in `block in autorun'ruby-1.9.2-p180 :002

and

    from /Users/jtrupiano/projects/twilio-rb/spec/spec_helper.rb:6:in `<top (required)>'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/configuration.rb:419:in `load'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/configuration.rb:419:in `block in load_spec_files'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/configuration.rb:419:in `map'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/configuration.rb:419:in `load_spec_files'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:18:in `run'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:80:in `run_in_process'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:69:in `run'
    from /Users/jtrupiano/.rvm/gems/ruby-1.9.2-p180@twilio-rb/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:11:in `block in autorun'ruby-1.9.2-p180

It looks like the second load of spec_helper is happening within rspec-core itself.

Paul Annesley pda referenced this issue from a commit
Paul Annesley Removed unnecessary "config.mock_with :rspec" from spec_helper.rb
This statement configures RSpec to do what it would do anyway, and
triggers errors regarding RSpec.configure being called after examples
have been defined.

See: rspec/rspec-rails#371
See: rspec/rspec-core#455
a5030df
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.