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

[Bug] Minitest/Assert* having conflicts with Rails/RefuteMethods policy #319

Open
bogdan opened this issue Sep 6, 2024 · 6 comments
Open

Comments

@bogdan
Copy link

bogdan commented Sep 6, 2024

Bug Description

The rules Minitest/Assert* has some conflicts with Rails/RefuteMethods. It has default policy to convert refute_* statements to assert_not_* which causes Minitest/Assert* to be ignored.

Example:

# Statement
refute user.new_record?
# Will be converted by Rails/RefuteMethods to
assert_not user.new_record? 
# And will be ignored by Minitest/AssertPredicate

Fix

Make all rules consistent when converting generic refute or assert_not statements to refute_* or assert_not_* statements making sure it supports default policy established by Rails/RefuteMethods:

# bad
assert_not(obj.one?)
assert_not(obj.one?, 'message')

# good
assert_not_predicate(obj, :one?)
assert_not_predicate(obj, :one?, 'message')
@bogdan bogdan changed the title Minitest/AssertPredicate should fix negated assertion [Bug] Minitest/Assert* rules having conflicts with Rails/RefuteMethods policy Sep 6, 2024
@bogdan bogdan changed the title [Bug] Minitest/Assert* rules having conflicts with Rails/RefuteMethods policy [Bug] Minitest/Assert* having conflicts with Rails/RefuteMethods policy Sep 6, 2024
@Earlopain
Copy link
Contributor

assert_not_predicate is rails specific, I have a PR for rubocop-rails to handle this: rubocop/rubocop-rails#1259. Also see rubocop/rubocop-rails#1155

@bogdan
Copy link
Author

bogdan commented Sep 6, 2024

Will it fix others like assert_not array.include?(value)?

@Earlopain
Copy link
Contributor

Into what would this correct? I don't believe these methods work with arguments like that.

@Earlopain
Copy link
Contributor

Earlopain commented Sep 6, 2024

Sorry, my bad. No, this will not be handled by that PR. I can look into that once the other PR gets merged.

@bogdan
Copy link
Author

bogdan commented Sep 6, 2024

Hm, you are right assert_not_include doesn't exist (which looks like a bug to me, but unrelated to rubocop in any wway). But assert_not_equal, assert_not_nil and many others do exist. The full list is here: https://api.rubyonrails.org/classes/ActiveSupport/TestCase.html#method-i-assert_no_match

@Earlopain
Copy link
Contributor

refute array.include?(value) corrects into assert_not_includes array, value. Just one of those ruby File.exist?/File.exists? things I'd guess. Still, this is rails specific as well. Would you mind opening an issue in https://github.com/rubocop/rubocop-rails for that?

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

No branches or pull requests

2 participants