Add Rubocop check to avoid using `be_success`

Prevent using `be_success` call in controller specs to avoid
getting following deprecation warning:

```
DEPRECATION WARNING: The success? predicate is deprecated and
will be removed in Rails 6.0.
Please use successful? as provided by Rack::Response::Helpers.
```

4 files changed, 174 insertions, 0 deletions

diff --git a/rubocop/cop/rspec/be_success_matcher.rb b/rubocop/cop/rspec/be_success_matcher.rb
new file mode 100644
index 00000000000..a137e2dba69
--- /dev/null
+++ b/rubocop/cop/rspec/be_success_matcher.rb
@@ -0,0 +1,50 @@
+require_relative '../../spec_helpers'
+
+module RuboCop
+  module Cop
+    module RSpec
+      # This cop checks for `be_success` usage in controller specs
+      #
+      # @example
+      #
+      #   # bad
+      #   it "responds with success" do
+      #     expect(response).to be_success
+      #   end
+      #
+      #   it { is_expected.to be_success }
+      #
+      #   # good
+      #   it "responds with success" do
+      #     expect(response).to be_successful
+      #   end
+      #
+      #   it { is_expected.to be_successful }
+      #
+      class BeSuccessMatcher < RuboCop::Cop::Cop
+        include SpecHelpers
+
+        MESSAGE = "Don't use deprecated `success?` method, use `successful?` instead.".freeze
+
+        def_node_search :expect_to_be_success?, <<~PATTERN
+          (send (send nil? :expect (send nil? ...)) :to (send nil? :be_success))
+        PATTERN
+
+        def_node_search :is_expected_to_be_success?, <<~PATTERN
+          (send (send nil? :is_expected) :to (send nil? :be_success))
+        PATTERN
+
+        def be_success_usage?(node)
+          expect_to_be_success?(node) || is_expected_to_be_success?(node)
+        end
+
+        def on_send(node)
+          return unless in_controller_spec?(node)
+          return unless be_success_usage?(node)
+
+          add_offense(node, location: :expression, message: MESSAGE)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index d1328c4eb38..c342df6d6c9 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -31,6 +31,7 @@ require_relative 'cop/migration/timestamps'
 require_relative 'cop/migration/update_column_in_batches'
 require_relative 'cop/migration/update_large_table'
 require_relative 'cop/project_path_helper'
+require_relative 'cop/rspec/be_success_matcher'
 require_relative 'cop/rspec/env_assignment'
 require_relative 'cop/rspec/factories_in_migration_specs'
 require_relative 'cop/rspec/top_level_describe_path'
diff --git a/rubocop/spec_helpers.rb b/rubocop/spec_helpers.rb
new file mode 100644
index 00000000000..b38dc3f8cdb
--- /dev/null
+++ b/rubocop/spec_helpers.rb
@@ -0,0 +1,46 @@
+module RuboCop
+  module SpecHelpers
+    SPEC_HELPERS = %w[fast_spec_helper.rb rails_helper.rb spec_helper.rb].freeze
+    MIGRATION_SPEC_DIRECTORIES = ['spec/migrations', 'spec/lib/gitlab/background_migration'].freeze
+    CONTROLLER_SPEC_DIRECTORIES = ['spec/controllers', 'spec/support/shared_examples/controllers'].freeze
+
+    # Returns true if the given node originated from the spec directory.
+    def in_spec?(node)
+      path = node.location.expression.source_buffer.name
+      pwd = RuboCop::PathUtil.pwd
+
+      !SPEC_HELPERS.include?(File.basename(path)) &&
+        path.start_with?(File.join(pwd, 'spec'), File.join(pwd, 'ee', 'spec'))
+    end
+
+    def migration_directories
+      @migration_directories ||= MIGRATION_SPEC_DIRECTORIES.map do |dir|
+        pwd = RuboCop::PathUtil.pwd
+        [File.join(pwd, dir), File.join(pwd, 'ee', dir)]
+      end.flatten
+    end
+
+    # Returns true if the given node originated from a migration spec.
+    def in_migration_spec?(node)
+      path = node.location.expression.source_buffer.name
+
+      in_spec?(node) &&
+        path.start_with?(*migration_directories)
+    end
+
+    def controller_directories
+      @controller_directories ||= CONTROLLER_SPEC_DIRECTORIES.map do |dir|
+        pwd = RuboCop::PathUtil.pwd
+        [File.join(pwd, dir), File.join(pwd, 'ee', dir)]
+      end.flatten
+    end
+
+    # Returns true if the given node originated from a controller spec.
+    def in_controller_spec?(node)
+      path = node.location.expression.source_buffer.name
+
+      in_spec?(node) &&
+        path.start_with?(*controller_directories)
+    end
+  end
+end
diff --git a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb
new file mode 100644
index 00000000000..fcb5791ef57
--- /dev/null
+++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb
@@ -0,0 +1,77 @@
+require 'spec_helper'
+
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../../rubocop/cop/rspec/be_success_matcher'
+
+describe RuboCop::Cop::RSpec::BeSuccessMatcher do
+  include CopHelper
+
+  OFFENSE_CALL_EXPECT_TO_BE_SUCCESS = %(expect(response).to be_success).freeze
+  OFFENSE_CALL_IS_EXPECTED_TO_BE_SUCCESS = %(is_expected.to be_success).freeze
+  CALL_EXPECT_TO_BE_SUCCESSFUL = %(expect(response).to be_successful).freeze
+  CALL_IS_EXPECTED_TO_BE_SUCCESSFUL = %(is_expected.to be_successful).freeze
+
+  let(:source_file) { 'spec/foo_spec.rb' }
+
+  subject(:cop) { described_class.new }
+
+  shared_examples 'an offensive be_success call' do |content|
+    it "registers an offense for `#{content}`" do
+      inspect_source(content, source_file)
+
+      expect(cop.offenses.size).to eq(1)
+      expect(cop.offenses.map(&:line)).to eq([1])
+      expect(cop.highlights).to eq([content])
+    end
+  end
+
+  context 'in a controller spec file' do
+    before do
+      allow(cop).to receive(:in_controller_spec?).and_return(true)
+    end
+
+    context "using expect(response).to be_success call" do
+      it_behaves_like 'an offensive be_success call', OFFENSE_CALL_EXPECT_TO_BE_SUCCESS
+    end
+
+    context "using is_expected.to be_success call" do
+      it_behaves_like 'an offensive be_success call', OFFENSE_CALL_IS_EXPECTED_TO_BE_SUCCESS
+    end
+
+    context "using expect(response).to be_successful" do
+      it "does not register an offense" do
+        inspect_source(CALL_EXPECT_TO_BE_SUCCESSFUL)
+
+        expect(cop.offenses.size).to eq(0)
+      end
+    end
+
+    context "using is_expected.to be_successful" do
+      it "does not register an offense" do
+        inspect_source(CALL_IS_EXPECTED_TO_BE_SUCCESSFUL)
+
+        expect(cop.offenses.size).to eq(0)
+      end
+    end
+  end
+
+  context 'outside of a controller spec file' do
+    context "using expect(response).to be_success call" do
+      it "does not register an offense" do
+        inspect_source(OFFENSE_CALL_EXPECT_TO_BE_SUCCESS)
+
+        expect(cop.offenses.size).to eq(0)
+      end
+    end
+
+    context "using is_expected.to be_success call" do
+      it "does not register an offense" do
+        inspect_source(OFFENSE_CALL_IS_EXPECTED_TO_BE_SUCCESS)
+
+        expect(cop.offenses.size).to eq(0)
+      end
+    end
+  end
+end