rubocop-rspec: `RSpec/Rails/AvoidSetupHook` consider `minitest` setup hook as problem

Hi there, I got rails project with a bunch of legacy code, and for some forgotten reason it uses both rspec and minitest tests

If I run rubocop with auto-fix in root folder - it will consider this as a problem and fix it and broke minitest file

rubocop test/controllers/clients_controller_test.rb
Inspecting 1 file
C

Offenses:

test/controllers/clients_controller_test.rb:6:3: C: [Correctable] RSpec/Rails/AvoidSetupHook: Use before instead of setup.
  setup do ...
  ^^^^^^^^

1 file inspected, 1 offense detected, 1 offense auto-correctable

And after applying fix I got:

rake test
rake aborted!
NoMethodError: undefined method `before' for ClientsControllerTest:Class

I think this cop should ignore files containg minitest tests, maybe by detecting class ClientsControllerTest < ActionController::TestCase or something like this

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Comments: 15 (14 by maintainers)

Commits related to this issue

Most upvoted comments

@pirj Actually I’m not very familar with Rubocop internal mechanics and config default format, I usually only report bugs )

So I’m little bit worried that I may broke something and not sure I’ll be able to write a correct test suite

This problem is very similar to rubocop/rubocop#9889 and we were able to resolve that problem, but I failed to realize that the solution should not be in @dmke’s local configuration but in the configuration of rubocop-rspec.

The crux of the matter is that configuration for the RSpec department isn’t automatically inherited down to subdepartments like RSpec/Rails. I’m not sure if we could change RuboCop so that kind of inheritance takes place. The other solution would be to add

RSpec/Rails:
  Include:
    - "**/*_spec.rb"
    - "**/spec/**/*"

in config/default.yml of rubocop-rspec.

TIL bundle add ...

That would be helpful, thanks!

@pirj Thanks for your investigation

I can simplify my repo to minimal reproducible case if this needed by @koic or @bbatsov, because right now project is rather complicated

@ShockwaveNN Ha! The documentation is correct and corresponds to the default configuration. I can’t remember seeing rubocop-rspec inspecting outside */spec/, and if it does, it’s either a misconfiguration, or a regression in rubocop.

I can reproduce the issue with the repository you provided with RuboCop 1.16.1, though. Can’t see anything suspicious in your configuration.

The problem seems to be somewhere here:

# rubocop-1.16.1/lib/rubocop/cop/team.rb:83
on_duty = roundup_relevant_cops(processed_source.file_path)

and deeper inside:

# rubocop-1.16.1/lib/rubocop/cop/team.rb:158
def roundup_relevant_cops(filename)
  cops.reject do |cop|
    cop.excluded_file?(filename)
# rubocop-1.16.1/lib/rubocop/cop/base.rb:229:
def excluded_file?(file)
  !relevant_file?(file)
end
# rubocop-1.16.1/lib/rubocop/cop/base.rb:223
def relevant_file?(file)
  file == RuboCop::AST::ProcessedSource::STRING_SOURCE_NAME ||
    file_name_matches_any?(file, 'Include', true) &&
      !file_name_matches_any?(file, 'Exclude', false)
end
> on_duty.first.name
=> "RSpec/Rails/AvoidSetupHook"
> on_duty.first.relevant_file?(processed_source.file_path)
=> true
> on_duty.first.__send__(:file_name_matches_any?, processed_source.file_path, 'Include', true)
=> true
> processed_source.file_path
=> "/home/pirj/source/tmp/testing-wrata/Gemfile"

@koic @bbatsov Can you please transfer this issue to rubocop? Wondering if there were some changes made that could affect how Include merging between cops’s own config and AllCops/Include? As far as I remember, cops’ and extensions’ configs should override the default broad list of Includes according to https://docs.rubocop.org/rubocop/1.9/configuration.html#unusual-files-that-would-not-be-included-by-default.