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: '<>' + 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