Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Specify shared_context used with conditions in example group #2775

Closed
wants to merge 1 commit into from

Conversation

avit
Copy link
Contributor

@avit avit commented Oct 19, 2020

This is not a fully realized example, but some kind of functionality similar to this could help make it easier to use shared contexts.

Currently, calling include_context within an example group, and global RSpec.configuration.include_context have a subtle but major difference that is not documented:

  • In global configuration, the context is not included globally, but metadata filters get registered instead.
  • Under the example group, the metadata filters argument is ignored, and the shared context is included directly.

It's not exactly clear that this happens from reading the docs, and there is no warning where the filters are ignored. (There is also no example in the docs that shows how a shared_context can be declared locally within a group.)

Ultimately, I would like a way to more easily scope shared contexts with metadata filters, and combine them with shared examples to write a test matrix. Maybe this fictional example will show what I'm after:

RSpec.describe "polling" do

  # DECLARE EXPECTED OUTCOMES AS SHARED EXAMPLES

  shared_examples "vote counted" do
      # ...
  end

  shared_examples "vote discarded" do
    # ...
  end

  shared_examples "voter turned away" do
    # ...
  end

  # DECLARE DIMENSIONS IN THE TEST MATRIX AS SHARED CONTEXTS

  shared_context "is registered" do
    let(:voter_status) { :registered }
  end

  shared_context "is unregistered" do
    let(:voter_status) { nil }
  end

  shared_context "is early" do
    let(:poll_open) { true }
  end

  shared_context "is late" do
    let(:poll_open) { false }
  end

  # REGISTER CONTEXTS FOR METADATA CONDITIONS
  # (This is the part where I'm dreaming)

  include_context "is registered", registered: true
  include_context "is unregistered", unregistered: true
  include_context "is early", early: true
  include_context "is late", late: true

  # TEST MATRIX

  context "as a good citizen", :registered, :early do
    it_behaves_like "vote counted"
  end
end

The obvious workaround is to manually include_context repeatedly where each one applies. It just seems like there could be a way to use filters to make things more readable & easier to spot that all the permutations are covered.

@pirj
Copy link
Member

pirj commented Oct 19, 2020

The following works:

RSpec.describe "polling" do
  let(:voter_status) { nil }
    let(:poll_open) { false }

  # DECLARE DIMENSIONS IN THE TEST MATRIX AS SHARED CONTEXTS

  shared_context "is registered", registered: true do
    let(:voter_status) { :registered }
  end

  shared_context "is early", early: true do
    let(:poll_open) { true }
  end

  # TEST MATRIX

  context "as a good citizen", :registered, :early do
    specify "vote counted" do
      expect(voter_status).to eq :registered
      expect(poll_open).to eq true
    end
  end

  context "late citizen" do
    specify "vote not counted" do
      expect(voter_status).to be_nil
      expect(poll_open).to eq false
    end
  end
end

Do you find it counter-intuitive?

@pirj
Copy link
Member

pirj commented Oct 19, 2020

In global configuration, the context is not included globally

Not sure I understand. It is included globally with config.include_context. Unless you specify metadata, which filters the examples to include the shared context to.
Can you provide a reproduction example where it's not included where it feels it should have?

@pirj
Copy link
Member

pirj commented Oct 19, 2020

The following looks like a bug, or at least it's confusing

RSpec.describe "polling" do
  let(:open) { false }

  shared_context "is early" do
    let(:open) { true }
  end

  # Works fine with `it_behaves_like` with metadata
  it_behaves_like "is early", early: true
  # Fails with `include_context`/`include_examples`
  #   it_behaves_like "is early", early: true

  context "early", :early do
    specify "yeah" do
      expect(open).to eq true
    end
  end

  context "late" do
    specify "oops" do
      expect(open).to eq false
    end
  end
end

The reason that I find this confusing is that without metadata the shared context gets included.
Sure, it's included in the example group where it is defined directly, and not to any nested groups with matching metadata.
It's more or less justifiable for include_examples where counterpart it_behaves_like exists. Not so much for include_context though.

@JonRowe What is the expected behaviour? Is this by design and we should emphasize this in the documentation?

@avit Would you like to hack on it?

klass.update_inherited_metadata(@metadata) unless @metadata.empty?
and
def self.include_context(name, *args, &block)
may be a good start.

@avit
Copy link
Contributor Author

avit commented Oct 20, 2020

The following works:
...
Do you find it counter-intuitive?

That's the way I've been writing RSpec for years. I think it's great, but sometimes I think flatter contexts by inclusion are easier to reason about than nesting.

The nesting often means related examples (with just one changed condition) end up being specified very far apart, and the stack of nested contexts must be decided carefully to get the best grouping of examples together. Wider contexts like "when logged in" or "as an admin" make sense to set up outermost, or else you end up repeating them a lot. A conflict arises when 2 or more intersecting contexts are equally common and no stacking is clearly "better".

Nesting also means that outer contexts are implicitly inherited. I would rather see the error that something like voter_status is not defined, than to get the wrong inherited value.

@avit
Copy link
Contributor Author

avit commented Oct 20, 2020

It is included globally with config.include_context. Unless you specify metadata

It's specifically the difference in usage with metadata conditions that I'm interested in here. The unconditional configuration.include_context obviously gets included globally, and that's fine.

(I really think the intersection of shared examples with including different shared contexts could be useful.)

The following looks like a bug, or at least it's confusing

it_behaves_like "is early", early: true

According to the docs, is this passing metadata to the context, or yielded parameters to the block? It's not clear.

The differences between include_examples and it_behaves_like being a nested context could also be clarified. I'd be happy to work on this if we can work out how it should work. Possibly:

  • Better documentation
  • An error where block params / metadata conditions are not supported
  • Make something like nested_contexts_include "happy", happy: true (??)

@pirj
Copy link
Member

pirj commented Jan 6, 2021

Please disregard my above code examples. It slipped my mind the implicit inclusion with the matching metadata was discouraged, and will be removed in RSpec 4.

There is an interesting conversation here regarding this.

It seems that metadata specified in example group include_context call is ignored, and the context gets included straight away, and it seems to be a bug to me:

RSpec.describe "polling" do
  shared_examples "vote counted" do
    it 'is counted' do
      expect(poll_open).to be(true)
    end
  end

  shared_context "is early" do
    let(:poll_open) { true }
  end

  include_context "is early", early: 'early'

  context "as a good citizen", early: 'early' do
    it_behaves_like "vote counted"
  end

  context "as a BAD citizen", early: 'LATE' do # This passes, but should not
    it_behaves_like "vote counted"
  end
end

@pirj
Copy link
Member

pirj commented Jan 6, 2021

@JonRowe We still have this (I didn't remove this line in #2834):

  A shared group is included in another group using any of:
...
  matching metadata            # include the examples in the current context

but this doesn't work:

RSpec.describe do
  shared_examples "conditionally included", :cond do
    it 'runs' do
      expect(true).to be(true)
    end
  end

  context "cond group", :cond do
  end
end
0 examples, 0 failures

PS It works with shared_context_metadata_behavior = :trigger_inclusion though.

@pirj pirj added this to the 4.0 milestone Jan 6, 2021
@pirj
Copy link
Member

pirj commented Jan 6, 2021

There is one more example that confuses me:

RSpec.describe do
  shared_examples "conditionally included", cond: '1' do
    it 'runs' do |example|
      expect(example.metadata[:cond]).to eq('1')
    end
  end

  context "cond group" do
    include_examples 'conditionally included', cond: '2'
    it_behaves_like 'conditionally included', cond: '2'
  end
end

Both of the examples pass. It doesn't seem that the metadata passed to include_examples/it_behaves_like affects example metadata.

@pirj
Copy link
Member

pirj commented Jan 28, 2021

@myronmarston I resort to summoning you to express your opinion on :apply_to_host_groups once more for a tricky case.

@myronmarston
Copy link
Member

@myronmarston I resort to summoning you to express your opinion on :apply_to_host_groups once more for a tricky case.

To clarify, are you referring to this example you posted above?

RSpec.describe do
  shared_examples "conditionally included", cond: '1' do
    it 'runs' do |example|
      expect(example.metadata[:cond]).to eq('1')
    end
  end

  context "cond group" do
    include_examples 'conditionally included', cond: '2'
    it_behaves_like 'conditionally included', cond: '2'
  end
end

Both of the examples pass. It doesn't seem that the metadata passed to include_examples/it_behaves_like affects example metadata.

In this case, cond: '2' being passed to include_examples and it_behaves_like are not treated as metadata and AFAIK args passed to those methods have never been treated as metadata. Instead, they are arguments to the shared example group. You can see that at work here:

RSpec.describe do
  shared_examples "conditionally included", cond: '1' do |cond:|
    it 'runs' do |example|
      expect(example.metadata[:cond]).to eq('1')
      expect(cond).to eq '2'
    end
  end

  context "cond group" do
    include_examples 'conditionally included', cond: '2'
    it_behaves_like 'conditionally included', cond: '2'
  end
end

This passes, because when you pass cond: '2' those args get passed to the shared examples block when it is evaluated. See this cucumber scenario for an example of where we've documented this behavior.

@pirj
Copy link
Member

pirj commented Mar 1, 2021

@myronmarston Please accept my apologies for the very late response. Thanks for getting right to the gist of my confusion. Your explanation helped me to finally wrap my head around the current design and describe it in #2878.

@avit I clearly understand your proposal. Probably metadata is not the best way to approach the problem. I won't let myself to come with more examples and alternative solutions to this. Sorry for all the confusion dealt.

@pirj pirj closed this Mar 1, 2021
@pirj pirj removed this from the 4.0 milestone Mar 1, 2021
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants