From c4d900e01f6cd7b0a14c2326b0f7f4cd0f98ab8b Mon Sep 17 00:00:00 2001 From: ignacio-chiazzo Date: Sat, 26 Mar 2022 18:03:05 -0400 Subject: [PATCH] Added new duplicated_test_run cop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a Minitest class inherits from another class, it will also inherit its methods causing Minitest to run the parent’s tests methods twice. This PR detects when there are two tests classes, one inherits from the other, and both have tests methods. MinitestRubocop will add an offence to the Child class in such a case. Note that this cop only works if both classes are on the same file. --- changelog/new_duplicated_test_run_cop.md | 0 config/default.yml | 5 + .../cop/minitest/duplicated_test_run.rb | 87 ++++++++++++++ lib/rubocop/cop/minitest_cops.rb | 1 + .../cop/minitest/duplicated_test_run_test.rb | 106 ++++++++++++++++++ 5 files changed, 199 insertions(+) create mode 100644 changelog/new_duplicated_test_run_cop.md create mode 100644 lib/rubocop/cop/minitest/duplicated_test_run.rb create mode 100644 test/rubocop/cop/minitest/duplicated_test_run_test.rb diff --git a/changelog/new_duplicated_test_run_cop.md b/changelog/new_duplicated_test_run_cop.md new file mode 100644 index 00000000..e69de29b diff --git a/config/default.yml b/config/default.yml index dd836ca6..8ce257a8 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1,3 +1,8 @@ +Minitest/DuplicatedTestRun: + Description: 'This cop detects when a test class has test methods, and its parent also has tests methods causing the parents tests to run them twice.' + Enabled: pending + VersionAdded: '<>' + Minitest: Enabled: true Include: diff --git a/lib/rubocop/cop/minitest/duplicated_test_run.rb b/lib/rubocop/cop/minitest/duplicated_test_run.rb new file mode 100644 index 00000000..aa6668a8 --- /dev/null +++ b/lib/rubocop/cop/minitest/duplicated_test_run.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Minitest + # If a Minitest class inherits from another class, + # it will also inherit its methods causing Minitest to run the parent's tests methods twice. + # + # This cop detects when there are two tests classes, one inherits from the other, and both have tests methods. + # This cop will add an offence to the Child class in such a case. + # + # @example + # # bad + # class ParentTest < Minitest::Test + # def test_parent # it will run this test twice. + # end + # end + # + # class ChildTest < ParentTest + # def test_child + # end + # end + # + # + # # good + # class ParentTest < Minitest::Test + # def test_parent + # end + # end + # + # class ChildTest < Minitest::Test + # def test_child + # end + # end + # + # # good + # class ParentTest < Minitest::Test + # end + # + # class ChildTest + # def test_child + # end + # + # def test_parent + # end + # end + # + class DuplicatedTestRun < Base + include MinitestExplorationHelpers + + MSG = 'Subclasses with test methods causes the parent\' tests to run them twice.' + + def on_class(class_node) + return unless test_class?(class_node) + + return unless test_methods?(class_node) + + # If the class if a test method and has tests on it, + # we look at the parent and see if it has tests too. + + parent_class = class_node.parent_class + + return unless parent_class_has_test_methods(class_node, parent_class) + + message = format(MSG) + add_offense(class_node, message: message) + end + + private + + def parent_class_has_test_methods(class_node, parent_class) + parent_class_node = class_node.parent.each_child_node(:class).detect do |klass| + klass.identifier == parent_class + end + + return unless parent_class_node + + test_methods?(parent_class_node) + end + + def test_methods?(class_node) + test_cases(class_node).size.positive? + end + end + end + end +end diff --git a/lib/rubocop/cop/minitest_cops.rb b/lib/rubocop/cop/minitest_cops.rb index f2a8516f..ab7cfcf8 100644 --- a/lib/rubocop/cop/minitest_cops.rb +++ b/lib/rubocop/cop/minitest_cops.rb @@ -23,6 +23,7 @@ require_relative 'minitest/assert_respond_to' require_relative 'minitest/assert_silent' require_relative 'minitest/assert_truthy' +require_relative 'minitest/duplicated_test_run' require_relative 'minitest/global_expectations' require_relative 'minitest/literal_as_actual_argument' require_relative 'minitest/multiple_assertions' diff --git a/test/rubocop/cop/minitest/duplicated_test_run_test.rb b/test/rubocop/cop/minitest/duplicated_test_run_test.rb new file mode 100644 index 00000000..00fb9803 --- /dev/null +++ b/test/rubocop/cop/minitest/duplicated_test_run_test.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require 'test_helper' + +class DuplicatedTestRunTest < Minitest::Test + def test_registers_offense_when_parent_and_child_have_tests_methods + assert_offense(<<~RUBY) + class ParentTest < Minitest::Test + def test_parent + end + end + + class ChildTest < ParentTest + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Subclasses with test methods causes the parent' tests to run them twice. + def test_child_asserts_twice + assert_equal(1, 1) + end + end + RUBY + end + + def test_registers_offense_when_parent_and_children_have_tests_methods + assert_offense(<<~RUBY) + class ParentTest < Minitest::Test + def test_parent + end + end + + class Child1Test < ParentTest + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Subclasses with test methods causes the parent' tests to run them twice. + def test_parent + end + end + + class Child2Test < ParentTest + end + + class Child3Test < ParentTest + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Subclasses with test methods causes the parent' tests to run them twice. + def test_1_child_2_asserts_twice + end + + def test_2_child_2_asserts_twice + end + + def test_3_child_2_asserts_twice + end + end + RUBY + end + + def test_does_not_register_offense_if_the_parent_does_not_have_test_methods + assert_no_offenses(<<~RUBY) + class ParentTest < Minitest::Test + end + + class ChildTest < ParentTest + def test_child_asserts_twice + assert_equal(1, 1) + end + end + RUBY + end + + def test_does_not_register_offense_if_the_child_does_not_have_test_methods + assert_no_offenses(<<~RUBY) + class ParentTest < Minitest::Test + def test_child_asserts_twice + assert_equal(1, 1) + end + end + + class ChildTest < ParentTest + end + RUBY + end + + def test_does_not_register_offense_if_the_class_has_no_children + assert_no_offenses(<<~RUBY) + class ParentTest < Minitest::Test + def test_child_asserts_twice + assert_equal(1, 1) + end + end + + class ClassTwo < ParentTest + end + RUBY + end + + def test_does_not_register_offense_if_the_class_is_not_a_test_class + assert_no_offenses(<<~RUBY) + class ParentTest < ExampleClass + def test_child_asserts_twice + assert_equal(1, 1) + end + end + + class ChildClass < ParentTest + def test_child_asserts_twice + assert_equal(1, 1) + end + end + RUBY + end +end