Minor improvements #2362
Minor improvements #2362
Conversation
This happens in some circumstances:
bundle exec rails new ./tmp/example_app --no-rc --skip-javascript --skip-bootsnap --skip-sprockets --skip-git --skip-test-unit --skip-listen --skip-bundle --template=example_app_generator/generate_app.rb
Gem::RuntimeRequirementNotMetError: spring requires Ruby version >= 2.4.0. The current ruby version is 2.3.0.
An error occurred while installing spring (2.1.0), and Bundler cannot continue.
spec/support/helpers.rb
Outdated
| @@ -4,6 +4,7 @@ module Helpers | |||
| def with_isolated_config | |||
| original_config = RSpec.configuration | |||
| RSpec.configuration = RSpec::Core::Configuration.new | |||
| RSpec.configuration.world = RSpec.world | |||
There was a problem hiding this comment.
The whole point of isolated configuration is to be isolated from the global test run, we don't want the child configuration sharing the parent world, else those specs get counted in various places. What were you seeing that made you want to do this?
There was a problem hiding this comment.
Sorry, I disagree. with_isolated_config sounds like the configuration is isolated, not that what is inside the block should be in a different world.
We wrap each example to run in its own world:
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)
endAlso, all current usages of with_isolated_config wrap the whole example it is used in, there are no separate "inside" and "outside" example parts that would be prone to affect each other.
WDYT of making with_isolated_config a metadata-induced around hook to make this more obvious?
There was a problem hiding this comment.
Wow I had no idea we did that, ok, well you need that world, I don't think you can rely on the global here...
There was a problem hiding this comment.
So it seems that the real problem, that prevents us from using a unique isolated configuration is in RSpec::Core::ExampleGroup.describe:
RSpec.world.example_groupsShould it be a RSpec.configuration.world.example_groups?
This results in the following when called from within with_isolated_config:
I don't think you can rely on the global here...
The whole point is that I don't rely on the global, real one. The global is kept in real_world during the examples, and RSpec.world is assigned (using instance_variable_set) a World.new.
But RSpec::Core::Configuration.new gets its own world:
def with_isolated_config
original_config = RSpec.configuration
RSpec.configuration = RSpec::Core::Configuration.newwhich is an RSpec::Core::World::Null.
I strongly doubt it would make sense to change the Core to:
- RSpec.world.example_groups
+ RSpec.configuration.world.example_groupsBut in this case, setting an isolated world initialized specifically for this example to an isolated config that was also initialized for this example makes total sense.
There was a problem hiding this comment.
Can you drop this change for now, and open another PR to work on this? I'm happy to merge the rest of these changes, but I'd like to look at this a bit more
| group.run(failure_reporter) | ||
| expect(failure_reporter.exceptions).to eq [] | ||
| expect(run_count).to eq 1 | ||
| group = RSpec::Core::ExampleGroup.describe 'a view', type: :view do |
There was a problem hiding this comment.
Can you restore this, it was a regression check specifically for configuration hooks added (7 years ago!!) in #833
There was a problem hiding this comment.
Happy for the doc string / or a comment to be changed to mention this though, as I had to check to find it 😂
The example is hard to comprehend In this example, we're making sure that `view` is available in hooks. There's no big difference in globally defined hooks and local hooks, so it's not necessary to define one on the `configuration`, and thus `with_isolated_config` becomes unnecessary as well.
Follow-up to #2360
Explanations in commit messages.