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

dchelimsky · 2012-08-27T03:14:41Z

Let's yield current running example to it/example and before/after hooks instead of exposing it via the implicit example getter.

@myronmarston, @alindeman comments, please

travisbot · 2012-08-27T03:29:23Z

This pull request passes (merged 478a8e4 into 4f101b3).

travisbot · 2012-08-27T03:34:59Z

This pull request fails (merged 0861172 into 4f101b3).

myronmarston · 2012-08-27T03:35:01Z

Interesting idea. I assume this is in response to #663?

I like the fact that this creates fewer potential naming conflicts (especially since example could reasonably be used as a method name by end users), but I do think this potentially makes things more cumbersome. Consider some code like this:

module SomeHelperMethods
  def some_helper_method
    if example.metadata[:blah]
      do_something
    end
  end
end

RSpec.configure do |c|
  c.include SomeHelperMethods
end

In this highly contrived and totally generic example, the helper method is able to directly access the example metadata in a conditional. With your proposed change, this would no longer be possible -- all helper methods that need access to the example would have to accept (and be passed) an example argument. That feels more cumbersome than the current way that it works -- particularly when you have a helper method that calls a helper method that calls a helper method that needs access to the example. All the intermediary helper methods would need to accept (and pass on) the example arg, and then the example itself would have to pass it in.

So I guess you could say I'm on the fence about this.

BTW, if we do make this change, how many other method names are we "squatting" (for lack of a better term)? Are there likely to be other methods that could have similar conflicts?

dchelimsky · 2012-08-27T10:42:42Z

What if we hung example on RSpec? e.g.

module SomeHelperMethods
  def some_helper_method
    if RSpec.example.metadata[:blah]
      do_something
    end
  end
end

RSpec.configure do |c|
  c.include SomeHelperMethods
end

That makes it available for the edge case. Another option would be to offer a config option:

RSpec.configure do |c|
  c.expose_current_running_example_as :rspec_example
end

or some such.

The only other instance methods on ExampleGroup are running_example (deprecated predecessor to example), described_class (which, arguably, should be treated the same way) and instance_eval_with_rescue, which would be easy enough to refactor out (only used internally).

dchelimsky · 2012-08-27T10:43:06Z

Doing this w/ described_class, btw, would hurt :)

dchelimsky · 2012-08-27T10:51:45Z

What if we don't deprecate these methods for rspec-2.x, but "soft deprecate" them (i.e. a deprecation notice tells users how to turn it off - requires some infrastructure).

BTW - this is something I've wanted for a long time. I actually did it in response to a reaction I had working on slides for a preso I'm doing next week. This happened a lot w/ The RSpec Book - I'm writing about something and think "wow, that's lame," and fix it :) Perhaps it was subconsciously on my mind due to #633 as well, but that's not what I was thinking of.

In regard to #633, however, part of this commit points at the solution if we don't expose the block arg or deprecate example: always reference an ivar internally instead of accessing the example through the method, or hand the method internally on RSpec. WDYT about doing that regardless?

myronmarston · 2012-08-27T16:21:37Z

What if we hung example on RSpec?

I'm OK with that idea, although I think it should be current_example, because that's a lot more self-descriptive.

I also realized there's a fairly trivial way for end-users to add an example helper method to there example groups if they need it:

module ExampleHelper
  extend RSpec::Core::SharedContext
  attr_reader :example
  before { |ex| @example = ex }
end

RSpec.configure do |c|
  c.include ExampleHelper
end

Given the simplicity of that, maybe we should just direct users who need this towards doing that? It feels like overkill to add a bunch of infrastructure for a fairly simple deprecation like this.

Anyhow, one other thing I thought of...I think this is a potentially breaking change for anyone who uses lambdas for hooks or examples, e.g.:

def before_hook_for_foo(some_arg)
  lambda do
    # do something
  end
end

before &before_hook_for_foo(:bar)

Lambda semantics dictate that it raises an error if the number of args don't match (in contrast, proc semantics -- which are used for blocks -- do not raise an error if the number of args don't match). With the new example arg passed to the lambda, it'll blow up, I think.

In regard to #633, however, part of this commit points at the solution if we don't expose the block arg or deprecate example: always reference an ivar internally instead of accessing the example through the method, or hand the method internally on RSpec. WDYT about doing that regardless?

I think that's a good idea, regardless.

samwgoldman · 2012-08-27T16:33:49Z

I do use example in the manner described by @myronmarston, but only ever to get at the current example's metadata. I wonder if there isn't some nicer way to expose metadata

patmaddox · 2012-09-11T09:18:12Z

I personally like the block syntax & explicitly passing the example to the helper. Perhaps it's verbose because all helper methods will need it, but I like that the example is an explicit argument. It means the helper has a bit less knowledge of RSpec's internals. Granted you know it's in a module that's mixed into RSpec so the whole point is to extend it...but I figure anything we can do to have an explicit API is good.

JonRowe · 2013-03-31T07:10:47Z

What @totallymike said... /cc @myronmarston

myronmarston · 2013-03-31T14:39:53Z

I need to think about it some more, but I'd say the earliest we would make this change is 3.0.

myronmarston · 2013-06-18T06:21:17Z

This has sat for a while but I still like it and would like to get it in RSpec 3. FWIW :).

dchelimsky · 2013-06-19T12:27:05Z

It needs to be rebased. Working on that now.

coveralls · 2013-06-19T17:10:45Z

Coverage increased (+0%) when pulling 45d0e2b on yield-example into dee12fc on master.

…tance_eval

dchelimsky · 2013-06-19T17:22:00Z

I rebased this off master, force pushed, and added another commit. I'm pretty happy with it as/is but welcome feedback /cc @myronmarston @alindeman @JonRowe @soulcutter @samphippen

coveralls · 2013-06-19T17:35:16Z

Coverage increased (+0%) when pulling 7e0a9c3 on yield-example into dee12fc on master.

myronmarston · 2013-06-19T17:41:35Z

lib/rspec/core/example_group.rb

+        #   end
+        #
+        #   example "does something" do |ex|
+        #     # ex is a wrapper for the current running example


I thought ex was the current example object? In what sense is it a wrapper of the current example rather than than the current example itself?

You're correct. I think at one point I had a wrapper in mind (a la example and group proxies for rspec-1 formatters), but ended up just with the example.

myronmarston · 2013-06-19T17:53:02Z

Looking good. There are probably some cukes that we should edit so as to use this new style as well.

JonRowe · 2013-07-04T12:29:17Z

I'm more than prepared to go and lend a hand to things like capybara to help them gain RSpec 3.x support, I think in most cases the changes won't be that extreme, there are probably going to be more gems out there version locked to 2.x with a ~> that will cause issues than soft loaded gems like Capybara.

2.99 should print deprecation warnings for this, and we are planning to have that around for a significant time to help people get ready for the change, so I don't feel it's necessary to leave things like this around deprecated... we can help people upgrade for the change, we can document things they can do and suggested solutions, I hardly think this will be cutting people off at the knees.

I do however apologise for the language in my comment, it was more humour than perhaps it was taken.

dchelimsky · 2013-07-04T12:47:11Z

I'm more than prepared to go and lend a hand to things like capybara to help them gain RSpec 3.x support

That's great.

2.99 should print deprecation warnings for this, and we are planning to have that around for a significant time to help people get ready for the change

I think it's the combination of 2.99 and 3.0.0.beta/rc that needs to be around for a while before 3.0.0 final goes out so lib maintainers and end-users alike can see the deprecation warnings and validate that their resulting changes work with the new version. We've talked about this before but haven't really defined what "a significant time" means. I think we need some means of validating that enough people (definition TBD) are successfully using 3.0 pre-releases with a sufficiently wide variety of 3rd party gems (definition TBD) in play. Surveys? A user-sourced compatibility chart? An opt-in stats report that rspec-3.0.0 pre-releases can send to a server so we can collect data like gem versions, which we can use to publish a compatibility chart? Other ideas?

alindeman · 2013-07-04T13:37:52Z

What if we did expose RSpec.current_example as mention earlier? That way capybara and others could do something like:

current_example = RSpec.respond_to?(:current_example) ? RSpec.current_example : example

(the check could also be written in such a way that it'd only be performed once for performance reasons)

... and maintain compatibility? Or is there a compelling reason to not expose it like that?

alindeman · 2013-07-04T13:48:10Z

RSpec.current_example could be exposed publicly, but we wouldn't necessary document it anywhere other than the API docs. It'd be for library authors, and users writing examples in their apps would be encouraged to use the block argument.

Thoughts?

myronmarston · 2013-07-04T16:39:53Z

FWIW, I think the best way to encourage adoption of 3.x is to make the upgrade as simple and painless as possible, both for users and library authors, so there's no reason not to upgrade. That's why we're putting significant effort into 2.99, so that users have a very clear upgrade path, with explicit instructions about what's changing and how it affects them, rather than just a changelog they have to read to figure out how it applies to them.

In fact - I'm beginning to think we should deprecate example but leave it in RSpec-3.

I'm 👎 on this idea, but certainly could be convinced otherwise once we get more community feedback. One of the main wins of this PR was reducing the amount of namespace squatting we're doing, and if we leave it in place deprecated, we lose that benefit until RSpec 4...which is a long time to wait. I'd first like to see if we can't ease the transition in other ways.

RSpec.current_example could be exposed publicly, but we wouldn't necessary document it anywhere other than the API docs. It'd be for library authors, and users writing examples in their apps would be encouraged to use the block argument.

Thoughts?

👍 I like this.

dchelimsky · 2013-07-04T21:34:45Z

RSpec.current_example could be exposed publicly, but we wouldn't necessary document it anywhere other than the API docs. It'd be for library authors, and users writing examples in their apps would be encouraged to use the block argument.

Thoughts?
I like this.

That's backwards to me. If a lib author is going to make a change, we want him/her to follow the path forward and use the yielded example object, not propagate the dependency on example or running_example methods. Exposing the example and running_example methods helps end-users who are using libs that haven't been upgraded yet. They'd still be encouraged (via docs) to use the block arg in their own spec suites.

JonRowe · 2013-07-05T03:41:30Z

Thought, could we deprecate the direct calls to example now before we put out 2.14? That'd give even more notice in this case? I'm with @dchelimsky on the exposing RSpec.current_example is a bit of a backwards step...

dchelimsky · 2013-07-05T10:53:10Z

I'm with @dchelimsky on the exposing RSpec.current_example is a bit of a backwards step...

Except that's not what I intended :)

I was saying that RSpec.current_example would serve end users, not lib authors, which is the opposite of what @alindeman had proposed. I'm actually in favor of including it to support end-users and don't view it as a step back.

alindeman · 2013-07-05T19:13:24Z

I'm a tad confused, sorry. Why would end users want RSpec.current_example?

I'm nervous right now because rspec-rails' test suite on master is badly red due to capybara using example in a before and after hook. And I don't know how to fix it without making a breaking change that forces capybara to release a new version than only supports RSpec >= 2.14 and 3.x (where this change is merged) ... or necessitate adding some conditional logic against ::RSpec:Core::VERSION

... and this makes me suspect there are many other libraries that are now broken against RSpec master and will need to release a new major version if we don't provide some easier way to support both RSpec 2 and 3.

I agree that sometimes we need to break compatibility in a major version release, but I feel like we should be conservative about it and only do it when there's a really compelling reason and no great way to support a simple unifying API.

My thought was that RSpec.current_example could be that API. But maybe there's something better or something I'm missing?

In my opinion, even if you think this change is positive, it's simultaneously small yet far-reaching: I think we should add, document, and maybe help (via pull requests) gems that integrate with RSpec to support both 2.x and 3.x.

Thoughts?

dchelimsky · 2013-07-05T21:33:24Z

I'm a tad confused, sorry. Why would end users want RSpec.current_example?

They wouldn't except to protect against lib authors who are slow to upgrade.

Today there is some set of end users (possibly a set of 0, but I doubt it) who use example in their apps. When they upgrade to 2.99 they'll get deprecation warnings, they'll be able to use the yielded example instead, and they'll be ready for 3.0. Hooray!

Similarly there is some set of lib authors who use example in their libs. For the ones who update their libs to support RSpec 3 (with or without our help) they'll do the same thing: upgrade to 2.99, see deprecation warnings and use the yielded example instead (or use let to define a method that yields the example as @myronmarston described as a potential solution for rspec-rails). Once they release those changes their users can upgrade to RSpec 3 as well. Hooray!

The problem case is the end user who uses lib xyz, and lib xyz does not get updated, even though we've submitted pull requests and bought the author a bottle of Octomore (which is not particularly inexpensive, but oh, so delicious!). That end user can not upgrade to RSpec 3 without:

a) bagging lib xyz. Boo!
b) using RSpec.current_example to expose example so lib xyz works even though lib xyz's author didn't update it yet. Hooray!

Make sense?

That solves the Capybara issue, btw, without imposing anything on Capybara in the short run. It just makes the end user do a little extra work until the Capybara issue is properly solved within Capybara.

myronmarston · 2013-07-05T21:46:18Z

That solves the Capybara issue, btw, without imposing anything on Capybara in the short run. It just makes the end user do a little extra work until the Capybara issue is properly solved within Capybara.

This sounds good in theory, but the part that confuses me (and may be confuses Andy) is this:

using RSpec.current_example to expose example so lib xyz works even though lib xyz's author didn't update it yet. Hooray!

I don't see how an end user using RSpec.current_example will solve the issue of lib xyz using RSpec::ExampleGroup#example, which will raise a NoMethodError in RSpec 3. Users can use RSpec.current_example all they want to get access to the current example, but in lib xyz, as long as it calls the removed example method, it will still cause a NoMethodError to get raised.

dchelimsky · 2013-07-05T21:52:41Z

Aha! And now I see the disconnect. Buried deep within this thread I proposed something like this:

RSpec.configure do |c|
  c.expose_current_running_example_as :example
end

And I mistook RSpec.current_example for that. So, backtracking ...

I think we should include the expose_current_running_example_as config option to support the end user who uses a lib that has not upgraded to.
I'm OK with or without RSpec.current_example

myronmarston · 2013-07-05T22:05:31Z

I think we should include the expose_current_running_example_as config option to support the end user who uses a lib that has not upgraded to.

👍

I'm OK with or without RSpec.current_example

I think RSpec.current_example is potentially useful as a means to help libraries remain compatible with RSpec 2.x and 3.0, but it's not super important.

JonRowe · 2013-07-05T23:07:08Z

I'd rather

RSpec.configure do |c|
  c.expose_current_running_example_as :example
end

than

RSpec.current_example

(just my 2¢)

myronmarston · 2013-07-07T01:12:21Z

I'd rather

RSpec.configure do |c|
 c.expose_current_running_example_as :example
end

than

RSpec.current_example

(just my 2¢)

They have completely different purposes. It's not an either/or.

expose_current_running_example_as :example config option is meant to support users who are using gems that depend on the RSpec 2 API. It's not meant for library authors to use.
RSpec.current_example is an idea @alindeman suggested as a possible means to assist library authors in making their code work on RSpec 2 and RSpec 3. It's not meant for end users to use.

alindeman · 2013-07-07T15:46:34Z

I'm still really confused. This is not about Capybara using RSpec for its own test suite; instead, the problem is that Capybara integrates with RSpec ... adding methods and hooks to example groups for users to drive web browsers.

When you bring in Capybara to your application or library with require 'capybara/rspec', Capybara adds hooks to your own specs that use example.

The issue is how we make those hooks compatible with both RSpec <= 2.14 and 3, without creating a rift in Capybara itself (where Capybara has to release a version that's only compatible with RSpec >= 2.14, 3). If I were a maintainer of Capybara, I'd be pretty annoyed if RSpec made me do this, since my integration with RSpec is somewhat incidental to my gem's purpose.

I think RSpec.current_example is a way to do this, but I'm open to other ideas. What do you think?

dchelimsky · 2013-07-07T17:58:36Z

The issue is how we make those hooks compatible with both RSpec <= 2.14 and 3, without creating a rift in Capybara itself

@alindeman I don't think that the addition of RSpec.current_example is going to save 3rd party libs that use example from having to make changes to function with both rspec 2 and 3. They'll still have to something like:

if RSpec.respond_to?(:current_example)
  # use RSpec.current_example
else
  # use the local example method
end

Right?

alindeman · 2013-07-07T18:30:11Z

Right?

Yep, right. But at least there's an option, even if it's a conditional.

If we keep it as it is right now, there's nothing that doesn't feel very fragile and ugly (as far as I can tell anyway). I don't like this because there are a lot of gems that integrate optionally with RSpec (so you don't get rubygems backing up the version constraint) and because many gems integrate with RSpec as a convenience for users, even when that's not a core feature. Making a breaking change like this and giving no good option to support both versions feels like we're not being a very good citizen.

dchelimsky · 2013-07-07T19:41:05Z

Yep, right. But at least there's an option, even if it's a conditional.
If we keep it as it is right now, there's nothing that doesn't feel very fragile and ugly (as far as I can tell anyway).

I agree we don't have a better alternative at the moment. I read your concern about Capybara to mean that you thought there was a way that Capy wouldn't need any changes at all to properly support rspec-3.

I don't like this because there are a lot of gems that integrate optionally with RSpec (so you don't get rubygems backing up the version constraint) and because many gems integrate with RSpec as a convenience for users, even when that's not a core feature. Making a breaking change like this and giving no good option to support both versions feels like we're not being a very good citizen.

Agree with all of that ^^ as well, so I think we're on the same page at this point re: RSpec.current_example: we should add it unless we can come up with a better option for lib authors. That is a separate issue from end-users who use libs that haven't adapted, which could be resolved with config.expose_current_example_as :xxx, which should be documented as an interim measure. You on board w/ that @alindeman?

alindeman · 2013-07-07T19:46:36Z

Agree with all of that ^^ as well, so I think we're on the same page at this point re: RSpec.current_example: we should add it unless we can come up with a better option for lib authors. That is a separate issue from end-users who use libs that haven't adapted, which could be resolved with config.expose_current_example_as :xxx, which should be documented as an interim measure. You on board w/ that @alindeman?

Yes, these two things sound good to me :) I'm sorry for all the misunderstandings throughout this thread.

I'll make a PR for RSpec.current_example shortly. I'll open an issue for expose_current_example_as to make sure it doesn't get lost too.

As of RSpec 3, we can't call A0DE example in test. see discussion: rspec/rspec-core#666

As of RSpec 3, we can't call example in test. see discussion: rspec/rspec-core#666

This commit was imported from rspec/rspec-core@917b3a9.

dchelimsky added 2 commits June 19, 2013 14:02

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

d941ba7

rdoc tweaks for deprecated example method and block argument

45d0e2b

rename to instance_exec_with_rescue and remove unused override of ins…

7e0a9c3

…tance_eval

myronmarston reviewed Jun 19, 2013
View reviewed changes

dchelimsky added 3 commits June 19, 2013 16:26

doc tweaks [ci skip]

aab3c4d

add examples for deprecated Example#example and Example#running_example

ed5a3be

use double quotes (consistency w/in the file)

6e02be3

myronmarston added a commit that referenced this pull request Jul 7, 2013

Add changelog entry for #666.

89412cc

myronmarston added a commit that referenced this pull request Jul 7, 2013

Add changelog entry for #666,

917b3a9

alindeman mentioned this pull request Jul 7, 2013

RSpec.configuration.expose_current_running_example_as :example #963

Closed

thomas-holmes mentioned this pull request Jul 20, 2013

Support RSpec 3's deprecation of use of example outside of hooks teamcapybara/capybara#1122

Closed

yujinakayama mentioned this pull request Nov 13, 2013

Support conversion of RSpec::Core::ExampleGroup#example yujinakayama/transpec#23

Closed

shin1ohno pushed a commit to shin1ohno/autodoc that referenced this pull request Dec 19, 2013

Make Rspec 3 compliant

61dc733

As of RSpec 3, we can't call A0DE example in test. see discussion: rspec/rspec-core#666

shin1ohno mentioned this pull request Dec 19, 2013

Make Rspec 3 compliant r7kamura/autodoc#7

Closed

yuya373 pushed a commit to yuya373/autodoc that referenced this pull request Oct 24, 2014

Make Rspec 3 compliant

016f6a2

As of RSpec 3, we can't call example in test. see discussion: rspec/rspec-core#666

junaruga mentioned this pull request May 24, 2016

Update to RSpec 3 teamcapybara/xpath#73

Closed

yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 6, 2021

[core] Add changelog entry for rspec/rspec-core#666,

ec5d91e

This commit was imported from rspec/rspec-core@917b3a9.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

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

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

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment