rubocop / rubocop-minitest

Code style checking for Minitest files.
https://docs.rubocop.org/rubocop-minitest
MIT License
144 stars 44 forks source link

`test_*` method out of `ActiveSupport::TestCase` subclasses #279

Closed yahonda closed 11 months ago

yahonda commented 11 months ago

Is your feature request related to a problem? Please describe.

Reviewing https://github.com/rails/rails/pull/50334#issuecomment-1851411434 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 of ActiveSupport::TestCase subclasses.

Steps to reproduce

git clone https://github.com/rails/rails
cd rails/activerecord
bundle install
bin/test test/cases/assertions/query_assertions_test.rb -v

Actual behavior

Only ActiveRecord::Assertions::QueryAssertionsTest#test_assert_queries is executed.

$ bin/test test/cases/assertions/query_assertions_test.rb -v
Using sqlite3
Run options: -v --seed 60664

# Running:

ActiveRecord::Assertions::QueryAssertionsTest#test_assert_queries = 0.02 s = .

Finished in 0.024659s, 40.5533 runs/s, 486.6391 assertions/s.
1 runs, 12 assertions, 0 failures, 0 errors, 0 skips
$

There are two test_assert_queries and test_assert_no_queries methods and only test_assert_queries test is executed because it belongs to QueryAssertionsTest class. test_assert_no_queries is out of QueryAssertionsTest class then not executed.

Describe the solution you'd like

I wanted some cop to find any test_* methods that are out of ActiveSupport::TestCase subclasses.

Describe alternatives you've considered

Compare the Execute the test with -v option and git grep "def test_" to see which tests are executed, that is actually hard because there are many tests.

$ bin/test test/cases/assertions/query_assertions_test.rb -v
Using sqlite3
Run options: -v --seed 60664

# Running:

ActiveRecord::Assertions::QueryAssertionsTest#test_assert_queries = 0.02 s = .

Finished in 0.024659s, 40.5533 runs/s, 486.6391 assertions/s.
1 runs, 12 assertions, 0 failures, 0 errors, 0 skips
$
$ git grep 'def test_' test/cases/assertions/query_assertions_test.rb
test/cases/assertions/query_assertions_test.rb:      def test_assert_queries
test/cases/assertions/query_assertions_test.rb:    def test_assert_no_queries
$

Additional context

Add any other context or screenshots about the feature request here.

koic commented 11 months ago

I see. Just to be sure, can you write examples of both a bad case and a good case that you expect?

yahonda commented 11 months ago

Sure. Will update the good and bad cases.

yahonda commented 11 months ago

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
Earlopain commented 11 months ago

Tests may be extracted into modules to include them later with different behaviour, that should be considered.

I know the httpx gem makes heave use of that, see https://gitlab.com/os85/httpx/-/blob/master/test/support/requests/plugins/auth.rb?ref_type=heads and https://gitlab.com/os85/httpx/-/blob/master/test/http_test.rb?ref_type=heads with https://gitlab.com/os85/httpx/-/blob/master/test/https_test.rb?ref_type=heads for testing against http and https with the same code.

andyw8 commented 11 months ago

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.

koic commented 11 months ago

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.

koic commented 11 months ago

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.