-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
[Fix #169] Add new Minitest/SkipEnsure
cop
#171
[Fix #169] Add new Minitest/SkipEnsure
cop
#171
Conversation
8e4e7a2
to
5ddc634
Compare
36b2ae5
to
1843925
Compare
@yahonda I'm worried about the cop name. It may be abstract, but the cop name |
Let me have time to think about the cop's name. |
I like |
Thank you for your feedback and suggestion. I'll take the simplest |
Closes rubocop#169. This cop checks that `ensure` call even if `skip`. It is unexpected that `ensure` will be called when skipping test. If conditional `skip` is used, it checks that `ensure` is also called conditionally. On the other hand, it accepts `skip` used in `rescue` because `ensure` may be teardown process to `begin` setup process. ```ruby # bad def test_skip skip 'This test is skipped.' assert 'foo'.present? ensure do_something end # bad def test_conditional_skip skip 'This test is skipped.' if condition assert do_something ensure do_teardown end # good def test_skip skip 'This test is skipped.' begin assert 'foo'.present? ensure do_something end end # good def test_conditional_skip skip 'This test is skipped.' if condition assert do_something ensure if condition do_teardown end end # good def test_skip_is_used_in_rescue do_setup assert do_something rescue skip 'This test is skipped.' ensure do_teardown end ```
1843925
to
01feafd
Compare
Minitest/EnsureCallEvenIfSkip
copMinitest/SkipEnsure
cop
@yahonda RuboCop Minitest 0.20 has been released. |
Thanks for implementing |
Opened rails/rails#45216 |
Closes #169.
This cop checks that
ensure
call even ifskip
. It is unexpected thatensure
will be called when skipping test.If conditional
skip
is used, it checks thatensure
is also called conditionally.On the other hand, it accepts
skip
used inrescue
becauseensure
may be teardown process tobegin
setup process.Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.