Warn if with_them is used before where is defined

[splattael]

This PR emits a warning if with_them is used before where is defined.

See #8

Impact

On this repository we are now seeing 3 warnings:

/home/peter/devel/rspec-parameterized-core/spec/rspec/parameterized/core_spec.rb:265: `where` not defined.
/home/peter/devel/rspec-parameterized-core/spec/rspec/parameterized/core_spec.rb:271: `where` not defined.
/home/peter/devel/rspec-parameterized-core/spec/rspec/parameterized/core_spec.rb:286: `where` not defined.

They relate to

rspec-parameterized-core/spec/rspec/parameterized/core_spec.rb

Lines 264 to 304 in 94ed779

	context "when the where block is after with_them" do
	with_them do
	it "should do additions" do
	expect(a + b).to eq answer
	end
	end
	with_them do
	subject { a }
	it { should be_a Numeric }
	end
	where(:a, :b, :answer) do
	[
	[1 , 2 , 3],
	[5 , 8 , 13],
	[0 , 0 , 0]
	]
	end
	end
	context "when the where block is between with_thems" do
	with_them do
	it "should do additions" do
	expect(a + b).to eq answer
	end
	end
	where(:a, :b, :answer) do
	[
	[1 , 2 , 3],
	[5 , 8 , 13],
	[0 , 0 , 0]
	]
	end
	with_them do
	subject { a }
	it { should be_a Numeric }
	end
	end

which makes sense

However, emitting a warning in those cases would also cover the following case:

describe 'empty' do
  where(:x) { [1] }

  describe 'another scope' do # !!! NOTE THIS EXTRA CONTEXT!!
    with_them do
      it 'works' do

      end
    end
  end
end

# 0 examples, 0 failures

Full output

$ bundle exec rspec -f p --dry-run
warning: parser/current is loading parser/ruby30, which recognizes 3.0.6-compliant syntax, but you are running 3.0.5.
Please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
/home/peter/devel/rspec-parameterized-core/spec/rspec/parameterized/core_spec.rb:265: `where` not defined.
/home/peter/devel/rspec-parameterized-core/spec/rspec/parameterized/core_spec.rb:271: `where` not defined.
/home/peter/devel/rspec-parameterized-core/spec/rspec/parameterized/core_spec.rb:286: `where` not defined.
Run options: include {:current=>true}

All examples were filtered out; ignoring {:current=>true}
......***..................................................................................

Pending: (Failures listed here are expected and do not affect your suite's status)

  1) RSpec::Parameterized where and with_them a: 1, b: 2, answer: 3 should do additions
     # PENDING
     # ./spec/rspec/parameterized/core_spec.rb:34

  2) RSpec::Parameterized where and with_them a: 5, b: 8, answer: 13 should do additions
     # PENDING
     # ./spec/rspec/parameterized/core_spec.rb:34

  3) RSpec::Parameterized where and with_them a: 0, b: 0, answer: 0 should do additions
     # PENDING
     # ./spec/rspec/parameterized/core_spec.rb:34


Deprecation Warnings:

RSpec::Core::Configuration#treat_symbols_as_metadata_keys_with_true_values= is deprecated, it is now set to true as default and setting it to false has no effect.


If you need more of the backtrace for any of these deprecations to
identify where to make the necessary changes, you can configure
`config.raise_errors_for_deprecations!`, and it will turn the
deprecation warnings into errors, giving you the full backtrace.

1 deprecation warning total

Finished in 0.00623 seconds (files took 0.15319 seconds to load)
91 examples, 0 failures, 3 pending

[splattael]

Marking as "Draft" for now as this idea and the implementation has to be tested at GitLab.

[splattael]

where can be called without a block so the source location would be missing.

Edit: Perhaps use caller in those cases or always use caller? 🤔

We'd need to store caller in @parameter as well then 🤷

[splattael]

On the other hand, redefining where is OKish and used at GitLab like this:

context "foo" do
  where(...)
  with_them do
    it 'allow' do
    end
  end

  where(...)
  with_them do
    it 'disallow' do
    end
  end

Removing this warning again.

[splattael]

In https://gitlab.com/gitlab-org/gitlab/-/issues/430870#note_1637281194 I found some cases where where is defined in a parent example group and with_them is used in a nested context assuming that where is shared:

describe 'foo' do
  where(foo: [1, 2, 3])

  describe 'bar' do
    with_them do
      it('works') {}
    end
  end

  describe 'bar2' do
    with_them do
      it('works') {}
    end
  end
end

This ☝️ gives 0 examples, 0 failures.

[splattael]

Not sure if with_them makes sense to be called without a block 🤷

[splattael]

Happy to add specs once the concept is accepted.

[splattael]

👋 @sue445 Please excuse the noise but I've made this PR ready for review again 😅

Do you mind checking? 🙏

[sue445]

@splattael Thank you for patch. I understood your issue.

However, in rspec the behavior generally does not change depending on the order in which blocks are defined, so this change (even if it is a warning) would not be in line with rspec behavior.

For example, the following two both have the same definition and the same behavior.

describe "sum" do
  subject { a + b }

  let(:a) { 1 }
  let(:b) { 2 }

  it { should eq 3 }
end

describe "sum" do
  it { should eq 3 }

  let(:b) { 2 }
  let(:a) { 1 }

  subject { a + b }
end

Therefore, I think we should only treat it as a warning when where is not defined in same scope as with_them.

[splattael]

Therefore, I think we should only treat it as a warning when where is not defined in same scope as with_them.

I agree 👍

I was wondering how we can achieve this 🤷 Currently, where and with_them are not lazy so I am not sure how to let with_them know if there's a where following it. For example:

describe 'foo' do
  with_them do # How do we know that `where` will follow in the same scope?
    ...
  end

  where(...)
end