Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Remove extra mocks teardowns/sandboxing #1242

Merged
merged 3 commits into from

2 participants

Myron Marston Jon Rowe
Myron Marston
Owner

This goes along with rspec/rspec-mocks#519.

Myron Marston myronmarston Don't verify/teardown mocks extra times.
The new rspec-mocks lifecycle logic supports
nested mock spaces, based on calling setup
or teardown extra times. It's important
they are only called once per example.
6661803
Jon Rowe
Owner

I assume the changes to Gemfile and script/test_all are just until rspec/rspec-mocks#519 is merged? I like the fact we're removing a big chunk of sandbox cruft here :)

Myron Marston
Owner

yep, see the commit message on that commit :).

Jon Rowe
Owner

Ahh, hidden behind a ... ;)

myronmarston added some commits
Myron Marston myronmarston Use rspec-mocks' new temporary scope support for the sandboxing.
Historically, this sandboxing logic has been brittle. Using
built-in rspec-mocks public APIs will be less brittle.
1500df7
Myron Marston myronmarston Add calls to `super`.
We don't want to modify the mock lifecycle behavior here,
we just want to count the number of calls.
8039cd2
Myron Marston
Owner

The build is failing with an rspec-expectations failure on ree that I can't repro. I'm going to merge anyway since the rspec-core master build will fail on all rubies until we merge this due to the merge of the rspec-mocks PR.

Myron Marston myronmarston merged commit d4c9fd8 into from
Myron Marston myronmarston deleted the branch
Jon Rowe
Owner

@myronmarston I can reproduce that failure locally, the seed is important, but it shouldnt be a blocker.

Myron Marston
Owner

@myronmarston I can reproduce that failure locally, the seed is important, but it shouldnt be a blocker.

Cool. Can you open an issue for rspec-expectations with repro steps/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 7, 2014
  1. Myron Marston

    Don't verify/teardown mocks extra times.

    myronmarston authored
    The new rspec-mocks lifecycle logic supports
    nested mock spaces, based on calling setup
    or teardown extra times. It's important
    they are only called once per example.
Commits on Jan 9, 2014
  1. Myron Marston

    Use rspec-mocks' new temporary scope support for the sandboxing.

    myronmarston authored
    Historically, this sandboxing logic has been brittle. Using
    built-in rspec-mocks public APIs will be less brittle.
  2. Myron Marston

    Add calls to `super`.

    myronmarston authored
    We don't want to modify the mock lifecycle behavior here,
    we just want to count the number of calls.
