Cop idea: ExampleStatementsCount

[zverok]

In my experience, RSpec/ExampleLength is somewhat useful cop which, though, frequently misses a point: it constantly flag statements like this (especially in some JSON API testing):

it {
  is_expected.to match(
    # ....40 lines of nested response specification...
  )
}

I argue that for testing deeply nested structures the code above is the most useful way of writing it :)
When rubocop-rspec flags it, people just move the matcher part into lets, making readability and debuggability worse, not better.
I thought that maybe what we really want is to limit the number of top-level statements inside an example.
It is not the same as limiting the number of expectations -- the statements might prepare some data before expecting (which, in my book, is a sin on itself, so I'll probably set ExampleStatementsCount strictly to 1... But that's just me :)

[pirj]

Agree. Even for a non-nested structure, e.g.:

it_expected.to have_attributes(
  a: 1,
  b: 2,
  c: 3,
  d: 4,
  e: 5
)

move the matcher part into lets

Extracting a part of the matcher into a let is an offence as well to my taste, and is an idea for a cop as well.

Do you think setting

# spec/.rubocop.yml
Metrics/BlockLength:
  IgnoredMethods:
    - describe
    - context
   CountAsOne:
     - hash
     - array

would do?

[zverok]

How's Metrics/BlockLength related here? 🤔
If you've meant that ExampleLength should have similar settings, I don't believe it will work... nested matchers with hash_including(...) or contain_exactly(...), chains of .and ..., have_attributes(...) and so on.
I believe that just literal counting of top-level statement in example block is 100% enough. If this one statement uses 300-line data structure to compare -- let it be.

[pirj]

Metrics/BlockLength's Max is 25, but you're right, its folding (CountAsOne) capabilities are quite limited.

Another option would be RSpec/MultipleExpectations with e.g. Max: 5 (or 1).
Since it's possible to define rules on a directory basis, ExampleLength could be disabled for such specs, and MultipleExpectations enabled.
Do you think it makes sense to count other statements towards the limit as well?

[zverok]

Do you think it makes sense to count other statements towards the limit as well?

I do! But YMMV. In my book, this is bad:

it 'does blah blah' do
  create(:something)
  create(:something_else)

  get(:foo)

  body = last_response.body
  data = JSON.parse(body)

  expect(data).to now_we_are_talking
end

So, ideally, it should be exactly one statement always :) One exception is:

it 'sends its messages' do
  do_something
  expect(something).to have_received(:something)
end

I solve it in saharspec with

its_block { is_expected.to send_message(something, :something) }

...but that's obviously not for everybody.

And actually, my "bad" example is a "typical" example in tutorials, and even on the main of https://rspec.info/ 🤷

[Darhazer]

Related: #249

also we 💯 should add support for CountAsOne in ExampleLength

[Darhazer]

Since ExampleLength supports the CountAsOne option, perhaps we can close this?

[pirj]

Fixed in #1139