Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Require users to invoke `RSpec.reset` #703

Closed
wants to merge 2 commits into from

5 participants

@samphippen
Collaborator

This is related to #621.

In the issue report it seems to have been decided that the best course of action is to require users to invoke RSpec.reset instead of it being called automatically. This pull request does that.

@samphippen samphippen Remove the automatic call to RSpec.reset
This also updates the specs that are effected by this call. The order
specs were effected because the RSpec runner was being invoked within
the same ruby process (I think). The run_command helper in the
order_spec file has been updated to explicitly invoke the reset method.

Signed-off-by: Sam Phippen <samphippen@googlemail.com>
d6d0d35
@samphippen
Collaborator

@dchelimsky you were pretty involved with 621. Can you take a look at this please? Cheers :)

@pierreozoux pierreozoux referenced this pull request in sandro/specjour
Closed

Rspec-core current dev > 2.11.1 #44

@wallace

@myronmarston @alindeman , I know @dchelimsky is lessening his work in rspec. Can someone else review and merge, please? This will close #621.

@dchelimsky
Owner

FWIW I think this is a potentially breaking change that would need to be released as part of 3.0. It also should be documented very clearly, which it is not yet.

@wallace

@dchelimsky, good point.

@samphippen are you interested in updating this PR with documentation? If not, I can take a stab at.

@JonRowe
Owner

@myronmarston should this not be tagged for rspec 3?

@samphippen
Collaborator

I somehow missed these emails. I'll take a look at this in the near future.

@JonRowe
Owner

This is also a dependency for #621 which is similarly awaiting RSpec 3

@samphippen samphippen Add some docs for the new reset behaviour
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
4fe3704
@samphippen
Collaborator

I added some docs.

@mskraddoc

I am seeing a 'stack level too deep' error immediately following a call to rspec.reset as detailed in a lengthy SO question here: http://stackoverflow.com/questions/16512114/stack-level-too-deep-systemstackerror-when-using-both-rspec-and-cucumber-with

Could this be related?

@JonRowe
Owner

I'm afraid I don't believe these are related, StackLevelTooDeep errors are caused by your Ruby interpreter running out of stack, normally they are caused by infinite recursion but sometimes they can be caused by long running loops, looking at your SO question I'm hazarding a guess that the rspec call is incidental, especially as it the ActiveSupport dependencies code where the exception is raised. If you can piece together a reproducible example as a git repo I might be able to take a look at it for you, but so far on SO you've not posted any application code which is most likely where the culprit lies.

@samphippen
Collaborator

If I rebase this, now that we're working towards 3.0 is this something that we want merged. @myronmarston @JonRowe @soulcutter @alindeman @dchelimsky

@JonRowe
Owner

Pass, have we figured out if it's definitely needed? What about docs? How would this work in practise?

@samphippen
Collaborator

@dchelimsky @myronmarston I don't know what's going on with this, but don't want to keep this open for too long, what should we do about it?

@dchelimsky
Owner

I'm concerned by the implications of #621 (comment). @sandro's issue was that clearing the config between runs blew away config that wasn't going to get reloaded. Right now we have no concept of config settings that should be persistent vs those that should be clearable. I'm not even sure we can do that in a universally correct way. I'll ponder this a bit and follow up if anything hits me, but I don't recommend merging this yet as it solves one problem by creating another IMO.

@samphippen
Collaborator

I think I'm going to close this unless we think of a better solution, based on what @dchelimsky says.

@samphippen samphippen closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 7, 2012
  1. @samphippen

    Remove the automatic call to RSpec.reset

    samphippen authored
    This also updates the specs that are effected by this call. The order
    specs were effected because the RSpec runner was being invoked within
    the same ruby process (I think). The run_command helper in the
    order_spec file has been updated to explicitly invoke the reset method.
    
    Signed-off-by: Sam Phippen <samphippen@googlemail.com>
Commits on Mar 30, 2013
  1. @samphippen

    Add some docs for the new reset behaviour

    samphippen authored
    Signed-off-by: Sam Phippen <samphippen@googlemail.com>
This page is out of date. Refresh to see the latest.
View
6 lib/rspec/core.rb
@@ -61,9 +61,11 @@ def self.world
@world ||= RSpec::Core::World.new
end
- # @private
- # Used internally to ensure examples get reloaded between multiple runs in
+ # Used to ensure examples get reloaded between multiple runs in
# the same process.
+ #
+ # Users must invoke this if they want to have the configuration reset when
+ # they use runner multiple times within the same process.
def self.reset
@world = nil
@configuration = nil
View
2  lib/rspec/core/runner.rb
@@ -68,8 +68,6 @@ def self.run(args, err=$stderr, out=$stdout)
else
CommandLine.new(options).run(err, out)
end
- ensure
- RSpec.reset
end
end
end
View
1  spec/command_line/order_spec.rb
@@ -194,6 +194,7 @@ def split_in_half(array)
def run_command(cmd)
RSpec::Core::Runner.run(cmd.split, stderr, stdout)
+ RSpec.reset
end
def run_command_expecting_error(cmd)
View
6 spec/rspec/core/runner_spec.rb
@@ -26,12 +26,6 @@ module RSpec::Core
let(:err) { StringIO.new }
let(:out) { StringIO.new }
- it "tells RSpec to reset" do
- RSpec.configuration.stub(:files_to_run => [])
- RSpec.should_receive(:reset)
- RSpec::Core::Runner.run([], err, out)
- end
-
context "with --drb or -X" do
before(:each) do
Something went wrong with that request. Please try again.