rubocop / rubocop-minitest

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

Minitest/TestMethodName false negative #312

Closed MatzFan closed 4 months ago

MatzFan commented 4 months ago

No offense is registered with the following setup:

foo.rb

# frozen_string_literal: true

# class
class Foo
  def bar
    'Hello world'
  end
end

test_foo.rb

# frozen_string_literal: true

require 'minitest/autorun'
require_relative 'foo'

# tests
class TestFoo < Minitest::Test
  def test_bar
    assert_raises(ArgumentError) { Foo.new.bar('an_arg') }
  end

  def not_a_test
    assert_raises(ArgumentError) { Foo.new.bar('an_arg') }
  end
end

.rubocop.yml

require: rubocop-minitest

AllCops:
  NewCops: enable

Minitest/TestMethodName:
  Enabled: true

Expected behavior

I expect an offense for the second method in the test file.

Actual behavior

No offense is registered.

Steps to reproduce the problem

Run rubocop or rubocop --require rubocop-minitest in the root directory with the above files.

RuboCop version

1.65.0 (using Parser 3.3.4.0, rubocop-ast 1.31.3, running on ruby 3.3.4) [x86_64-linux]
  - rubocop-minitest 0.35.1
andyw8 commented 4 months ago

Your test class name should end in Test:

https://github.com/rubocop/rubocop-minitest/blob/4d08365be08d06b02bb2c11415a4e8060331a0bb/lib/rubocop/cop/mixin/minitest_exploration_helpers.rb#L29

MatzFan commented 4 months ago

Thanks. Same result with

class FooTest < Minitest::Test
  def test_bar
    assert_raises(ArgumentError) { Foo.new.bar('an_arg') }
  end

  def not_a_test
    assert_raises(ArgumentError) { Foo.new.bar('an_arg') }
  end
end

And if my test classes are not correctly named shouldn't that trigger another cop? TestFoo seems to be the correct convention for Minitest class names and is the one I've always used.

andyw8 commented 4 months ago

After renaming to FooTest, I see this reported:

Offenses:

test/foo_test.rb:12:7: C: [Correctable] Minitest/TestMethodName: Test method name should start with test_ prefix.
  def not_a_test
      ^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense autocorrectable

And if my test classes are not correctly named shouldn't that trigger another cop? TestFoo seems to be the correct convention for Minitest class names and is the one I've always used.

It's may be better to create a separate issue for discussing this. The Rails convention is to have the class name end with Test, which is probably what most people follow (including RuboCop itelf), and so this hasn't been noticed.

MatzFan commented 4 months ago

OK, so after a class rename the problem is solved in my project. Agree 'Test' suffix is better anyhow. Thanks for your help.