rubocop-rspec: RSpec/NamedSubject: false positive when raising error

I would agree with @bquorning that in general I expect subject to be an instance of described_class. As @mikegee suggests you could potentially have a #call call in each example or something like that. I tend to think this is more clear and helpful.

On some occasions when i’m testing a very imperative-feeling service object with a lot of setup, i might do something like this:

def perform_action
  described_class.new(...).perform
end

it 'explodes' do
  expect { perform_action }.to raise_error(FooError)
end

I find it personally a little odd to have a memoized subject helper represent a result or an action most of the time. If you’re testing a side effect through subject that calling subject twice wouldn’t cause the behavior twice (since it is memoized) so sometimes a plain old method feels more natural to me.

That said, I think this is all a very grey area of testing. If you write a lot of tests that leave you fighting with this cop, disable it. Or disable it for a particular file. I think the expect block exception is probably too specific of a case to merit a special configuration option here, but I could possibly be convinced otherwise.

On a related note, I don’t use the implicit expect syntax at all because I prefer the expanded/explicit form but there are some testing situations where i could be convinced that it fits well.

This is a subjective cop and if you disagree with it that’s ok, but i wouldn’t consider this a false positive: it’s doing its job and asking you to use a descriptive subject name any time you want to explicitly reference your subject.

The RSpec cops live in the rubocop-rspec project. Please open up an issue in that project.

Since opening this ticket I’ve been experimenting with naming and not naming subjects, my personal feeling is that I would rather not name it and use subject as my safe, go to name. For example, say I am writing tests for NameImporterService, my subject is described_class.new(dependency: dependency).call, should I name this subject result, call_result, names_count?

In my opinion the extra clarity that a named subject could give in this situation defeats the mental energy required to think about a good name for it.

Nonetheless I understand how other people would like this rule and I have the option to disable it, so win/win for everyone. That’s the beauty of Rubocop, it’s extremely configurable, the cops have good SEOs and most of them have clear explanation why one style is preferred over another. ❤️

I would most often expect subject to be an instance of described_class; in your examples an instance of User.

Following that pattern, calling subject shouldn’t have side effects, except perhaps storing a record in a database.

I would recommend rewriting to e.g.:

RSpec.describe User do
  it { expect { User.find!(404) }.to raise(ActiveRecord::RecordNotFound) }
end

Yes, this works as expected, I am just not sure what do I get from naming the subject in this case, it’s a short 2 line spec, it’s clear and no attributes are being read from the subject, for example:

RSpec.describe User do
  subject { User.find!(404) }

  it { expect { subject }.to raise(ActiveRecord::RecordNotFound) }
end

vs

RSpec.describe User do
  subject(:user_find) { User.find!(404) }

  it { expect { user_find }.to raise(ActiveRecord::RecordNotFound) }
end

Is mentioning subject as a named variable a code smell or bad pattern in all scenarios?