-
-
Notifications
You must be signed in to change notification settings - Fork 277
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
feat: support correcting assert_nil
and refute_nil
to RSpec/Rails/MinitestAssertions
#1773
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,73 +1,146 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe RuboCop::Cop::RSpec::Rails::MinitestAssertions do | ||
it 'registers an offense when using `assert_equal`' do | ||
expect_offense(<<~RUBY) | ||
assert_equal(a, b) | ||
^^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
expect(b).to eq(a) | ||
RUBY | ||
end | ||
context 'with equal assertions' do | ||
it 'registers an offense when using `assert_equal`' do | ||
expect_offense(<<~RUBY) | ||
assert_equal(a, b) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was a miss in the original implementation, it’s confusing that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point - I'll leave it for now since this diff is already pretty big, but happy to do a follow up addressing it |
||
^^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`. | ||
RUBY | ||
|
||
it 'registers an offense when using `assert_equal` with no parentheses' do | ||
expect_offense(<<~RUBY) | ||
assert_equal a, b | ||
^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`. | ||
RUBY | ||
expect_correction(<<~RUBY) | ||
expect(b).to eq(a) | ||
RUBY | ||
end | ||
|
||
expect_correction(<<~RUBY) | ||
expect(b).to eq(a) | ||
RUBY | ||
end | ||
it 'registers an offense when using `assert_equal` with no parentheses' do | ||
expect_offense(<<~RUBY) | ||
assert_equal a, b | ||
^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`. | ||
RUBY | ||
|
||
it 'registers an offense when using `assert_equal` with failure message' do | ||
expect_offense(<<~RUBY) | ||
assert_equal a, b, "must be equal" | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`. | ||
RUBY | ||
expect_correction(<<~RUBY) | ||
expect(b).to eq(a) | ||
RUBY | ||
end | ||
|
||
expect_correction(<<~RUBY) | ||
expect(b).to(eq(a), "must be equal") | ||
RUBY | ||
end | ||
it 'registers an offense when using `assert_equal` with failure message' do | ||
expect_offense(<<~RUBY) | ||
assert_equal a, b, "must be equal" | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`. | ||
RUBY | ||
|
||
it 'registers an offense when using `assert_equal` with ' \ | ||
'multi-line arguments' do | ||
expect_offense(<<~RUBY) | ||
assert_equal(a, | ||
^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`. | ||
b, | ||
"must be equal") | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
expect(b).to(eq(a), "must be equal") | ||
RUBY | ||
end | ||
expect_correction(<<~RUBY) | ||
expect(b).to(eq(a), "must be equal") | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `refute_equal`' do | ||
expect_offense(<<~RUBY) | ||
refute_equal a, b | ||
^^^^^^^^^^^^^^^^^ Use `expect(b).not_to eq(a)`. | ||
RUBY | ||
it 'registers an offense when using `assert_equal` with ' \ | ||
'multi-line arguments' do | ||
expect_offense(<<~RUBY) | ||
assert_equal(a, | ||
^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`. | ||
b, | ||
"must be equal") | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
expect(b).not_to eq(a) | ||
RUBY | ||
end | ||
expect_correction(<<~RUBY) | ||
expect(b).to(eq(a), "must be equal") | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `expect(b).to eq(a)`' do | ||
expect_no_offenses(<<~RUBY) | ||
expect(b).to eq(a) | ||
RUBY | ||
it 'registers an offense when using `refute_equal`' do | ||
expect_offense(<<~RUBY) | ||
refute_equal a, b | ||
^^^^^^^^^^^^^^^^^ Use `expect(b).not_to eq(a)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
expect(b).not_to eq(a) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `expect(b).to eq(a)`' do | ||
expect_no_offenses(<<~RUBY) | ||
expect(b).to eq(a) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `expect(b).not_to eq(a)`' do | ||
expect_no_offenses(<<~RUBY) | ||
expect(b).not_to eq(a) | ||
RUBY | ||
end | ||
end | ||
|
||
it 'does not register an offense when using `expect(b).not_to eq(a)`' do | ||
expect_no_offenses(<<~RUBY) | ||
expect(b).not_to eq(a) | ||
RUBY | ||
context 'with nil assertions' do | ||
it 'registers an offense when using `assert_nil`' do | ||
expect_offense(<<~RUBY) | ||
assert_nil(a) | ||
^^^^^^^^^^^^^ Use `expect(a).to eq(nil)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
expect(a).to eq(nil) | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `assert_nil` with no parentheses' do | ||
expect_offense(<<~RUBY) | ||
assert_nil a | ||
^^^^^^^^^^^^ Use `expect(a).to eq(nil)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
expect(a).to eq(nil) | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `assert_nil` with failure message' do | ||
expect_offense(<<~RUBY) | ||
assert_nil a, "must be nil" | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(a).to(eq(nil), "must be nil")`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
expect(a).to(eq(nil), "must be nil") | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `assert_nil` with ' \ | ||
'multi-line arguments' do | ||
expect_offense(<<~RUBY) | ||
assert_nil(a, | ||
^^^^^^^^^^^^^ Use `expect(a).to(eq(nil), "must be nil")`. | ||
"must be nil") | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
expect(a).to(eq(nil), "must be nil") | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `refute_nil`' do | ||
expect_offense(<<~RUBY) | ||
refute_nil a | ||
^^^^^^^^^^^^ Use `expect(a).not_to eq(nil)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
expect(a).not_to eq(nil) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `expect(a).to eq(nil)`' do | ||
expect_no_offenses(<<~RUBY) | ||
expect(a).to eq(nil) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `expect(a).not_to eq(nil)`' do | ||
expect_no_offenses(<<~RUBY) | ||
expect(a).not_to eq(nil) | ||
RUBY | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change should not have been made. This should only be changed when the default settings are changed. For example, when you add a configurable option or change the default for a configurable option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix: #1777
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, my bad. I should have caught that in review.
Thanks for discovering, and fixing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for what it's worth I feel like that was required by
bundle exec rake
, but it might have been I just messed something up or changed something else I shouldn't have locally to make it think this needed updating 🤷I'll keep this in mind for future contributions and let you know if I do find the tasks behaving wrongly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind! Thank you for your work.