Skip to content
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

Do not treat test_ methods with arguments as test methods #231

Merged
merged 1 commit into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/change_what_is_considered_a_test_case.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#231](https://github.com/rubocop/rubocop-minitest/pull/231): Change what is considered a test case by `rubocop-minitest` (`public` method without arguments with `test_` name prefix). ([@fatkodima][])
9 changes: 7 additions & 2 deletions lib/rubocop/cop/mixin/minitest_exploration_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module Cop
# Helper methods for different explorations against test files and test cases.
# @api private
module MinitestExplorationHelpers
include DefNode
extend NodePattern::Macros

ASSERTION_PREFIXES = %w[assert refute].freeze
Expand All @@ -27,14 +28,14 @@ def test_class?(class_node)
end

def test_case?(node)
return false unless node&.def_type? && test_case_name?(node.method_name)
return false unless node&.def_type? && test_method?(node)

class_ancestor = node.each_ancestor(:class).first
test_class?(class_ancestor)
end

def test_cases(class_node)
test_cases = class_def_nodes(class_node).select { |def_node| test_case_name?(def_node.method_name) }
test_cases = class_def_nodes(class_node).select { |def_node| test_method?(def_node) }

# Support Active Support's `test 'example' { ... }` method.
# https://api.rubyonrails.org/classes/ActiveSupport/Testing/Declarative.html
Expand All @@ -43,6 +44,10 @@ def test_cases(class_node)
test_cases + test_blocks
end

def test_method?(def_node)
test_case_name?(def_node.method_name) && !def_node.arguments? && !non_public?(def_node)
end

def lifecycle_hooks(class_node)
class_def_nodes(class_node)
.select { |def_node| lifecycle_hook_method?(def_node) }
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/minitest/assert_offense.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ module AssertOffense

def setup
cop_name = self.class.to_s.delete_suffix('Test')
return unless RuboCop::Cop::Minitest.const_defined?(cop_name)

@cop = RuboCop::Cop::Minitest.const_get(cop_name).new
end
Expand Down
50 changes: 50 additions & 0 deletions test/rubocop/cop/mixin/minitest_exploration_helpers_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# frozen_string_literal: true

require 'test_helper'

class MinitestExplorationHelpersTest < Minitest::Test
module Helper
extend RuboCop::Cop::MinitestExplorationHelpers
class << self
public :test_case?
end
end

SOURCE = <<~RUBY
class FooTest < Minitest::Test
def not_test_case; end
def test_with_arguments(arg); end
def test_public; end

protected
def test_protected; end

private
def test_private; end
end
RUBY

def test_test_case_returns_true_for_test_case
assert Helper.test_case?(method_node(:test_public))
end

def test_test_case_returns_false_for_hidden_test_methods
refute Helper.test_case?(method_node(:test_protected))
refute Helper.test_case?(method_node(:test_private))
end

def test_test_case_returns_false_for_non_test_methods
refute Helper.test_case?(method_node(:non_test_case))
end

def test_test_case_returns_false_for_test_methods_with_arguments
refute Helper.test_case?(method_node(:test_with_arguments))
end

private

def method_node(method_name)
class_node = parse_source!(SOURCE).ast
class_node.body.each_child_node(:def).find { |def_node| def_node.method?(method_name) }
end
end