Reconsider `shared_context_metadata_behavior`
Created by: pirj
Background
We have had a problem with metadata defined on spec-local shared example groups (shared_examples
/ shared_examples_for
/ shared_context
) causing them to be included in completely unrelated example group reported in [1]/[2]. Some more information in https://github.com/rspec/rspec-core/issues/1762.
In https://github.com/rspec/rspec-core/issues/1790 (https://github.com/rspec/rspec-core/commit/ed2d59c8a97d32f4a20e91d0abdad87f90d2b930), shared_context_metadata_behavior
with an non-default :apply_to_host_groups
value. https://github.com/rspec/rspec-core/commit/3589ab577d09db88ef5d5f0d60e8c35bfc55691f added it to the project initializer along with a note that it will become the default and only option in RSpec 4.
:trigger_inclusion
remained the default setting, though, for SemVer reasons.
In a nutshell:
`:trigger_inclusion`: shared context will be implicitly included in any groups (or examples) that have matching metadata.
`:apply_to_host_groups`: the metadata will be inherited by the metadata hash of all host groups and examples.
Usage
Personally, I've never seen :apply_to_host_groups
being used the way it was designed for. On the other hand, I've seen some projects use :trigger_inclusion
for globally-defined shared example groups/contexts.
But it's a sample of one. Let's check around.
As a sandbox for new rubocop-rspec
cops, I've assembled a list of most starred Ruby projects that use RSpec, real-world-rspec
, ~35 projects total. Out of those 35 (it includes RSpec repos, too!), 7 use :apply_to_host_groups
in their spec helpers:
24pullrequests/ administrate/ Homebrew/ camaleon-cms/ canvas-lms/ capistrano/ capybara/ cartodb/ chatwoot/ chef/ diaspora/ discourse/ locomotivecms/ errbit/ fat_free_crm/ forem/ gitlabhq/ hound/ huginn/ lobsters/ loomio/ mastodon/ open-source-billing/ publify/ puppet/ radiant/ refinerycms/ rspec-core/ rspec-expectations/ rspec-mocks/ rspec-rails/ rubocop/ rubytoolbox/ sharetribe/ solidus/ spree/
engine/spec/spec_helper.rb|48| 10: config.shared_context_metadata_behavior = :apply_to_host_groups
rspec-rails/spec/spec_helper.rb|56| 10: config.shared_context_metadata_behavior = :apply_to_host_groups
lobsters/spec/spec_helper.rb|45| 10: config.shared_context_metadata_behavior = :apply_to_host_groups
rubytoolbox/spec/spec_helper.rb|49| 10: config.shared_context_metadata_behavior = :apply_to_host_groups
forem/spec/spec_helper.rb|58| 10: config.shared_context_metadata_behavior = :apply_to_host_groups
gitlabhq/qa/spec/spec_helper.rb|56| 10: config.shared_context_metadata_behavior = :apply_to_host_groups
chatwoot/spec/spec_helper.rb|16| 10: config.shared_context_metadata_behavior = :apply_to_host_groups
And one in lib
:
capybara/lib/capybara/spec/spec_helper.rb|19| 16: config.shared_context_metadata_behavior = :apply_to_host_groups
Others, since they don't have this setting and the default is :trigger_inclusion
, either don't use shared example groups metadata, or rely on triggering inclusion.
Let's take a look at usages. Open this spoiler to see **all** 40 shared groups/contexts with metadata (out of ~3000 total shared groups)
rspec-core/spec/rspec/core/metadata_spec.rb|317| 42: RSpec.shared_examples_for("some shared behavior", :include_it => true) do puppet/spec/shared_contexts/digests.rb|16| 1:shared_context('with supported digest algorithms', :uses_checksums => true) do puppet/spec/shared_contexts/digests.rb|27| 1:shared_context("when digest_algorithm is set to sha256", :digest_algorithm => 'sha256') do puppet/spec/shared_contexts/digests.rb|42| 1:shared_context("when digest_algorithm is set to md5", :digest_algorithm => 'md5') do puppet/spec/shared_contexts/digests.rb|57| 1:shared_context("when digest_algorithm is set to sha512", :digest_algorithm => 'sha512') do puppet/spec/shared_contexts/digests.rb|72| 1:shared_context("when digest_algorithm is set to sha384", :digest_algorithm => 'sha384') do puppet/spec/shared_contexts/digests.rb|87| 1:shared_context("when digest_algorithm is set to sha224", :digest_algorithm => 'sha224') do diaspora/spec/support/gon.rb|3| 1:shared_context :gon do diaspora/spec/spec_helper.rb|163| 1:shared_context suppress_csrf_verification: :none do rubocop/lib/rubocop/rspec/shared_contexts.rb|5| 7:RSpec.shared_context 'isolated environment', :isolated_environment do rubocop/lib/rubocop/rspec/shared_contexts.rb|43| 7:RSpec.shared_context 'maintain registry', :restore_registry do rubocop/lib/rubocop/rspec/shared_contexts.rb|56| 7:RSpec.shared_context 'config', :config do # rubocop:disable Metrics/BlockLength rubocop/lib/rubocop/rspec/shared_contexts.rb|114| 7:RSpec.shared_context 'mock console output' do rubocop/lib/rubocop/rspec/shared_contexts.rb|126| 7:RSpec.shared_context 'ruby 2.4', :ruby24 do rubocop/lib/rubocop/rspec/shared_contexts.rb|130| 7:RSpec.shared_context 'ruby 2.5', :ruby25 do rubocop/lib/rubocop/rspec/shared_contexts.rb|134| 7:RSpec.shared_context 'ruby 2.6', :ruby26 do rubocop/lib/rubocop/rspec/shared_contexts.rb|138| 7:RSpec.shared_context 'ruby 2.7', :ruby27 do rubocop/lib/rubocop/rspec/shared_contexts.rb|142| 7:RSpec.shared_context 'ruby 3.0', :ruby30 do brew/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb|34| 7:RSpec.shared_context "Homebrew Cask", :needs_macos do chef/spec/support/shared/functional/securable_resource.rb|78| 1:shared_context "use Windows permissions", :windows_only do canvas-lms/spec/lib/turnitin/turnitin_spec_helper.rb|22| 7:RSpec.shared_context "shared_tii_lti", :shared_context => :metadata do canvas-lms/spec/lib/turnitin/turnitin_spec_helper.rb|22| 41:RSpec.shared_context "shared_tii_lti", :shared_context => :metadata do canvas-lms/spec/lti2_course_spec_helper.rb|22| 7:RSpec.shared_context "lti2_course_spec_helper", :shared_context => :metadata do canvas-lms/spec/lti2_course_spec_helper.rb|22| 50:RSpec.shared_context "lti2_course_spec_helper", :shared_context => :metadata do canvas-lms/spec/plagiarism_platform_spec_helper.rb|22| 7:RSpec.shared_context "plagiarism_platform", :shared_context => :metadata do canvas-lms/spec/plagiarism_platform_spec_helper.rb|22| 46:RSpec.shared_context "plagiarism_platform", :shared_context => :metadata do canvas-lms/spec/lti2_spec_helper.rb|22| 7:RSpec.shared_context "lti2_spec_helper", :shared_context => :metadata do canvas-lms/spec/lti2_spec_helper.rb|22| 43:RSpec.shared_context "lti2_spec_helper", :shared_context => :metadata do canvas-lms/spec/lti_1_3_tool_configuration_spec_helper.rb|22| 7:RSpec.shared_context "lti_1_3_tool_configuration_spec_helper", shared_context: :metadata do canvas-lms/spec/lti_1_3_tool_configuration_spec_helper.rb|22| 64:RSpec.shared_context "lti_1_3_tool_configuration_spec_helper", shared_context: :metadata do canvas-lms/spec/lti_1_3_spec_helper.rb|23| 7:RSpec.shared_context "lti_1_3_spec_helper", shared_context: :metadata do canvas-lms/spec/lti_1_3_spec_helper.rb|23| 45:RSpec.shared_context "lti_1_3_spec_helper", shared_context: :metadata do canvas-lms/spec/apis/lti/lti2_api_spec_helper.rb|24| 7:RSpec.shared_context "lti2_api_spec_helper", :shared_context => :metadata do canvas-lms/spec/apis/lti/lti2_api_spec_helper.rb|24| 47:RSpec.shared_context "lti2_api_spec_helper", :shared_context => :metadata do gitlabhq/spec/lib/gitlab/git/merge_base_spec.rb|11| 3: shared_context 'existing refs with a merge base', :existing_refs do gitlabhq/spec/lib/gitlab/git/merge_base_spec.rb|17| 3: shared_context 'when passing a missing ref', :missing_ref do gitlabhq/spec/lib/gitlab/git/merge_base_spec.rb|23| 3: shared_context 'when passing refs that do not have a common ancestor', :no_common_ancestor do gitlabhq/spec/lib/gitlab/ci/config/entry/retry_spec.rb|8| 3: shared_context 'when retry value is a numeric', :numeric do gitlabhq/spec/lib/gitlab/ci/config/entry/retry_spec.rb|13| 3: shared_context 'when retry value is a hash', :hash do rspec-expectations/spec/spec_helper.rb|72| 7:RSpec.shared_context "with #should enabled", :uses_should do rspec-expectations/spec/spec_helper.rb|99| 7:RSpec.shared_context "with #should exclusively enabled", :uses_only_should do rspec-expectations/spec/spec_helper.rb|120| 7:RSpec.shared_context "with warn_about_potential_false_positives set to false", :warn_about_potential_false_positives do
If we make an intersection with the previous list (rspec-rails, lobsters, rubytoolbox, forem, gitlabhq, chatwoot), it turns out that no project uses :apply_to_host_groups
.
gitlabhq
might be a bit confusing, they actually have two spec helpers, gitlabhq/qa/spec/spec_helper.rb
and gitlabhq/spec/spec_helper.rb
.
And they do use :trigger_inclusion
:
shared_context 'existing refs with a merge base', :existing_refs do
let(:refs) do
%w(304d257dcb821665ab5110318fc58a007bd104ed 0031876facac3f2b2702a0e53a26e89939a42209)
end
end
describe '#sha' do
context 'when the refs exist', :existing_refs do
in their specs.
We use it, too. rspec-expectations
:
RSpec.shared_context "with warn_about_potential_false_positives set to false", :warn_about_potential_false_positives do
original_value = RSpec::Expectations.configuration.warn_about_potential_false_positives?
after(:context) { RSpec::Expectations.configuration.warn_about_potential_false_positives = original_value }
end
Preliminary conclusion: those popular Ruby projects that use RSpec who configured shared_context_metadata_behavior
to :apply_to_host_groups
did this blindlessly and never used it.
Semantic
We have two ways of inclusing shared groups. include_context
/include_examples
and it_behaves_like
. The latter creates a nested group.
It doesn't make much sense to apply metadata that is defined for an implicitly created nested group.
On the other hand, just like with several let
s defined in different included contexts, metadata, if applied from different included contexts, has a chance to override one another, and we don't print a warning for this case, leaving space for confusion.
If we happen to remove :trigger_inclusion
, what would we recommend to replace it? E.g. for globally-defined shared groups:
For globally-defined shared groups this works:
RSpec.shared_examples 'it is odd' do
it { is_expected.to be_odd }
end
RSpec.configure do |config|
config.include_context 'it is odd', :odd
end
RSpec.describe do
context 'odd', :odd do
subject { 1 }
end
end
However, why is it include_context
?
Well, we only have include_context
on our Configuration
object. There's not include_examples
or it_behaves_like
there.
More common and contrived usage:
config.include_context "example guest user", :type => :request
But, what to promise in return for locally-defined shared groups?
Sometimes, it's quite unweildy to call include_context
/it_behaves_like
, and implicit inclusion via matching metadata is useful to make things dry. At a cost of a little magic. Which RSpec metadata is all about anyway.
Unpopular opinion
No doubt that this option has solved the issue with the inclusion of shared examples defined in a completely unrelated scope.
However, was there a good reason to "apply metadata to host group"? Is it used? Is it useful? Isn't it confusing?
Proposal
I suggest:
- keep
shared_context_metadata_behavior
option along with its default:trigger_inclusion
value - remove notions of deprecations
... release 4.0
- fix the problem with incorrect inclusions (proof of concept)
- change the project initializer, remove deprecation note, and use
:trigger_inclusion
as the default - deprecate
:apply_to_host_groups
If we keep :trigger_inclusion
the default, the behaviour won't change for the majority. Less tickets.
Doubts
It may require a significant overhaul to fix the inclusion, up to the point where it makes it barely possible. I still hope the reality won't shatter my youthful overly-optimistic dreams, and it's doable.