Add config.when_first_matching_example_defined.

[myronmarston]

In working on implementing the solution for #1920, I realized that implementing it becomes trivial if we have a hook like config.when_first_matching_example_defined so I decided to start there. We could stick with just this API -- focus filtering can be setup with the desired semantics with:

RSpec.configure do |config|
  config.when_first_matching_example_defined(:focus) do
    config.focus_run :focus
  end
end

It's probably still worth providing config.filter_run_when_matching for this common case, though.

Before merging this we at least need a cuke documenting it.

Thoughts, @rspec/rspec?

[chrisarcand]

It might be trivial to implement but it seems pretty odd to someone never seeing it before (and filter_run_when_matching seems pretty straightforward). What other use case besides this one would you ever use when_first_matching_example_defined for? 😕 Is this worth adding an option over now when you really just want the filter_run_when_matching option?

[fables-tales]

@myronmarston I think this makes sense as an internal api (we could even label it public) but I think most people who are only RSpec users, as opposed to plugin authors or RSpec maintainers, won't ever use it. I think I agree with @chrisarcand in that we should surface a meaningful API to the user as the primary interface to this. In general though, having an API like this is going to be useful for plugin authors, so I'm OK with exposing it.

[myronmarston]

What other use case besides this one would you ever use when_first_matching_example_defined for?

If you're focused on keeping RSpec boots low by making spec/spec_helper.rb very lightweight, I think this is a useful API to help with that. Anytime I have some one-time setup that is only required by a subset of specs, I tend to put it in a separate file (under spec/support, typically) and then spec files that need it require the support file and tag the example group to opt-in to any associated hooks and module inclusions, e.g.:

require 'support/db'

RSpec.describe SomeClassThatUsesTheDB, :db do
  # ...
end

It's always felt sub-optimal to me that both the require and the :db tag are necessary to make this work. It's duplication that happens in every spec file that uses the DB. If I forget to put the require 'support/db' line in a spec file that uses the DB, I can get in a situation where the spec file passes when run with the entire suite, but fails when run individually (because other spec files load the support file).

With this option, these problems can be eliminated with one line of config in spec_helper:

RSpec.configure do |config|
  config.when_first_matching_example_defined(:db) do
    require 'support/db'
  end
end

Or, the contents of support/db could just be moved into spec_helper in that when_first_matching_example_defined(:db) block if preferred.

The point is, it allows me to automatically do something if any :db examples are loaded while avoiding that cost if none are -- which makes it easier to ensure the necessary DB boostrapping/setup is performed when needed w/o burdening spec_helper with code that always runs and bogs things down.

Anyhow, I agree with adding the config.filter_run_when_matching option but will probably do it in a separate PR after this one is merged. Any concerns with what's here besides the lack of a cuke?

[chrisarcand]

Thanks for the thoughtful response; I like your example, would certainly utilize that. 😄 👍

[xaviershay]

It's always felt sub-optimal to me that both the require and the :db tag are necessary to make this work.

This.

[xaviershay]

semantically, it seems weird to be running setup code at define time. before(:suite) feels more like "they're all defined, now let's run something".

I can't think of a realistic situation where this matters though, so maybe not important.

Did you consider options like before_matching(:db, :suite) etc?

[myronmarston]

Did you consider options like before_matching(:db, :suite) etc?

I did not. Actually, for before(:suite), extra args are ignored (although we may print a warning -- I'd have to check), so we could potentially use the extra args here for this purpose.

I'm leaning against it, though. Consider the use case I explained above -- where you want to conditionally require 'support/db' only if there are loaded examples matching :db. If support/db also defines a before(:suite) hook, and the require 'support/db' line is executed by a before(:suite) hook -- then I'm not sure it'll work as desired. We don't generally support you defining additional hooks while running said hooks. I think we just get the list of hooks up front, and run them, without re-querying the hook state to see if there are new ones.

Or consider the main use case that drove this PR: the new filter_run_when_matching API we want to add. Currently before(:suite) hooks are executed and example filtering is applied between all spec files being loaded and running the first spec, but I'm not sure which order they happen in. Suite hooks might execute first, then filtering could be applied, or vice versa. It hasn't been important before now and I don't think I want to make it important as part of this feature.

So I think I'd like to stick with when_first_matching_example_defined but I can rewrite the docs here to no longer compare it directly to before(:suite). I may re-write the example to show it being used to require a support file as well.

Would that be sufficient, @xaviershay?

[xaviershay]

sounds good, re-writing example to emphasize seems good

[JonRowe]

As before(:suite) doesn't support the metadata filtering I'd +1 a rewrite here

[JonRowe]

It's always felt sub-optimal to me that both the require and the :db tag are necessary to make this work.
This.

Also 👍. I regularly build "short circuit"s into my spec helper for specific before hooks so as to execute setup "just in time" before running specs.

I'd use this and I'd educate people to use it.

[myronmarston]

OK, I rewrote the docs and also added a cuke.

Unfortunately, I can't get the cuke to pass. Specifically, the "Starting a DB transaction" and "Rolling back a DB transaction" lines are not printed, indicating the around(:example, :db) hook isn't being applied. I've figured out why but I'm not sure what the best course of action is. Here's what's going on:

1. The integration spec example group tagged with :db is defined.
2. The lambda is invoked and hits the return unless example_or_group_meta.key?(:example_group) line. Since it's a group it does not have an :example_group key and it returns w/o running the user's callback.
3. If the around(:example, :db) hook was defined at this point, it would get added to the group, but it's not, so it is not added to the group.
4. The integration example is defined, triggering the user's callback to get run.
5. spec/support/db.rb is loaded and the around(:example, :db) hook is defined, but it's defined too late for it to apply to the group as was intended.

So...I'm not sure what the best course of action is. Thoughts? #1939 relates to this; if it was fixed, I don't think this would be a problem.

[myronmarston]

OK, I've reworked this PR a bit (based on #2185, #2188, #2189 and #2196) and it should pass now. Does someone want to re-review?

[myronmarston]

...and now it's green :).

[JonRowe]

s/childeren/children/

[JonRowe]

Cleaned it up post merge :)

[myronmarston]

Thanks :).