[NestedGroups] Reset nesting to 1 on describe block

[Krystosterone]

PR that introduced the cop: #156
Issue that spawned the idea of the cop: #177

I recently updated to the newest rubocop-rspec and ran into a problem of rapidly triggering this cop when having a nested describe for my methods. For example, it would trigger:

describe MyClass do
  describe '#foobar' do
    context 'when boop' do
    ^^^^^^^^^^^^^^^^^^^ Maximum example group nesting exceeded
    end
  end
end

which I don't think is a legitimate case.

@mockdeep 's original comment:

It's generally a code smell when the nesting has to go deeper than 1

While I agree that nesting should be limited, I also think that it should only start counting at the very nearest describe block (but I might be mistaken).

[backus]

@Krystosterone Thanks for the PR! I have some questions / concerns about this approach.

Why should it reset at every describe block? It seems wrong to me that this cop now, with the default configuration, does not flag this:

RSpec.describe MyClass do
  describe '1' do
    describe '2' do
      describe '3' do
        describe '4' do
          describe '5' do
            context '6' do
              describe '7' do
              end
            end
          end
        end
      end
    end
  end
end

Have you considered either of the following options?

1. Use the describe method construct in RSpec that doesn't require another level of nesting:

RSpec.describe MyClass, '#foobar' do
  context 'when boop' do # this is now within the allowed levels of nesting!
  end
end

1. Bump your MaxNesting configuration to 3. If you don't like describe TheClass, '.the_method' syntax then bumping your config to three will certainly fit your use case.

[backus]

@mockdeep would you like to weigh in here?

[mockdeep]

I've definitely done the 3 level approach that @Krystosterone mentions before. It makes sense if you're testing all of your methods in the same file:

describe MyClass do
  describe '#my_one_method' do
    context 'when stuff' do
    end
  end

  describe '#my_other_method' do
    context 'when something' do
    end
  end
end

I'm not sure what the best approach for rubocop-rspec is in this regard. It seems like bumping the nesting configuration up to 3 should do the trick, though.

[backus]

If people use --auto-gen-config then it will automatically bump their config up for them. I realize that many users wont know all of the features that rubocop-rspec provides (my hands are sort of tied here too since this is an extension of rubocop and I can't add flags or runner behavior). I'd be open to bumping the max nesting to three potentially. I think what I would like most is something like this:

RSpec.describe Foo do
  describe '#bar' do
    context 'when doing something' do # does not register a violation
    end
  end
end

RSpec.describe Foo do
  describe 'when I do not describe a method' do
    context 'when doing something' do # registers a violation
    end
  end
end

RSpec.describe Foo do
  describe '#bar' do
    describe '#baz' do # registers a violation
    end
  end
end

RSpec.describe Foo do
  describe '#bar' do
    describe 'invalid usage of describe' do # registers a violation
    end
  end
end

[waiting-for-dev]

In my case, 3 is also usually the accepted nesting, and not only when I am describing pure ruby methods, but also in feature tests, where # convention is not so broadly used. Something like:

describe 'Users request' do
  describe 'POST sign_in' do
    context 'when credentials are valid' do
      # ...
    end

    context 'when credentials are not valid' do
      # ...
    end
  end
end

If we have to rely on convention (or style), I think it is safer to just accept two describe before a context. Or if you prefer one Rspec.describe and a describe before a context, but I'm not sure I would force it (I would like to study pro and cons).

Cheers

[PanisSupraOmnia]

Just wanted to give this a bump; it's not too huge but I do have a few specs that trigger this cop in a way that seems unfair, including one that's almost exactly like @waiting-for-dev's example. For my specs, the best way for it to go would be allowing Rspec.describe and a describe before the context, but I'm not sure how well that might work for others.

[backus]

@waiting-for-dev @ChallahuAkbar a separate issue with NestedGroups (#270) made me revisit the cop and I'm planning on bumping the default to allow three levels of nesting. Can you take a look at #271 and maybe try it out and let me know what you think?

[waiting-for-dev]

@backus it looks good for me. I also think that 3 should be the default. Thanks!

[backus]

Never heard from you @ChallahuAkbar. Would still be interested in whether #271 satisfied your problem. I'm going to close this PR for now since we are trying to keep the open issue count lower

[PanisSupraOmnia]

Whoops, yeah, I'm happy with the way the behavior is now.