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

Consider removing rubocop-minitest dependency #13

Closed
igor-alexandrov opened this issue Jan 6, 2024 · 4 comments · Fixed by #18
Closed

Consider removing rubocop-minitest dependency #13

igor-alexandrov opened this issue Jan 6, 2024 · 4 comments · Fixed by #18

Comments

@igor-alexandrov
Copy link

I am not quite sure that having rubocop-minitest as a dependency is correct. On one side – yes, Minitest is a default testing framework in Rails; on the other side, a lot of Rails apps use RSpec, and having rubocop-minitest in the dependency graph will look quite strange.

What do you think?

@igor-alexandrov igor-alexandrov changed the title Remove rubocop-minitest dependency Consider removing rubocop-minitest dependency Jan 10, 2024
@vietqhoang
Copy link

Related to this issue, how does one require rubocop-rspec when inheriting the omakase's rubocop config?

Here is what I have below:

require:
  - rubocop-rspec

inherit_gem:
  rubocop-rails-omakase: rubocop.yml

I can't get rubocop-rspec to run when using rubocop. I tried swapping the positions of the require and inherit_gem with no success. This is on a clean Rails 7.1.3.2 install with rubocop-rails-omakase 1.0.0 and rubocop-rspec 2.26.1.

After removing the inherit gem I can get rubocop-rspec to run and catch violations.

My guess is there is conflict with the require in my app's config and the require present in omakase's config.

https://github.com/rails/rubocop-rails-omakase/blob/main/rubocop.yml#L1-L4

Can anyone steer me in the right direction?

@igor-alexandrov
Copy link
Author

igor-alexandrov commented Feb 23, 2024

@vietqhoang this is the config I use usually:

inherit_gem:
  rubocop-rails-omakase: rubocop.yml
  rubocop-rspec: config/default.yml
  rubocop-faker: config/default.yml
  action_policy: config/rubocop-rspec.yml
  test-prof:
    - config/default.yml
    - config/rubocop-rspec.yml

require:
  - test_prof/rubocop
  - rubocop-faker
  - rubocop-rspec
  - rubocop-md

require section is used to include new RuboCop cops to your config, at the same time inherit_gem is used to include configuration for those rules.

@vietqhoang
Copy link

vietqhoang commented Feb 23, 2024

@igor-alexandrov

Thanks for sharing your config. It led me to get my configuration working.

inherit_gem:
  rubocop-rails-omakase: rubocop.yml
  rubocop-rspec: config/default.yml

require:
  - rubocop-rspec

One other question. Although the cops are working as expected, I am experiencing a lot of output noise from rubocop-rspec. Is this something you also experience? I haven't experienced this in my other projects which use rubocop-rspec. Ideally I would like to silence these if these are just warnings.

/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/Capybara/CurrentPathExpectation has the wrong namespace - should be Capybara
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/Capybara/MatchStyle has the wrong namespace - should be Capybara
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/Capybara/NegationMatcher has the wrong namespace - should be Capybara
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/Capybara/SpecificActions has the wrong namespace - should be Capybara
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/Capybara/SpecificFinders has the wrong namespace - should be Capybara
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/Capybara/SpecificMatcher has the wrong namespace - should be Capybara
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/Capybara/VisibilityMatcher has the wrong namespace - should be Capybara
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/FactoryBot/AttributeDefinedStatically has the wrong namespace - should be FactoryBot
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/FactoryBot/ConsistentParenthesesStyle has the wrong namespace - should be FactoryBot
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/FactoryBot/CreateList has the wrong namespace - should be FactoryBot
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/FactoryBot/FactoryClassName has the wrong namespace - should be FactoryBot
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/FactoryBot/FactoryNameStyle has the wrong namespace - should be FactoryBot
/Users/viet/.rvm/gems/ruby-3.3.0/gems/rubocop-rspec-2.26.1/config/default.yml: RSpec/FactoryBot/SyntaxMethods has the wrong namespace - should be FactoryBot

Thanks again. And sorry for derailing the issue.

@igor-alexandrov
Copy link
Author

I have the same warnings when rubocop-rspec is enabled.

koic added a commit to koic/rubocop-rails-omakase that referenced this issue Jul 10, 2024
Closes rails#13.

This PR drops rubocop-minitest from gem dependency.

In fact, the `Minitest` department's Cop is not used in rubocop-rails-omakase:
https://github.com/rails/rubocop-rails-omakase/blob/v1.0.0/rubocop.yml

This means that it might be appropriate for users to add the `Minitest` department's cop only when they start using it.
As a result, there will be no need to install gems that are not included in the rubocop-rails-omakase configuration.

Users can be given the choice to add cops based on the testing framework they are using.
@jeremy jeremy closed this as completed in #18 Nov 3, 2024
@jeremy jeremy closed this as completed in d5066bd Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants