Get rid of the code poking around RSpec's internals

[pirj]

Problem

We used to access and replace RSpec.configuration and RSpec.world in several places.
This was prone to error, since those two can run out of sync, e.g.:

with_isolated_config do
  RSpec::Core::ExampleGroup.describe('a view') { it { } }
  RSpec.configuration.world.example_groups # => []
end

This case specifically is due to in RSpec::Core::ExampleGroup.describe:

RSpec.world.example_groups

Also, we were using this:

  config.around(:example) do |example|
    real_world = RSpec.world
    RSpec.instance_variable_set(:@world, RSpec::Core::World.new)
    example.run
    RSpec.instance_variable_set(:@world, real_world)
  end

RSpec::Core::Configuration.new gets its own world:

  def with_isolated_config
    original_config = RSpec.configuration
    RSpec.configuration = RSpec::Core::Configuration.new

which is an RSpec::Core::World::Null.

I have strong doubts if it would make sense to change the Core to:

- RSpec.world.example_groups
+ RSpec.configuration.world.example_groups

Not to say that those hooks and helpers dig too deep into RSpec's internals.

Solution

It's our lucky day! RSpec provides sandboxing out of the box.

[JonRowe]

So we shouldn't be doing this:

config.around(:example) do |example|
  real_world = RSpec.world
  RSpec.instance_variable_set(:@world, RSpec::Core::World.new)
  example.run
  RSpec.instance_variable_set(:@world, real_world)
end

We're reaching into RSpec's internals and this gem is no longer supposed to be doing that.

I would like us to eliminate this, and if that requires refactoring the world to not be so global, thats a good thing.

[pirj]

I think I've found a better solution.

[pirj]

Better viewed as https://github.com/rspec/rspec-rails/pull/2365/files?diff=split&w=1

[pirj]

Was included in Helpers that didn't hesitate to include itself in RSpec.configuration.

[JonRowe]

I mean does it have any effect? It can't have been used very often...

[pirj]

36 failures, 100% in generator specs, all undefined method 'type_metatag'.
Besides type_metatag, generator specs use no methods from FeatureCheck.

I'll include this module exclusively to generator specs.

Frankly, I don't see a good reason for this method to exist:

      def type_metatag(type)
        "type: :#{type}"
      end

but I'd prefer to avoid this unrelated refactoring here.

[JonRowe]

I mean does it have any effect? It can't have been used very often...

[JonRowe]

Can we not just include this where it's used?

[pirj]

We can, but type_metatag is used in every one of them except for install generator spec.

[pirj]

I intended to get rid of this method altogether, replacing all usages to type: :some_type, and this include as well.
Ok to keep it here and remove in a follow-up?
Or keep type_metatag and inline includes into generator specs?
Or keep this include as is and leave type_metatag alone?
I don't have a preference here, up to you.

[JonRowe]

So you can remove this from here, and add it here:

module RSpec
  module Rails
    module Specs
      module Generators
        module Macros
        # spec/support/generators.rb:18
        def self.included(klass)
          klass.extend(Macros)
          klass.include(RSpec::Rails::FeatureCheck)
        end

Which I think is neater until a later refactoring is done.

[pirj]

Good point. Done.