Why does the RSpec/SubjectStub cop exist?

[DannyBen]

Can anyone help me understand the logic behind the RSpec/SubjectStub cop?

It disallows stubbing of inner methods of the subject under test, but to me this seems like a legitimate, intuitive and readable approach. I do not see the downside, and any alternative I can think of would require more coding and less readability.

Consider this test subject:

# stock.rb
class Stock
  def price
    raise "Invalid data" unless valid?
    12.3
  end

  def valid?
    # some logic
    true
  end
end

and this spec:

# stock_spec.rb
require './stock'

describe Stock do
  subject(:stock) { described_class.new }

  describe '#price' do
    it 'returns the price' do
      expect(stock.price).to eq 12.3
    end

    context 'when it is invalid' do
      before { allow(stock).to receive(:valid?).and_return false }

      it 'raises an error' do
        expect { stock.price }.to raise_error('Invalid data')
      end
    end
  end
end

running rubocop --only RSpec will trigger this offense:

RSpec/SubjectStub: Do not stub methods of the object under test

why is this bad? and what is an acceptable alternative?

[pirj]

It exists to back this guideline, originated here.

Speaking of the example itself, I see an option to avoid stubbing by passing invalid values to the object initializer (e.g. new(-1)). This, though, would increase the testing surface to valid?, that is most probably covered in other examples.
On the other hand, this would prevent from relying on the implementation details.

[pirj]

one liner syntax is discouraged

Is it?

subject "construction pattern"

Another way that I believe Effective Testing with RSpec suggests is to use methods building the subject:

def stock_for(id)
  described_class.new(id)
end

it { expect(stock_for(:SPY)).to ... }

But with more than 3-4 inputs to the constructor, it can quickly get out of control, too.

[DannyBen]

is it?

From what I see, absolutely.
I could not find the posts where I actually saw it first, but a quick google search shows some links where it is discussed negatively:

• Implicit block expectation syntax rspec-style-guide#76
• https://blog.rubystyle.guide/rspec/2019/07/17/rspec-implicit-block-syntax.html
• https://www.reddit.com/r/ruby/comments/cejl3q/call_for_discussion_rspec_implicit_block/?st=jy8s65z0&sh=0e5bb1da
• is_expected with block expectations rspec/rspec-expectations#805

Notice this comment in particular: rspec/rspec-expectations#805 (comment)

Q: Is one-liner syntax something that we (as a core team) use much, or is it in the expect_any_instance_of bucket of extras?
A: It's in the same bucket, I certainly don't recommend it.

Furthermore, I am using block expectations quite a lot for output and error checking using my rspec_approvals interactive approval matchers, and is_expected cannot be used with block matchers, which will mean I will end up with a mixed bag of rspec syntaxes.

Another hint, is that the its(method) { is_expected.to ... } was removed.

I wish they were both alive, but I can understand the reasoning.

Another way that I believe Effective Testing with RSpec suggests is to use methods building the subject:

The top appealing rspec feature for me, is that it is a readable DSL.
It is a priority for me that the specs are readable, and easy to write, and utilize the DSL aspects that make it read like English.
Otherwise, I would have used assertions with minitest or something... :)

I want: expect(something).to ... with as little fluff in between as possible.

I avoid def in specs at all costs, and for the same reason I even avoid factories and complicated stubbing. So far this approach has served me quite well - I feel my tests are efficient and effective, but maybe I am just used to them.

[pirj]

one liner syntax is discouraged

Those links are all about the implicit block expectation syntax, not specifically one-liner expectation syntax.
The fact that the core team doesn't use it doesn't mean that everyone else should not, too.
As far as I'm aware, is_expected is not going anywhere.

its(method) { is_expected.to ... } was removed

Was it?

Anyway, it feels that the discussion seems to have drifted.
Do you still have some concerns regarding stubbing the subject?

[DannyBen]

Those links are all about the implicit block expectation syntax, not specifically one-liner expectation syntax.

How else can is_expected be used other than in it { is_expected }?
I never saw it in any other appearance.

Was it?

I must have misread this:

its for RSpec 3 extracted from rspec-core 2.x

as a sign that it is extracted since the core team do not want to mess with it, but still want to provide this functionality to anyone who needs it. It feels like shoulda matchers that are now replaced with expectations. I may be wrong, but I definitely got the feeling its is just something the rspec team agreed to create, but not encourage.

Add to the fact that the one liner syntax page does not mention its, even though it clearly belongs there and you get the overall "discouraged" smell. At least I do.

Anyway, it feels that the discussion seems to have drifted.
Do you still have some concerns regarding stubbing the subject?

Just enjoying the discussion. No, I feel I understand the reasoning enough to make an educated choice as to disabling this and other cops when I need to.

Thanks.

[pirj]

implicit block expectation syntax

There's some confusion here.

is_expected is defined as expect(subject).to .... It is a value expectation syntax. They can be used interchangeably, and there are no arguments against is_expected. There are, though, arguments against using a literal subject. Subject should be named, and not referenced directly as subject. expect(stock).to is more intent-revealing than expect(subject), as it doesn't require to look up what the subject is from the reader.
I'd say is_expected is preferred for one-liner expectations, where subject is defined somewhere close to those grouped one-liners.

In contrary, the block expectation syntax is expect { some_method }.to ....

Those two syntaxes are different in a sense that the former is testing the returned value, while the latter is testing side effects.
Matchers usually only support one of them. E.g. expect { puts "hi" }.to eq(true) doesn't make sense.

Theoretically, expect(stock).to change(rate).to(1) is not supposed to work.
But it did, if stock was defined as a lambda, let(:stock) { -> { rate.increment! } }. This is what we call the implicit block expectation syntax. Something that doesn't look like a block but behaves like one.
It is now deprecated and will cease to work in RSpec 4.

is_expected will remain intact.

[pirj]

its for RSpec 3 extracted from rspec-core 2.x
as a sign that it is extracted since the core team do not want to mess with it

It's a personal preference, but I don't use its, as there are other ways to describe the same behaviour:

subject(:number) { 3 }

its(:odd?) { is_expected.to be true }
it { expect(number).to be_odd }
it { is_expected.to be_odd }

There are even cases where I can't tell if its has a direct substitute:

it { expect(number.allbits?(1)).to be true }
it { is_expected.to be_allbits(1) }

[DannyBen]

Thanks for both messages. I didn't see anybody use the one liner syntax like this:

it { expect(number).to be_odd }

it does not read right. I guess this is the place to use specify?

specify { expect(number).to be_odd }

[pirj]

Fair enough. I don't have a definitive answer, and there's no written down agreement. Related style guide issue. Your example reads like a correct English sentence.
The only hint is from the code defining specify:

      # Useful for when your docstring does not read well off of `it`.
      # @example
      #  RSpec.describe MyClass do
      #    specify "#do_something is deprecated" do
      #      # ...
      #    end
      #  end
      define_example_method :specify

even though, personally, I'd use it here:

      #  RSpec.describe MyClass do
      #    describe "#do_something" do
      #      it "is deprecated" do
      #        # ...
      #      end
      #    end
      #  end

and would reserve specify for examples with no implicit subject (is_expected) and no docstring, both multi-line and one-line.

specify do
  expect(stock).to be_on_the_rise
end
it { is_expected.to be_on_the_rise }
specify { expect(input.split).to include('A', 'b') }

[DannyBen]

Yup. Seems right.
Thanks for the nice discussion and tips.

[pirj]

Likewise, @DannyBen !