Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autogenconfig doesn't exclude RSpec/NestedGroups #7484

Open
luke-hill opened this issue Nov 6, 2019 · 11 comments
Open

Autogenconfig doesn't exclude RSpec/NestedGroups #7484

luke-hill opened this issue Nov 6, 2019 · 11 comments

Comments

@luke-hill
Copy link

I've added this here as it feels like a core rubocop issue (not from rubocop/rspec)

When you have both a manually defined config and an autogenerated config for RSpec/NestedGroups - rubocop will always fail


Expected/Actual behavior

  • Create a codebase, with some rspec tests which have 5 nestings
  • Set .rubocop.yml to
RSpec/NestedGroups:
  Max: 4
  • Run rubocop --auto-gen-config
  • Observe the todo file contains
RSpec/NestedGroups:
  Max: 5
  • run rubocop
  • You will get failures for (I've added my OSS codebase example here)
spec/site_prism/page_spec.rb:187:9: C: RSpec/NestedGroups: Maximum example group nesting exceeded [5/4].
        context 'when validations are disabled' do
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/site_prism/page_spec.rb:206:9: C: RSpec/NestedGroups: Maximum example group nesting exceeded [5/4].
        context 'when validations are disabled' do
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Steps to reproduce the problem

You can clone the codebase at https://github.com/site-prism/site_prism Then add a manual value of 4 to the .rubocop.yml file

RuboCop version

Include the output of rubocop -V or bundle exec rubocop -V if using Bundler. Here's an example:

$ [bundle exec] rubocop -V
0.73.0 (using Parser 2.6.3.0, running on ruby 2.3.8 x86_64-linux)
@koic
Copy link
Member

koic commented Nov 6, 2019

RSpec/NestedGroups cop is a cop that RuboCop RSpec has. I'm going to transfer the issue to rubocop-rspec repo.

@koic koic transferred this issue from rubocop/rubocop Nov 6, 2019
@pirj
Copy link
Member

pirj commented Nov 6, 2019

@koic I believe the NestedGroups is only used as an example, and the problem is not related to RuboCop RSpec. It would be easy to reproduce the same issue with, say, ModuleLength.

As far as I understand, if there's a setting in .rubocop.yml and there are offences in the codebase when a .rubocop_todo.yml gets generated with a more permissive setting, it's getting overridden by a more strict setting in .rubocop.yml.

I'm not sure if it's a problem with --auto-gen-config, and it should make sure the settings from .rubocop_todo.yml won't be overridden, or there should be a warning message given when a setting is overridden (very unlikely, since it's a common use case), or settings from .rubocop_todo.yml should take priority somehow.

@luke-hill
Copy link
Author

Also, I think it's with the way the auto-gen works.

When auto-gen has excludes, they're ignored fine. And can be combined with a rule override.

When auto-gen has a rule override, and you provide a more recessive rule override then the issue occurs.

i.e.

# todo
CopOne/Thing
  Exclude:
    - file/a
    - file/b

combines with

# yml
CopOne/Thing
  Max: 4

BUT

# todo
CopOne/Thing
  Max: 8

DOES NOT combine with

# yml
CopOne/Thing
  Max: 4

@koic
Copy link
Member

koic commented Nov 6, 2019

@pirj Oops. Thanks for your explanation.

@koic koic transferred this issue from rubocop/rubocop-rspec Nov 6, 2019
@stale
Copy link

stale bot commented May 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale Issues that haven't been active in a while label May 4, 2020
@stale
Copy link

stale bot commented Aug 2, 2020

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

@stale stale bot closed this as completed Aug 2, 2020
@marcandre marcandre added bug help wanted and removed stale Issues that haven't been active in a while labels Aug 2, 2020
@marcandre marcandre reopened this Aug 2, 2020
@maxshort
Copy link

maxshort commented Aug 3, 2020

I think this is "working as intended" though confusing for the auto-gen case.

.rubocop.yml inherits from .rubocop_todo.yml so .rubocop.yml will always have the final say.

Some quick (breaking) changes that could fix this:

  • flip the order of inheritance so rubocop_todo.yml inherits from .rubocop.yml.
  • Put overrides in the .rubocop.yml file itself

I think the most robust/user-friendly fix would be to separate _todo from inheritance.

@marcandre
Copy link
Contributor

Oh, right, good point!

We could introduce override_with: path that would do a reverse merge. It would be good for todo, and inherit_from would still be great for defaults.

@stale
Copy link

stale bot commented Feb 2, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale Issues that haven't been active in a while label Feb 2, 2021
@luke-hill
Copy link
Author

luke-hill commented Feb 2, 2021

The issue isn't the inheritance order per sé. But the way in which the auto gen is being generated.

In my eyes, auto gen means the following.

I don't care what the failures are. Make my damn suite pass! I'll fix stuff up later.

not ....

Please try to fix things up, and if it doesn't work. Try again maybe?

Maybe for auto-gen instead of making a "numerical" value which won't work due to the reasons specified above. It just disables said cop. Or provide a massive exclude list as I illustrated at the top. I'm indifferent to "what" the solution is, because I guess when running auto-gen, I'm just in a "Get this thing fixed in 10seconds" mood. Not a "How do I fix this?" mood.

@stale stale bot removed the stale Issues that haven't been active in a while label Feb 2, 2021
@luke-hill
Copy link
Author

Note to self. Re-test this with rubocop 1.60.0+ - I believe it's fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants