-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
test_*
method out of ActiveSupport::TestCase
subclasses
#279
Comments
I see. Just to be sure, can you write examples of both a bad case and a good case that you expect? |
Sure. Will update the good and bad cases. |
Here is my expected bad and good example. # bad
module ActiveRecord
module Assertions
class FooTest < ActiveSupport::TestCase
end
def test_something
assert true
end
end
end # good
module ActiveRecord
module Assertions
class FooTest < ActiveSupport::TestCase
def test_something
assert true
end
end
end
end |
Tests may be extracted into modules to include them later with different behaviour, that should be considered. I know the |
I worry this will cause too many false positives – I'd like to see how it runs against https://github.com/jeromedalbert/real-world-ruby-apps. |
Resolves rubocop#279. This PR adds new `Minitest/NonExecutableTestMethod` cop, which checks for the use of test methods outside of a test class. Test methods should be defined within a test class to ensure their execution. NOTE: This cop assumes that classes whose superclass name includes the word "`Test`" are test classes, in order to prevent false positives. ```ruby # bad class FooTest < Minitest::Test end def test_method_should_be_inside_test_class end # good class FooTest < Minitest::Test def test_method_should_be_inside_test_class end end ```
Resolves rubocop#279. This PR adds new `Minitest/NonExecutableTestMethod` cop, which checks for the use of test methods outside of a test class. Test methods should be defined within a test class to ensure their execution. NOTE: This cop assumes that classes whose superclass name includes the word "`Test`" are test classes, in order to prevent false positives. ```ruby # bad class FooTest < Minitest::Test end def test_method_should_be_inside_test_class end # good class FooTest < Minitest::Test def test_method_should_be_inside_test_class end end ```
I've opened #280. This new cop assumes that classes whose superclass name includes the word "Test" are test classes, in order to prevent false positives. Stricter criteria for detection are implemented to prevent false positives; however, the occurrence of some false negatives cannot be completely avoided. |
Unfortunately, https://github.com/jeromedalbert/real-world-ruby-apps is too large, resulting in high costs for inspection, so it has not been carried out. However, it has been tested with rails/rails repo. |
…t_method_cop [Fix #279] Add new `Minitest/NonExecutableTestMethod` cop
This pull request enables `Minitest/NonExecutableTestMethod` cop to find non-executed test that is out of `ActiveSupport::TestCase` and its subclasses. This cop is based on the request since there was a test that is not executed found at rails#50334 (comment) and implemented to RuboCop Minitest 0.34.0 via rubocop/rubocop-minitest#279 This cop works as follows. As of right now, there is no offenses by reverting the merge commit via rails#50334 ``` $ git revert -m 1 9517841 $ bundle exec rubocop Inspecting 3254 files ... snip ... Offenses: activerecord/test/cases/assertions/query_assertions_test.rb:27:5: W: Minitest/NonExecutableTestMethod: Test method should be defined inside a test class to ensure execution. def test_assert_no_queries ... ^^^^^^^^^^^^^^^^^^^^^^^^^^ 3254 files inspected, 1 offense detected $ ```
This pull request enables `Minitest/NonExecutableTestMethod` cop to find non-executed test that is out of `ActiveSupport::TestCase` and its subclasses. This cop is based on the request since there was a test that is not executed found at rails#50334 (comment) and implemented to RuboCop Minitest 0.34.0 via rubocop/rubocop-minitest#279 This cop works as follows. As of right now, there is no offenses by reverting the merge commit via rails#50334 ``` $ git revert -m 1 9517841 $ bundle exec rubocop Inspecting 3254 files ... snip ... Offenses: activerecord/test/cases/assertions/query_assertions_test.rb:27:5: W: Minitest/NonExecutableTestMethod: Test method should be defined inside a test class to ensure execution. def test_assert_no_queries ... ^^^^^^^^^^^^^^^^^^^^^^^^^^ 3254 files inspected, 1 offense detected $ ``` * `Gemfile.lock` has been updated as follows ``` bundle update rubocop rubocop-minitest --conservative ```
This pull request enables `Minitest/NonExecutableTestMethod` cop to find non-executed test that is out of `ActiveSupport::TestCase` and its subclasses. This cop is based on the request since there was a test that is not executed found at rails#50334 (comment) and implemented to RuboCop Minitest 0.34.0 via rubocop/rubocop-minitest#279 This cop works as follows. As of right now, there is no offenses by reverting the merge commit via rails#50334 ``` $ git revert -m 1 9517841 $ bundle exec rubocop Inspecting 3254 files ... snip ... Offenses: activerecord/test/cases/assertions/query_assertions_test.rb:27:5: W: Minitest/NonExecutableTestMethod: Test method should be defined inside a test class to ensure execution. def test_assert_no_queries ... ^^^^^^^^^^^^^^^^^^^^^^^^^^ 3254 files inspected, 1 offense detected $ ``` * `Gemfile.lock` has been updated as follows ``` bundle update rubocop rubocop-minitest --conservative ```
Is your feature request related to a problem? Please describe.
Reviewing rails/rails#50334 (comment) and found that there are some tests that are not executed because it does not belongs to
ActiveSupport::TestCase
subclass.I'd like some cop to find
test_*
method out ofActiveSupport::TestCase
subclasses.Steps to reproduce
Actual behavior
Only
ActiveRecord::Assertions::QueryAssertionsTest#test_assert_queries
is executed.There are two
test_assert_queries
andtest_assert_no_queries
methods and onlytest_assert_queries
test is executed because it belongs toQueryAssertionsTest
class.test_assert_no_queries
is out ofQueryAssertionsTest
class then not executed.Describe the solution you'd like
I wanted some cop to find any
test_*
methods that are out ofActiveSupport::TestCase
subclasses.Describe alternatives you've considered
Compare the Execute the test with
-v
option andgit grep "def test_"
to see which tests are executed, that is actually hard because there are many tests.Additional context
Add any other context or screenshots about the feature request here.
The text was updated successfully, but these errors were encountered: