From b33f71729882caef8766da5752ec38212abef7f4 Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Fri, 4 Feb 2022 18:15:40 -0500
Subject: [PATCH] Add new Rails/ActionControllerTesting cop

Add controller testing cop to discourage use of
ActionController::TestCase. ActionDispatch::IntegrationTest should be
used instead to test controllers.
---
 ...new_rails_action_controller_testing_cop.md |  1 +
 config/default.yml                            | 10 ++++
 .../cop/rails/action_controller_testing.rb    | 47 +++++++++++++++++
 lib/rubocop/cop/rails_cops.rb                 |  1 +
 .../rails/action_controller_testing_spec.rb   | 52 +++++++++++++++++++
 5 files changed, 111 insertions(+)
 create mode 100644 changelog/new_rails_action_controller_testing_cop.md
 create mode 100644 lib/rubocop/cop/rails/action_controller_testing.rb
 create mode 100644 spec/rubocop/cop/rails/action_controller_testing_spec.rb

diff --git a/changelog/new_rails_action_controller_testing_cop.md b/changelog/new_rails_action_controller_testing_cop.md
new file mode 100644
index 0000000000..551c3be9ef
--- /dev/null
+++ b/changelog/new_rails_action_controller_testing_cop.md
@@ -0,0 +1 @@
+* [#638](https://github.com/rubocop/rubocop-rails/pull/638): Add new `Rails/ActionControllerTesting` cop. ([@gmcgibbon][])
diff --git a/config/default.yml b/config/default.yml
index 446672b277..958a648962 100644
--- a/config/default.yml
+++ b/config/default.yml
@@ -38,6 +38,16 @@ Lint/NumberConversion:
     - fortnights
     - in_milliseconds
 
+Rails/ActionControllerTesting:
+  Description: 'Use `ActionDispatch::IntegrationTest` instead of `ActionController::TestCase`.'
+  Reference: 'https://api.rubyonrails.org/classes/ActionController/TestCase.html'
+  StyleGuide: 'https://rails.rubystyle.guide/#integration-testing'
+  Enabled: true
+  SafeAutocorrect: false
+  VersionAdded: '<<next>>'
+  Include:
+    - '**/test/**/*.rb'
+
 Rails/ActionFilter:
   Description: 'Enforces consistent use of action filter methods.'
   Enabled: true
diff --git a/lib/rubocop/cop/rails/action_controller_testing.rb b/lib/rubocop/cop/rails/action_controller_testing.rb
new file mode 100644
index 0000000000..d615a3ec68
--- /dev/null
+++ b/lib/rubocop/cop/rails/action_controller_testing.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 ActionControllerTesting < Base
+        extend AutoCorrector
+        extend TargetRailsVersion
+
+        MSG = 'Use `ActionDispatch::IntegrationTest` instead.'
+
+        minimum_target_rails_version 5.0
+
+        def_node_matcher :controller_test_case?, <<~PATTERN
+          (class
+            (const nil? _)
+            (const (const _ :ActionController) :TestCase) nil?)
+        PATTERN
+
+        def on_class(node)
+          return unless controller_test_case?(node)
+
+          add_offense(node.parent_class) do |corrector|
+            corrector.replace(node.parent_class, 'ActionDispatch::IntegrationTest')
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb
index 31404629a9..1b624b9fcf 100644
--- a/lib/rubocop/cop/rails_cops.rb
+++ b/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_testing'
 require_relative 'rails/action_filter'
 require_relative 'rails/active_record_aliases'
 require_relative 'rails/active_record_callbacks_order'
diff --git a/spec/rubocop/cop/rails/action_controller_testing_spec.rb b/spec/rubocop/cop/rails/action_controller_testing_spec.rb
new file mode 100644
index 0000000000..a93dec7acd
--- /dev/null
+++ b/spec/rubocop/cop/rails/action_controller_testing_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::ActionControllerTesting, :config do
+  context 'Rails 4.2', :rails42, :config 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