Commit 4f2ac516 authored by Vitali Tatarintev's avatar Vitali Tatarintev

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.
```
parent be3ec705
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
......@@ -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'
......
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
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment