Skip to content

yield current running example to it/example and before/after hooks #666

Merged
merged 7 commits into from Jun 26, 2013
View
11 lib/rspec/core/example.rb
@@ -111,7 +111,7 @@ def run(example_group_instance, reporter)
with_around_each_hooks do
begin
run_before_each
- @example_group_instance.instance_eval(&@example_block)
+ @example_group_instance.instance_exec(self, &@example_block)
rescue Pending::PendingDeclaredInExample => e
@pending_declared_in_example = e.message
rescue Exception => e
@@ -233,13 +233,8 @@ def fail_with_exception(reporter, exception)
end
# @private
- def instance_eval(*args, &block)
- @example_group_instance.instance_eval(*args, &block)
- end
-
- # @private
- def instance_eval_with_rescue(context = nil, &block)
- @example_group_instance.instance_eval_with_rescue(context, &block)
+ def instance_exec_with_rescue(context = nil, &block)
+ @example_group_instance.instance_exec_with_rescue(self, context, &block)
end
# @private
View
55 lib/rspec/core/example_group.rb
@@ -55,6 +55,7 @@ def description
# @param [String] name
# @param [Hash] extra_options
# @param [Block] implementation
+ # @yield [Example] the example object
def self.define_example_method(name, extra_options={})
define_method(name) do |*all_args, &block|
desc, *args = *all_args
@@ -67,10 +68,22 @@ def self.define_example_method(name, extra_options={})
end
# Defines an example within a group.
+ # @example
+ # example do
+ # end
+ #
+ # example "does something" do
+ # end
+ #
+ # example "does something", :with => 'additional metadata' do
+ # end
+ #
+ # example "does something" do |ex|
+ # # ex is the Example object that evals this block
+ # end
define_example_method :example
# Defines an example within a group.
- #
- # @see example
+ # @example
define_example_method :it
# Defines an example within a group.
# This is here primarily for backward compatibility with early versions
@@ -79,17 +92,23 @@ def self.define_example_method(name, extra_options={})
define_example_method :specify
# Shortcut to define an example with `:focus` => true
+ # @see example
define_example_method :focus, :focused => true, :focus => true
# Shortcut to define an example with `:focus` => true
+ # @see example
define_example_method :focused, :focused => true, :focus => true
# Shortcut to define an example with :pending => true
+ # @see example
define_example_method :pending, :pending => true
# Shortcut to define an example with :pending => 'Temporarily disabled with xexample'
+ # @see example
define_example_method :xexample, :pending => 'Temporarily disabled with xexample'
# Shortcut to define an example with :pending => 'Temporarily disabled with xit'
+ # @see example
define_example_method :xit, :pending => 'Temporarily disabled with xit'
# Shortcut to define an example with :pending => 'Temporarily disabled with xspecify'
+ # @see example
define_example_method :xspecify, :pending => 'Temporarily disabled with xspecify'
# Works like `alias_method :name, :example` with the added benefit of
@@ -439,16 +458,24 @@ def self.set_ivars(instance, ivars)
ivars.each {|name, value| instance.instance_variable_set(name, value)}
end
- # @attr_reader
- # Returns the {Example} object that wraps this instance of
- # `ExampleGroup`
- attr_accessor :example
+ def initialize
+ @_current_rspec_example = nil
+ end
+
+ def example=(current_example)
+ @_current_rspec_example = current_example
+ end
@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

Is this being called anywhere? I've scrolled down this diff 3 times looking for where it is being called but can't find it. I figure it must be called somewhere given that other things are relying on @_current_rspec_example...

@dchelimsky
RSpec member
dchelimsky added a note Jun 19, 2013

Example#run

@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

Thanks, I missed the fact that example= already existed via the attr_accessor declaration. I was thinking this was a new method and didn't see anything in the diff that was calling it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ # @deprecated use a block argument
+ def example
+ RSpec.deprecate("example", :replacement => "a block argument")
+ @_current_rspec_example
+ end
@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

I was originally thinking we'd remove this method in 3.0 and than add a deprecation like this to 2.99. I'm open to keeping it deprecated in 3.0, though, especially if you think it's important. Generally, for most breaking changes like this I'm in favor of deprecating in 2.99 and removing in 3.0, except in a few circumstances:

  • It could trip up newcomers who are using an old tutorial
  • The API is likely to be used by an extension gem, which the user may not have direct control over and can't easily update during the 2.x -> 2.99 -> 3.0 migration process.

Do you think either of these apply here?

@dchelimsky
RSpec member
dchelimsky added a note Jun 19, 2013

I agree that's the right path - just wasn't thinking clearly that master is 3.0. So let's merge this (when we do) as/is to both 2-14-maintenance and master and I'll add another commit to master to remove the deprecated methods. Fair?

@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

Sounds fair...although we need this in 2-99-maintenance, not 2-14-maintenance. Unless you want this to go out in 2.14? I think just being deprecated in 2.99 is sufficient, given that it's not very common to access the example.

@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

So if we're going to remove the #example and #running_example, is there a way we can remove @_current_rspec_example as well? Or do we still need the instance variable?

@dchelimsky
RSpec member
dchelimsky added a note Jun 19, 2013

We still need to access it internally - just not expose it via a method. The assumption here is that it won't conflict with a @_current_rspec_example instance var in your app's examples :)

@dchelimsky
RSpec member
dchelimsky added a note Jun 19, 2013

re: 2.14 v 2.99 v 3.0. We can definitely do 2.99 (w/ deprecations) and 3.0 (with removal), but it feels odd to add a new feature to 2.99 and only give people 2.99 on to use it. WDYT?

@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

We still need to access it internally - just not expose it via a method. The assumption here is that it won't conflict with a @_current_rspec_example instance var in your app's examples :)

Random idea: what if we stored the current example in a thread local? That would remove the need for the ivar. If/when we ever wanted to make a multi-threaded runner, that would be automatically thread safe in a way that instance variables aren't. We could then expose it from RSpec.current_example (which I think was one of your earlier ideas), which can be handy for when a helper method needs to access the example metadata, and we could remove the example= method in this class. I don't know how likely it is that users would have an example= helper method, but it's not outside the realm of possibility.

re: 2.14 v 2.99 v 3.0. We can definitely do 2.99 (w/ deprecations) and 3.0 (with removal), but it feels odd to add a new feature to 2.99 and only give people 2.99 on to use it. WDYT?

I don't consider this a new feature. I consider it an API change. The current example has always been available; this just changes the means users use to access it.

@dchelimsky
RSpec member
dchelimsky added a note Jun 19, 2013

What's your concern about an instance variable here? It's got a name that is very unlikely to cause a conflict. We can always move to a thread local when we want to make a multi-threaded runner, which will likely need other changes as well.

I agree re: API change v new feature. 2.99 and 3.0 it is.

@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

What's your concern about an instance variable here? It's got a name that is very unlikely to cause a conflict. We can always move to a thread local when we want to make a multi-threaded runner, which will likely need other changes as well.

I don't have a specific concern. It's just the anal part of me thinking about the fact that the goal here was to stop /reduce polluting the user's namespace, and we're still doing so -- just with something that's less likely to conflict.

@dchelimsky
RSpec member
dchelimsky added a note Jun 19, 2013

I think thread local is overkill for this, and if we expose RSpec.current_example it will get used. I'd recommend keeping the ivar for now. Can always adjust later if a holistic approach emerges.

@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

I'm fine with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- # @deprecated use {ExampleGroup#example}
+ # @deprecated use a block argument
def running_example
- RSpec.deprecate("running_example",
- :replacement => "example")
- example
+ RSpec.deprecate("running_example", :replacement => "a block argument")
+ @_current_rspec_example
end
@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

I noticed there's no spec (that I can find, anyway) for either #example or #running_example, to show that it continues to return the example object and that they print a deprecation warning.

Do you think that's worth adding?

@dchelimsky
RSpec member
dchelimsky added a note Jun 19, 2013

Added in ed5a3be

@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
# Returns the class or module passed to the `describe` method (or alias).
@@ -468,12 +495,12 @@ def described_class
# @private
# instance_evals the block, capturing and reporting an exception if
# raised
- def instance_eval_with_rescue(context = nil, &hook)
+ def instance_exec_with_rescue(example, context = nil, &hook)
begin
- instance_eval(&hook)
+ instance_exec(example, &hook)
rescue Exception => e
- raise unless example
- example.set_exception(e, context)
+ raise unless @_current_rspec_example
+ @_current_rspec_example.set_exception(e, context)
@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

Given that this method accepts an example argument, it would seem cleaner to use that rather than the instance variable. Any reason to prefer the instance variable over the local?

@dchelimsky
RSpec member
dchelimsky added a note Jun 19, 2013

I changed it and got an error. Looks like this is a hack to ensure different behavior for an after(:all) block. Looking into a cleaner, more expressive fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
end
end
end
View
4 lib/rspec/core/hooks.rb
@@ -18,7 +18,7 @@ def options_apply?(example_or_group)
class BeforeHook < Hook
def run(example)
- example.instance_eval(&block)
+ example.instance_exec(example, &block)
end
def display_name
@@ -28,7 +28,7 @@ def display_name
class AfterHook < Hook
def run(example)
- example.instance_eval_with_rescue("in an after hook", &block)
+ example.instance_exec_with_rescue("in an after hook", &block)
end
def display_name
View
6 lib/rspec/core/memoized_helpers.rb
@@ -195,8 +195,10 @@ def let(name, &block)
# Apply the memoization. The method has been defined in an ancestor
# module so we can use `super` here to get the value.
- define_method(name) do
- __memoized.fetch(name) { |k| __memoized[k] = super(&nil) }
+ if block.arity == 1
+ define_method(name) { __memoized.fetch(name) { |k| __memoized[k] = super(@_current_rspec_example, &nil) } }
+ else
+ define_method(name) { __memoized.fetch(name) { |k| __memoized[k] = super(&nil) } }
end
end
View
13 lib/rspec/core/pending.rb
@@ -76,7 +76,7 @@ def pending_fixed?; true; end
# # ...
# end
def pending(*args)
- return self.class.before(:each) { pending(*args) } unless example
+ return self.class.before(:each) { pending(*args) } unless @_current_rspec_example
@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

This has nothing to do with your PR since it was already this way...but under which circumstances would this be called w/o a current example? I can't think of a case where this early return would actually happen.

@dchelimsky
RSpec member
dchelimsky added a note Jun 19, 2013

It's for the before(:all) { pending() } case. Happy to consider eliminating support for that, but it works today.

@myronmarston
RSpec member
myronmarston added a note Jun 19, 2013

Gotcha. It's fine as is (a refactoring to that would really be a separate issue). Mostly I was just trying to understand what this case was here for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
options = args.last.is_a?(Hash) ? args.pop : {}
message = args.first || NO_REASON_GIVEN
@@ -85,18 +85,17 @@ def pending(*args)
return block_given? ? yield : nil
end
- example.metadata[:pending] = true
- example.metadata[:execution_result][:pending_message] = message
+ @_current_rspec_example.metadata[:pending] = true
+ @_current_rspec_example.metadata[:execution_result][:pending_message] = message
if block_given?
begin
result = begin
yield
- example.example_group_instance.instance_eval { verify_mocks_for_rspec }
- true
+ @_current_rspec_example.example_group_instance.instance_eval { verify_mocks_for_rspec }
end
- example.metadata[:pending] = false
+ @_current_rspec_example.metadata[:pending] = false
rescue Exception => e
- example.execution_result[:exception] = e
+ @_current_rspec_example.execution_result[:exception] = e
ensure
teardown_mocks_for_rspec
end
View
58 spec/rspec/core/example_group_spec.rb
@@ -65,8 +65,8 @@ def metadata_hash(*args)
examples_run = []
group = ExampleGroup.describe("parent") do
describe("child") do
- it "does something" do
- examples_run << example
+ it "does something" do |ex|
+ examples_run << ex
end
end
end
@@ -79,13 +79,13 @@ def metadata_hash(*args)
it "runs its children " do
examples_run = []
group = ExampleGroup.describe("parent") do
- it "fails" do
- examples_run << example
+ it "fails" do |ex|
+ examples_run << ex
raise "fail"
end
describe("child") do
- it "does something" do
- examples_run << example
+ it "does something" do |ex|
+ examples_run << ex
end
end
end
@@ -663,19 +663,10 @@ def define_and_run_group(define_outer_example = false)
end
end
- it "has no 'running example' within before(:all)" do
- group = ExampleGroup.describe
- running_example = :none
- group.before(:all) { running_example = example }
- group.example("no-op") { }
- group.run
- expect(running_example).to be(nil)
- end
-
it "has access to example options within before(:each)" do
group = ExampleGroup.describe
option = nil
- group.before(:each) { option = example.options[:data] }
+ group.before(:each) {|ex| option = ex.options[:data] }
group.example("no-op", :data => :sample) { }
group.run
expect(option).to eq(:sample)
@@ -684,20 +675,11 @@ def define_and_run_group(define_outer_example = false)
it "has access to example options within after(:each)" do
group = ExampleGroup.describe
option = nil
- group.after(:each) { option = example.options[:data] }
+ group.after(:each) {|ex| option = ex.options[:data] }
group.example("no-op", :data => :sample) { }
group.run
expect(option).to eq(:sample)
end
-
- it "has no 'running example' within after(:all)" do
- group = ExampleGroup.describe
- running_example = :none
- group.after(:all) { running_example = example }
- group.example("no-op") { }
- group.run
- expect(running_example).to be(nil)
- end
end
%w[pending xit xspecify xexample].each do |method_name|
@@ -755,20 +737,20 @@ def define_and_run_group(define_outer_example = false)
describe Object, "describing nested example_groups", :little_less_nested => 'yep' do
describe "A sample nested group", :nested_describe => "yep" do
- it "sets the described class to the described class of the outer most group" do
- expect(example.example_group.described_class).to eq(ExampleGroup)
+ it "sets the described class to the described class of the outer most group" do |ex|
+ expect(ex.example_group.described_class).to eq(ExampleGroup)
end
- it "sets the description to 'A sample nested describe'" do
- expect(example.example_group.description).to eq('A sample nested group')
+ it "sets the description to 'A sample nested describe'" do |ex|
+ expect(ex.example_group.description).to eq('A sample nested group')
end
- it "has top level metadata from the example_group and its parent groups" do
- expect(example.example_group.metadata).to include(:little_less_nested => 'yep', :nested_describe => 'yep')
+ it "has top level metadata from the example_group and its parent groups" do |ex|
+ expect(ex.example_group.metadata).to include(:little_less_nested => 'yep', :nested_describe => 'yep')
end
- it "exposes the parent metadata to the contained examples" do
- expect(example.metadata).to include(:little_less_nested => 'yep', :nested_describe => 'yep')
+ it "exposes the parent metadata to the contained examples" do |ex|
+ expect(ex.metadata).to include(:little_less_nested => 'yep', :nested_describe => 'yep')
end
end
@@ -826,12 +808,12 @@ def define_and_run_group(define_outer_example = false)
expect(@before_all_top_level).to eq('before_all_top_level')
end
- it "can access the before all ivars in the before_all_ivars hash", :ruby => 1.8 do
- expect(example.example_group.before_all_ivars).to include('@before_all_top_level' => 'before_all_top_level')
+ it "can access the before all ivars in the before_all_ivars hash", :ruby => 1.8 do |ex|
+ expect(ex.example_group.before_all_ivars).to include('@before_all_top_level' => 'before_all_top_level')
end
- it "can access the before all ivars in the before_all_ivars hash", :ruby => 1.9 do
- expect(example.example_group.before_all_ivars).to include(:@before_all_top_level => 'before_all_top_level')
+ it "can access the before all ivars in the before_all_ivars hash", :ruby => 1.9 do |ex|
+ expect(ex.example_group.before_all_ivars).to include(:@before_all_top_level => 'before_all_top_level')
end
describe "but now I am nested" do
View
43 spec/rspec/core/example_spec.rb
@@ -27,7 +27,7 @@ def capture_stdout
$stdout = orig_stdout
end
- it 'can be pretty printed' do
+ it "can be pretty printed" do
output = ignoring_warnings { capture_stdout { pp example_instance } }
expect(output).to include("RSpec::Core::Example")
end
@@ -164,26 +164,26 @@ def assert(val)
end
end
- describe '#described_class' do
+ describe "#described_class" do
it "returns the class (if any) of the outermost example group" do
expect(described_class).to eq(RSpec::Core::Example)
end
end
describe "accessing metadata within a running example" do
- it "has a reference to itself when running" do
- expect(example.description).to eq("has a reference to itself when running")
+ it "has a reference to itself when running" do |ex|
+ expect(ex.description).to eq("has a reference to itself when running")
end
- it "can access the example group's top level metadata as if it were its own" do
- expect(example.example_group.metadata).to include(:parent_metadata => 'sample')
- expect(example.metadata).to include(:parent_metadata => 'sample')
+ it "can access the example group's top level metadata as if it were its own" do |ex|
+ expect(ex.example_group.metadata).to include(:parent_metadata => 'sample')
+ expect(ex.metadata).to include(:parent_metadata => 'sample')
end
end
describe "accessing options within a running example" do
- it "can look up option values by key", :demo => :data do
- expect(example.metadata[:demo]).to eq(:data)
+ it "can look up option values by key", :demo => :data do |ex|
+ expect(ex.metadata[:demo]).to eq(:data)
end
end
@@ -333,7 +333,7 @@ def run_and_capture_reported_message(group)
expect(message).to match(/An error occurred in an after.* hook/i)
end
- it 'does not print mock expectation errors' do
+ it "does not print mock expectation errors" do
group = RSpec::Core::ExampleGroup.describe do
example do
foo = double
@@ -422,7 +422,7 @@ def run_and_capture_reported_message(group)
end
end
- it 'does not interfere with per-example randomness when running examples in a random order' do
+ it "does not interfere with per-example randomness when running examples in a random order" do
values = []
RSpec.configuration.order = :random
@@ -436,4 +436,25 @@ def run_and_capture_reported_message(group)
expect(values.uniq).to have(2).values
end
+
+ describe "optional block argument" do
+ it "contains the example" do |ex|
+ expect(ex).to be_an(RSpec::Core::Example)
+ expect(ex.description).to match(/contains the example/)
+ end
+ end
+
+ %w[example running_example].each do |accessor|
+ describe accessor do
+ it "is deprecated" do
+ expect(RSpec).to receive(:deprecate)
+ send(accessor)
+ end
+
+ it "returns the current running example" do |ex|
+ allow(RSpec).to receive(:deprecate)
+ expect(send(accessor)).to eq ex
+ end
+ end
+ end
end
View
33 spec/rspec/core/memoized_helpers_spec.rb
@@ -56,6 +56,18 @@ def subject_value_for(describe_arg, &block)
end
describe "explicit subject" do
+ it "yields the example in which it is eval'd" do
+ example_yielded_to_subject = nil
+ example_yielded_to_example = nil
+
+ group = ExampleGroup.describe
+ group.subject { |e| example_yielded_to_subject = e }
+ group.example { |e| subject; example_yielded_to_example = e }
+ group.run
+
+ expect(example_yielded_to_subject).to eq example_yielded_to_example
+ end
+
[false, nil].each do |falsy_value|
context "with a value of #{falsy_value.inspect}" do
it "is evaluated once per example" do
@@ -184,6 +196,18 @@ def define_and_run_group
end
describe "with a name" do
+ it "yields the example in which it is eval'd" do
+ example_yielded_to_subject = nil
+ example_yielded_to_example = nil
+
+ group = ExampleGroup.describe
+ group.subject(:foo) { |e| example_yielded_to_subject = e }
+ group.example { |e| foo; example_yielded_to_example = e }
+ group.run
+
+ expect(example_yielded_to_subject).to eq example_yielded_to_example
+ end
+
it "defines a method that returns the memoized subject" do
list_value_1 = list_value_2 = subject_value_1 = subject_value_2 = nil
@@ -516,6 +540,15 @@ def count
expect(@nil_value_count).to eq(1)
end
+ let(:yield_the_example) do |example_yielded_to_let|
+ @example_yielded_to_let = example_yielded_to_let
+ end
+
+ it "yields the example" do |example_yielded_to_example|
+ yield_the_example
+ expect(@example_yielded_to_let).to equal example_yielded_to_example
+ end
+
let(:regex_with_capture) { %r[RegexWithCapture(\d)] }
it 'does not pass the block up the ancestor chain' do
Something went wrong with that request. Please try again.