Don't error out when config.mock_with or expect_with is re-specifying the current config #490

Closed
myronmarston opened this Issue Oct 31, 2011 · 10 comments

Comments

Projects
None yet
4 participants
@myronmarston
Member

myronmarston commented Oct 31, 2011

See this comment. config.mock_with :rspec was raising an error for a user due to the 2.7 change discussed here. In this case, raising the error was problematic for the user and unnecessary since it :rspec is the default and it was essentially a no-op.

We should change the code that raises an error so that it does nothing if these config options are simply re-specifying the current configuration (whether the default or not), since they are essentially a no-op in this case.

@nirvdrum

This comment has been minimized.

Show comment
Hide comment
@nirvdrum

nirvdrum Oct 31, 2011

Looking forward to this fix. The other annoying thing is it went from a warning message about deprecated behavior being removed in RSpec 3.0 to something that just flat out errors in RSpec 2.7, breaking both with the contract of RSpec minor releases and the deprecation notice.

Looking forward to this fix. The other annoying thing is it went from a warning message about deprecated behavior being removed in RSpec 3.0 to something that just flat out errors in RSpec 2.7, breaking both with the contract of RSpec minor releases and the deprecation notice.

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Oct 31, 2011

Member

The other annoying thing is it went from a warning message about deprecated behavior being removed in RSpec 3.0 to something that just flat out errors in RSpec 2.7, breaking both with the contract of RSpec minor releases and the deprecation notice.

The plan was to follow the deprecation/removal path until we got complaints about it. rspec/rspec-rails#371 contains a good portion of the discussion that went into changing the warning for all config-after-example-group-definition to an error for specific configs-after-example-group-definition. My comment there goes into detail about the specific ruby 1.9 bug we have been trying to work around with this.

For now, just remove config.mock_with :rspec as it doesn't actually do anything (that's the default that will get setup anyway).

Got any ideas for how we could have dealt with the ruby 1.9 bug better?

Member

myronmarston commented Oct 31, 2011

The other annoying thing is it went from a warning message about deprecated behavior being removed in RSpec 3.0 to something that just flat out errors in RSpec 2.7, breaking both with the contract of RSpec minor releases and the deprecation notice.

The plan was to follow the deprecation/removal path until we got complaints about it. rspec/rspec-rails#371 contains a good portion of the discussion that went into changing the warning for all config-after-example-group-definition to an error for specific configs-after-example-group-definition. My comment there goes into detail about the specific ruby 1.9 bug we have been trying to work around with this.

For now, just remove config.mock_with :rspec as it doesn't actually do anything (that's the default that will get setup anyway).

Got any ideas for how we could have dealt with the ruby 1.9 bug better?

@nirvdrum

This comment has been minimized.

Show comment
Hide comment
@nirvdrum

nirvdrum Oct 31, 2011

I actually mock with mocha, so removing the line isn't much of an option. I went through and changed all my require statements to be the relative require 'spec_helper', per the other thread, and that fixed it. The fix was very non-intuitive, as was the original error. I caught the error when upgrading from RSpec 1.3 => 2.6, so I didn't realize this was a recent change.

So, I can now run rake with RSpec 2.7 fine, but I've lost the ability to run any spec in isolation. I'm not wild about that prospect either.

I'm still running REE because 1.9.2 has been an unstable, buggy release for me. So unfortunately I can't weigh in more on that matter. Sounds like being stuck between a rock and a hard place. I guess having a wiki page or a better error message would have helped. For the life of me I couldn't figure out how I already had example groups defined. I didn't realize the configure stuff was non-reentrant. But if it can recall the example group state between calls, it should probably be able to remember its configuration between calls too, as you suggest above.

I actually mock with mocha, so removing the line isn't much of an option. I went through and changed all my require statements to be the relative require 'spec_helper', per the other thread, and that fixed it. The fix was very non-intuitive, as was the original error. I caught the error when upgrading from RSpec 1.3 => 2.6, so I didn't realize this was a recent change.

So, I can now run rake with RSpec 2.7 fine, but I've lost the ability to run any spec in isolation. I'm not wild about that prospect either.

I'm still running REE because 1.9.2 has been an unstable, buggy release for me. So unfortunately I can't weigh in more on that matter. Sounds like being stuck between a rock and a hard place. I guess having a wiki page or a better error message would have helped. For the life of me I couldn't figure out how I already had example groups defined. I didn't realize the configure stuff was non-reentrant. But if it can recall the example group state between calls, it should probably be able to remember its configuration between calls too, as you suggest above.

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Oct 31, 2011

Member

The fix was very non-intuitive, as was the original error.

If you have any ideas for making it more intuitive, please feel free to update the wording of the error and submit a pull request :).

So, I can now run rake with RSpec 2.7 fine, but I've lost the ability to run any spec in isolation. I'm not wild about that prospect either.

I think you just need to re-order have you require and configure things. The important thing is that to prevent mysterious, confusing SystemStackError exceptions, RSpec must include the mock and expectation modules in RSpec::Core::ExampleGroup before users have defined any example groups...which means the configuration of those options must take place before an example group is defined. I would LOVE to not have this requirement, but this bug on ruby 1.9 makes it necessary. Here's on way you could re-arrange things to deal with this error:

spec/fast_spec_helper.rb:

require 'rspec'

RSpec.configure do |c|
  c.mock_with :mocha
end

spec/spec_helper.rb:

require 'fast_spec_helper'

# load rails or whatever to get your full app environment booted

RSpec.configure do |c|
  # other spec configuration
end

in a fast isolated spec:

require 'fast_spec_helper'

describe MyClass do
end

in a non-isolated spec:

require 'spec_helper'

describe MyRailsController do
end

The idea here is that we need to configure mock_with before any example groups are defined. spec_helper, in and of itself, doesn't make a spec slow and non-isolated; it's the fact that people's typical spec_helper loads tons of stuff and boots the whole rails environment. So, you can define another spec helper (fast_spec_helper) that gives you the minimum you need that MUST be configured before any example groups are defined, and require that both from isolated specs and also from spec_helper itself.

Let me know if that doesn't work for you.

And thanks for your patience on this manner! I know the changes here have caused pain for some users, and I really wish there was a better way...

Member

myronmarston commented Oct 31, 2011

The fix was very non-intuitive, as was the original error.

If you have any ideas for making it more intuitive, please feel free to update the wording of the error and submit a pull request :).

So, I can now run rake with RSpec 2.7 fine, but I've lost the ability to run any spec in isolation. I'm not wild about that prospect either.

I think you just need to re-order have you require and configure things. The important thing is that to prevent mysterious, confusing SystemStackError exceptions, RSpec must include the mock and expectation modules in RSpec::Core::ExampleGroup before users have defined any example groups...which means the configuration of those options must take place before an example group is defined. I would LOVE to not have this requirement, but this bug on ruby 1.9 makes it necessary. Here's on way you could re-arrange things to deal with this error:

spec/fast_spec_helper.rb:

require 'rspec'

RSpec.configure do |c|
  c.mock_with :mocha
end

spec/spec_helper.rb:

require 'fast_spec_helper'

# load rails or whatever to get your full app environment booted

RSpec.configure do |c|
  # other spec configuration
end

in a fast isolated spec:

require 'fast_spec_helper'

describe MyClass do
end

in a non-isolated spec:

require 'spec_helper'

describe MyRailsController do
end

The idea here is that we need to configure mock_with before any example groups are defined. spec_helper, in and of itself, doesn't make a spec slow and non-isolated; it's the fact that people's typical spec_helper loads tons of stuff and boots the whole rails environment. So, you can define another spec helper (fast_spec_helper) that gives you the minimum you need that MUST be configured before any example groups are defined, and require that both from isolated specs and also from spec_helper itself.

Let me know if that doesn't work for you.

And thanks for your patience on this manner! I know the changes here have caused pain for some users, and I really wish there was a better way...

@dchelimsky

This comment has been minimized.

Show comment
Hide comment
@dchelimsky

dchelimsky Nov 3, 2011

Member

Closed by #494

Member

dchelimsky commented Nov 3, 2011

Closed by #494

@dchelimsky dchelimsky closed this Nov 3, 2011

dchelimsky added a commit that referenced this issue Nov 3, 2011

@kevinmccaughey

This comment has been minimized.

Show comment
Hide comment
@kevinmccaughey

kevinmccaughey Jul 27, 2012

This appears to be broken again, I have had to remove "config.mock_with :rspec" to get it to work.

This appears to be broken again, I have had to remove "config.mock_with :rspec" to get it to work.

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Jul 27, 2012

Member

@kevinmccaughey -- Sorry to hear that. Can you post a reproducible example? I'll take a look at it.

BTW, I posted a blog post describing the background to this issue:

http://myronmars.to/n/dev-blog/2011/11/recent-rspec-configuration-warnings-and-errors

...so if you want to understand more about what's going on, check that out.

Member

myronmarston commented Jul 27, 2012

@kevinmccaughey -- Sorry to hear that. Can you post a reproducible example? I'll take a look at it.

BTW, I posted a blog post describing the background to this issue:

http://myronmars.to/n/dev-blog/2011/11/recent-rspec-configuration-warnings-and-errors

...so if you want to understand more about what's going on, check that out.

@kevinmccaughey

This comment has been minimized.

Show comment
Hide comment
@kevinmccaughey

kevinmccaughey Jul 30, 2012

Hi Myron,

I am only a beginner so I was maybe confused about the functionality - I
am using the Rspec book from Pragprog and Michael Hardls' Rails
tutorial, so I don't understand enough to comment beyond saying that the
functionality as I see it (and I am trying to follow why it was changed)
seemed confusing and it was only by lots of googling that I knew to drop
the line for the mock.

Thanks for taking the time to check on this - I will keep learning and
maybe be more qualified to comment in a year or two's time ;)

All the best,

Kevin McCaughey

On 27/07/2012 19:22, Myron Marston wrote:

@kevinmccaughey -- Sorry to hear that. Can you post a reproducible example? I'll take a look at it.

BTW, I posted a blog post describing the background to this issue:

http://myronmars.to/n/dev-blog/2011/11/recent-rspec-configuration-warnings-and-errors


Reply to this email directly or view it on GitHub:
#490 (comment)

Hi Myron,

I am only a beginner so I was maybe confused about the functionality - I
am using the Rspec book from Pragprog and Michael Hardls' Rails
tutorial, so I don't understand enough to comment beyond saying that the
functionality as I see it (and I am trying to follow why it was changed)
seemed confusing and it was only by lots of googling that I knew to drop
the line for the mock.

Thanks for taking the time to check on this - I will keep learning and
maybe be more qualified to comment in a year or two's time ;)

All the best,

Kevin McCaughey

On 27/07/2012 19:22, Myron Marston wrote:

@kevinmccaughey -- Sorry to hear that. Can you post a reproducible example? I'll take a look at it.

BTW, I posted a blog post describing the background to this issue:

http://myronmars.to/n/dev-blog/2011/11/recent-rspec-configuration-warnings-and-errors


Reply to this email directly or view it on GitHub:
#490 (comment)

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Jul 30, 2012

Member

@kevinmccaughey -- can you post a link to the example in the rails tutorial?

Member

myronmarston commented Jul 30, 2012

@kevinmccaughey -- can you post a link to the example in the rails tutorial?

@kevinmccaughey

This comment has been minimized.

Show comment
Hide comment
@kevinmccaughey

kevinmccaughey Jul 30, 2012

I will link to the listing in the (free) online book (current version). The line is 2/3 of the way down: config.mock_with :rspec

http://ruby.railstutorial.org/chapters/static-pages#code:spork_spec_helper

Now this seemed sensible to me, so I was really confused when the only way to get the listing to work was to remove that line (config.mock_with :rspec)

I will link to the listing in the (free) online book (current version). The line is 2/3 of the way down: config.mock_with :rspec

http://ruby.railstutorial.org/chapters/static-pages#code:spork_spec_helper

Now this seemed sensible to me, so I was really confused when the only way to get the listing to work was to remove that line (config.mock_with :rspec)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment