Merge pull request #638 from gmcgibbon/controller_testing_cop

Add Rails/ControllerTesting cop

Author: koic

changelog/new_rails_action_controller_test_case_cop.md

@@ -0,0 +1 @@
+* [#638](https://github.com/rubocop/rubocop-rails/pull/638): Add new `Rails/ActionControllerTestCase` cop. ([@gmcgibbon][])

config/default.yml

@@ -38,6 +38,16 @@ Lint/NumberConversion:
     - fortnights
     - in_milliseconds
 
+Rails/ActionControllerTestCase:
+  Description: 'Use `ActionDispatch::IntegrationTest` instead of `ActionController::TestCase`.'
+  StyleGuide: 'https://rails.rubystyle.guide/#integration-testing'
+  Reference: 'https://api.rubyonrails.org/classes/ActionController/TestCase.html'
+  Enabled: 'pending'
+  SafeAutocorrect: false
+  VersionAdded: '<<next>>'
+  Include:
+    - '**/test/**/*.rb'
+
 Rails/ActionFilter:
   Description: 'Enforces consistent use of action filter methods.'
   Enabled: true

lib/rubocop/cop/rails/action_controller_test_case.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # Using `ActionController::TestCase`` is discouraged and should be replaced by
+      # `ActionDispatch::IntegrationTest``. Controller tests are too close to the
+      # internals of a controller whereas integration tests mimic the browser/user.
+      #
+      # @safety
+      #   This cop's autocorrection is unsafe because the API of each test case class is different.
+      #   Make sure to update each test of your controller test cases after changing the superclass.
+      #
+      # @example
+      #   # bad
+      #   class MyControllerTest < ActionController::TestCase
+      #   end
+      #
+      #   # good
+      #   class MyControllerTest < ActionDispatch::IntegrationTest
+      #   end
+      #
+      class ActionControllerTestCase < Base
+        extend AutoCorrector
+        extend TargetRailsVersion
+
+        MSG = 'Use `ActionDispatch::IntegrationTest` instead.'
+
+        minimum_target_rails_version 5.0
+
+        def_node_matcher :action_controller_test_case?, <<~PATTERN
+          (class
+            (const nil? _)
+            (const (const {nil? cbase} :ActionController) :TestCase) nil?)
+        PATTERN
+
+        def on_class(node)
+          return unless action_controller_test_case?(node)
+
+          add_offense(node.parent_class) do |corrector|
+            corrector.replace(node.parent_class, 'ActionDispatch::IntegrationTest')
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -7,6 +7,7 @@
 require_relative 'mixin/migrations_helper'
 require_relative 'mixin/target_rails_version'
 
+require_relative 'rails/action_controller_test_case'
 require_relative 'rails/action_filter'
 require_relative 'rails/active_record_aliases'
 require_relative 'rails/active_record_callbacks_order'

spec/rubocop/cop/rails/action_controller_test_case_spec.rb

@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::ActionControllerTestCase, :config do
+  context 'Rails 4.2', :rails42 do
+    it 'does not add offense when extending `ActionController::TestCase`' do
+      expect_no_offenses(<<~RUBY)
+        class MyControllerTest < ActionController::TestCase
+        end
+      RUBY
+    end
+  end
+
+  it 'adds offense when extending `ActionController::TestCase`' do
+    expect_offense(<<~RUBY)
+      class MyControllerTest < ActionController::TestCase
+                               ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActionDispatch::IntegrationTest` instead.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class MyControllerTest < ActionDispatch::IntegrationTest
+      end
+    RUBY
+  end
+
+  it 'adds offense when extending `::ActionController::TestCase`' do
+    expect_offense(<<~RUBY)
+      class MyControllerTest < ::ActionController::TestCase
+                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActionDispatch::IntegrationTest` instead.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class MyControllerTest < ActionDispatch::IntegrationTest
+      end
+    RUBY
+  end
+
+  it 'does not add offense when extending `ActionDispatch::IntegrationTest`' do
+    expect_no_offenses(<<~RUBY)
+      class MyControllerTest < ActionDispatch::IntegrationTest
+      end
+    RUBY
+  end
+
+  it 'does not add offense when extending custom superclass' do
+    expect_no_offenses(<<~RUBY)
+      class MyControllerTest < SuperControllerTest
+      end
+    RUBY
+  end
+end