This page is out of date. Refresh to see the latest.
9 lib/rspec/core/example.rb
View
@@ -293,7 +293,14 @@ def run_after_each
def verify_mocks
@example_group_instance.verify_mocks_for_rspec
rescue Exception => e
- set_exception(e, :dont_print)
+ if metadata[:execution_result][:pending_fixed]
+ metadata[:execution_result][:pending_fixed] = false
+ metadata[:pending] = true
+ @pending_declared_in_example = metadata[:execution_result][:pending_message]
+ @exception = nil
+ else
+ set_exception(e, :dont_print)
+ end
end
def assign_generated_description
12 lib/rspec/core/pending.rb
View
@@ -88,17 +88,15 @@ def pending(*args)
current_example.execution_result[:pending_fixed] = false
if block_given?
begin
- result = begin
- yield
- current_example.example_group_instance.instance_eval { verify_mocks_for_rspec }
- end
+ no_failure = false
+ yield
+ no_failure = true
current_example.metadata[:pending] = false
rescue Exception => e
current_example.execution_result[:exception] = e
- ensure
- teardown_mocks_for_rspec
end
- if result
+
+ if no_failure
current_example.execution_result[:pending_fixed] = true
raise PendingExampleFixedError.new
end
22 spec/rspec/core/pending_example_spec.rb
View
@@ -172,8 +172,28 @@ def run_example(*pending_args)
end
it "passes" do
- expect(run_example("just because")).to be_pending
+ expect(run_example.exception).to be_nil
end
+
+ it 'indicates it is pending with the given message' do
+ expect(run_example("just because")).to be_pending_with("just because")
+ end
+
+ it 'indicates the pending block was not fixed' do
+ expect(run_example.metadata[:execution_result][:pending_fixed]).to be false
+ end
+ end
+
+ it 'does not verify or teardown mocks multiple times' do
+ counts = Hash.new(0)
+
+ RSpec::Core::ExampleGroup.describe('group') do
+ define_method(:verify_mocks_for_rspec) { counts[:verify] += 1; super() }
+ define_method(:teardown_mocks_for_rspec) { counts[:teardown] += 1; super() }
+ example { pending { } }
+ end.run
+
+ expect(counts).to eq(:verify => 1, :teardown => 1)
end
context "that passes" do
2  spec/spec_helper.rb
View
@@ -73,7 +73,7 @@ def run(reporter=nil)
end
end
- RSpec::Core::SandboxedMockSpace.sandboxed do
+ RSpec::Mocks.with_temporary_scope do
object.instance_eval(&block)
end
ensure
106 spec/support/sandboxed_mock_space.rb
View
@@ -1,106 +0,0 @@
-require 'rspec/mocks'
-
-module RSpec
- module Core
- # Because rspec-core dog-foods itself, rspec-core's spec suite has
- # examples that define example groups and examples and run them. The
- # usual lifetime of an RSpec::Mocks::Proxy is for one example
- # (the proxy cache gets cleared between each example), but since the
- # specs in rspec-core's suite sometimes create test doubles and pass
- # them to examples a spec defines and runs, the test double's proxy
- # must live beyond the inner example: it must live for the scope
- # of wherever it got defined. Here we implement the necessary semantics
- # for rspec-core's specs:
- #
- # - #verify_all and #reset_all affect only mocks that were created
- # within the current scope.
- # - Mock proxies live for the duration of the scope in which they are
- # created.
- #
- # Thus, mock proxies created in an inner example live for only that
- # example, but mock proxies created in an outer example can be used
- # in an inner example but will only be reset/verified when the outer
- # example completes.
- class SandboxedMockSpace < ::RSpec::Mocks::Space
- def self.sandboxed
- sandboxed_space = RSpec::Core::SandboxedMockSpace.new
- RSpec::Mocks.send(:remove_const, "ERROR_SPACE")
- RSpec::Mocks.const_set("ERROR_SPACE", sandboxed_space)
- RSpec::Mocks.send(:remove_const, "MOCK_SPACE")
- RSpec::Mocks.const_set("MOCK_SPACE", sandboxed_space)
-
- RSpec::Core::Example.class_eval do
- alias_method :orig_run, :run
- def run(*args)
- RSpec::Mocks.space.sandboxed do
- orig_run(*args)
- end
- end
- end
-
- yield
- ensure
- RSpec::Core::Example.class_eval do
- remove_method :run
- alias_method :run, :orig_run
- remove_method :orig_run
- end
-
- RSpec::Mocks.send(:remove_const, "MOCK_SPACE")
- RSpec::Mocks.const_set("MOCK_SPACE", RSpec::Mocks::Space.new)
- RSpec::Mocks.send(:remove_const, "ERROR_SPACE")
- RSpec::Mocks.const_set("ERROR_SPACE", RSpec::Mocks::ErrorSpace.new)
- end
-
- class Sandbox
- attr_reader :proxies
-
- def initialize
- @proxies = Set.new
- end
-
- def verify_all
- @proxies.each { |p| p.verify }
- end
-
- def reset_all
- @proxies.each { |p| p.reset }
- end
- end
-
- def initialize
- @sandbox_stack = []
- super
- end
-
- def sandboxed
- @sandbox_stack << Sandbox.new
- yield
- ensure
- @sandbox_stack.pop
- end
-
- def verify_all
- return super unless sandbox = @sandbox_stack.last
- sandbox.verify_all
- end
-
- def reset_all
- return super unless sandbox = @sandbox_stack.last
- sandbox.reset_all
- end
-
- def proxy_for(object)
- new_proxy = !proxies.has_key?(object.__id__)
- proxy = super
-
- if new_proxy && sandbox = @sandbox_stack.last
- sandbox.proxies << proxy
- end
-
- proxy
- end
- end
- end
-end
-
Something went wrong with that request. Please try again.