Rubocop-rspec: RSpec/RepeatedDescription should not fire when using rspec-its

[obfuscoder]

We are using rspec-its and after updating rubocop-rspec experienced a lot of reports for this rule violation.

We often use contexts and check the resulting objects for specific attribute values like so:

describe 'foo' do
  subject { my_subject }
  context 'bar' do
    its(:attrib1) { is_expected.to eq 'value1' }
    its(:attrib2) { is_expected.to eq 'value2' }
  end
end

rubocop-rspec complaints about the two its-statements sharing the same description.

We do not expect this rule to pick up those autogenerated descriptions.

(EDIT: I updated this text to better reflect the problem. Please note that the comment below by @pirj is referring to the previous content of this description which was not specific enough.)

[pirj]

That's weird, I can't reproduce on master.
Can you please provide a reproducible example?

[georgebancila]

  describe '#sample' do
    subject(:operation) { OpenStruct.new(test_name: 'Test sample', attribute: 23) }

    its(:test_name) { is_expected.to eq 'Test sample' }
    its(:attribute) { is_expected.to eq 23 }
  end

this code raises these problems

spec/sample.rb:36:5: C: Don't repeat descriptions within an example group.
    its(:test_name) { is_expected.to eq 'Test sample' }
    ^^^^^^^^^^^^^^^
spec/sample.rb:37:5: C: Don't repeat descriptions within an example group.
    its(:attribute) { is_expected.to eq 23 }

[pirj]

@lazycoder9 Yet another interesting case to look at.

[obfuscoder]

Ok I misinterpreted the violation report. I originally assumed the violation was complaining about the same content of the its term. Instead it complained because there was another its term above it:

        its(:year) { is_expected.to eq 1980 }
        its(:month) { is_expected.to eq 1 }
        its(:day) { is_expected.to eq 1 }

And this is what @georgebancila showed as well.

[lazycoder9]

@pirj @obfuscoder @georgebancila just tried to reproduce your issues locally and there were no offenses on your code samples. Seems last PRs (#877 and #881) fixed it. I think this issue can be closed

[pirj]

Confirming, the issue went away:

$ rubocop --only RSpec/RepeatedDescription spec/a_spec.rb                
Inspecting 1 file
C

Offenses:

spec/a_spec.rb:3:9: C: RSpec/RepeatedDescription: Don't repeat descriptions within an example group.
        its(:year) { is_expected.to eq 1980 }
        ^^^^^^^^^^
spec/a_spec.rb:4:9: C: RSpec/RepeatedDescription: Don't repeat descriptions within an example group.
        its(:month) { is_expected.to eq 1 }
        ^^^^^^^^^^^
spec/a_spec.rb:5:9: C: RSpec/RepeatedDescription: Don't repeat descriptions within an example group.
        its(:day) { is_expected.to eq 1 }
        ^^^^^^^^^

1 file inspected, 3 offenses detected
pirj@air ~/source/rubocop-rspec (master ) $ g up
remote: Enumerating objects: 46, done.
remote: Counting objects: 100% (46/46), done.
remote: Compressing objects: 100% (23/23), done.
remote: Total 46 (delta 25), reused 30 (delta 21), pack-reused 0
Unpacking objects: 100% (46/46), done.
From github.com:rubocop-hq/rubocop-rspec
   9bce008..94d8084  master     -> origin/master
 * [new tag]         v1.38.1    -> v1.38.1
First, rewinding head to replay your work on top of it...
Fast-forwarded master to 94d8084d91b274f26355184366be312e6751ff07.
pirj@air ~/source/rubocop-rspec (master ) $ rubocop --only RSpec/RepeatedDescription spec/a_spec.rb 
Inspecting 1 file
.

1 file inspected, no offenses detected

[pirj]

However, it might be just a coincidence. Now, we consider the first argument to its a docstring:

def_node_matcher :extract_doc_string,     '(send _ _ $_ ...)'

So RepeatedExample ignores those as it thinks their descriptions are different.

Not sure if there might be some problems with that going forward, but at least it's confusing.

Since RuboCop::RSpec::Example is used to wrap its examples as well, I'd love to have some examples for it in example_spec, and in the best base fix metadata vs docstring vs its primary argument. A source for inspiration and a brief list of edge cases of its usage.

[pirj]

Actually, its is a special case, and it doesn't support docstrings.
Is there a good reason not to treat its primary argument as a docstring?
In this case, no lib code changes are necessary. Maybe a couple of examples showing how docstring (primary argument) and meta are extracted by RuboCop::RSpec::Example.

The only corner case (sorry for a very contrived example) I can think of:

its(:count) { is_expected.to be 3 }
it :count do
^^^^^^^^^^^^ offence, however, it might be a different example

Doesn't seem to be a big deal though.

[Kukunin]

Happens to me as well on the latest version:

        its(:first) { is_expected.to eq find("#channels_publisher_#{publisher.id}") }
        its(:first) { is_expected.to have_content article_0.created_at.to_formatted_s(:long) }

I believe, that this case shouldn't be fired.

[pirj]

@lazycoder9 Would you please take a look?

[lazycoder9]

@pirj sure, I was a little busy at work, but I believe that I can handle this in a week

[lazycoder9]

@Kukunin why do you think that this case shouldn't be fired?

its(:first) { is_expected.to eq find("#channels_publisher_#{publisher.id}") }
its(:first) { is_expected.to have_content article_0.created_at.to_formatted_s(:long) }

I see similar descriptions for its statements. Can you clarify your point of view?

[lazycoder9]

@pirj The only thing that I understand that we should distinguish it and its statements with the same descriptions, am I right?

[Kukunin]

@lazycoder9 because those are two different tests.

yeah, from a formal side, they use the same description. Actually, that's a known trade-off and is the main reason why its was extracted to a separate gem. Using its makes output documentation worse. People do that decision to lose in descriptiveness to keep tests concise.

This cop is still useful, in my project I found a couple of silly typos and duplicated tests because of it. So I don't want to disable it completely, just because I made the decision to use its. It'd be great if there is a configuration option to ignore its.

[pirj]

@Kukunin do you think the same can be written slightly differently:

describe '#first' do
  subject(:paragraph) { page.paragraphs.first }

  it { is_expected.to eq find("#channels_publisher_#{publisher.id}") }
  it { is_expected.to have_content article_0.created_at.to_formatted_s(:long) }

I miss some context regarding what's being under test, so had to resort to guessing.

[pirj]

And yet another way to achieve the same (considering you have aggregate_failures enabled:

its(:first) do
  is_expected.to eq find("#channels_publisher_#{publisher.id}")
  is_expected.to have_content article_0.created_at.to_formatted_s(:long)
end

or even without aggregate_failures:

its(:first) do
  is_expected
    .to eq find("#channels_publisher_#{publisher.id}")
    .and have_content article_0.created_at.to_formatted_s(:long)
end

@lazycoder9 I don't personally think there's anything that needs urgent attention or is a flaw of the existing cop. Sorry for summoning you without taking a proper look.

@Kukunin Would you like to tackle adding this IgnoreItsWithSameAttribute option and add a couple more specs to better cover how examples defined with its is being flagged/ignored?

[pirj]

@Kukunin please feel free to send a draft pull request, I'll provide any necessary guidance to help you along your way.
Plan:

• check how cops are configured with options, add IgnoreItsWithSameAttribute
• add something like conditional reject to filter out/collapse its with same attributes from RuboCop::RSpec::ExampleGroup.new(node).examples
• specs to cover that

Closing this issue, since the original issue its(:attrib1)/its(:attrib2) was resolved.