From 4f2ac51644754f9a4d4df697ac8984180b0bfdca Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Thu, 22 Aug 2019 11:42:16 +0200 Subject: 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. ``` --- rubocop/cop/rspec/be_success_matcher.rb | 50 +++++++++++++++ rubocop/rubocop.rb | 1 + rubocop/spec_helpers.rb | 46 ++++++++++++++ spec/rubocop/cop/rspec/be_success_matcher_spec.rb | 77 +++++++++++++++++++++++ 4 files changed, 174 insertions(+) create mode 100644 rubocop/cop/rspec/be_success_matcher.rb create mode 100644 rubocop/spec_helpers.rb create mode 100644 spec/rubocop/cop/rspec/be_success_matcher_spec.rb 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 -- cgit v1.2.1 From 3a71ab523ebe9e8d30aabbf12e659eb75036d0b5 Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Thu, 22 Aug 2019 11:57:33 +0200 Subject: Autocorrect `be_success` to `be_successful` --- rubocop/cop/rspec/be_success_matcher.rb | 6 ++++++ spec/rubocop/cop/rspec/be_success_matcher_spec.rb | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/rubocop/cop/rspec/be_success_matcher.rb b/rubocop/cop/rspec/be_success_matcher.rb index a137e2dba69..4ca1a7190b0 100644 --- a/rubocop/cop/rspec/be_success_matcher.rb +++ b/rubocop/cop/rspec/be_success_matcher.rb @@ -44,6 +44,12 @@ module RuboCop add_offense(node, location: :expression, message: MESSAGE) end + + def autocorrect(node) + lambda do |corrector| + corrector.insert_after(node.loc.expression, "ful") + end + end 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 index fcb5791ef57..0d86553af36 100644 --- a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb @@ -27,6 +27,14 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do end end + shared_examples 'an autocorrected be_success call' do |content, autocorrected_content| + it "registers an offense for `#{content}` and autocorrects it to `#{autocorrected_content}`" do + autocorrected = autocorrect_source(content, source_file) + + expect(autocorrected).to eql(autocorrected_content) + end + end + context 'in a controller spec file' do before do allow(cop).to receive(:in_controller_spec?).and_return(true) @@ -34,10 +42,12 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do context "using expect(response).to be_success call" do it_behaves_like 'an offensive be_success call', OFFENSE_CALL_EXPECT_TO_BE_SUCCESS + it_behaves_like 'an autocorrected be_success call', OFFENSE_CALL_EXPECT_TO_BE_SUCCESS, CALL_EXPECT_TO_BE_SUCCESSFUL 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 + it_behaves_like 'an autocorrected be_success call', OFFENSE_CALL_IS_EXPECTED_TO_BE_SUCCESS, CALL_IS_EXPECTED_TO_BE_SUCCESSFUL end context "using expect(response).to be_successful" do -- cgit v1.2.1 From d40b7ea37534e57d134b9a5e1268aa87d53c513d Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Thu, 22 Aug 2019 12:14:30 +0200 Subject: Enable frozen string literal --- rubocop/cop/rspec/be_success_matcher.rb | 2 ++ spec/rubocop/cop/rspec/be_success_matcher_spec.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/rubocop/cop/rspec/be_success_matcher.rb b/rubocop/cop/rspec/be_success_matcher.rb index 4ca1a7190b0..9ab344a3762 100644 --- a/rubocop/cop/rspec/be_success_matcher.rb +++ b/rubocop/cop/rspec/be_success_matcher.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../../spec_helpers' module RuboCop diff --git a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb index 0d86553af36..26044b1ef24 100644 --- a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' -- cgit v1.2.1 From e1b8b93207ccbe3221f5c8e0447d17582f14a0db Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Fri, 23 Aug 2019 09:33:33 +0200 Subject: Replace double quotes with single quotes --- rubocop/cop/rspec/be_success_matcher.rb | 4 ++-- spec/rubocop/cop/rspec/be_success_matcher_spec.rb | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/rubocop/cop/rspec/be_success_matcher.rb b/rubocop/cop/rspec/be_success_matcher.rb index 9ab344a3762..18bcfa6d582 100644 --- a/rubocop/cop/rspec/be_success_matcher.rb +++ b/rubocop/cop/rspec/be_success_matcher.rb @@ -26,7 +26,7 @@ module RuboCop class BeSuccessMatcher < RuboCop::Cop::Cop include SpecHelpers - MESSAGE = "Don't use deprecated `success?` method, use `successful?` instead.".freeze + MESSAGE = 'Do not 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)) @@ -49,7 +49,7 @@ module RuboCop def autocorrect(node) lambda do |corrector| - corrector.insert_after(node.loc.expression, "ful") + corrector.insert_after(node.loc.expression, 'ful') 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 index 26044b1ef24..b55e63ec7f0 100644 --- a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb @@ -42,17 +42,17 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do allow(cop).to receive(:in_controller_spec?).and_return(true) end - context "using expect(response).to be_success call" do + context 'using expect(response).to be_success call' do it_behaves_like 'an offensive be_success call', OFFENSE_CALL_EXPECT_TO_BE_SUCCESS it_behaves_like 'an autocorrected be_success call', OFFENSE_CALL_EXPECT_TO_BE_SUCCESS, CALL_EXPECT_TO_BE_SUCCESSFUL end - context "using is_expected.to be_success call" do + context 'using is_expected.to be_success call' do it_behaves_like 'an offensive be_success call', OFFENSE_CALL_IS_EXPECTED_TO_BE_SUCCESS it_behaves_like 'an autocorrected be_success call', OFFENSE_CALL_IS_EXPECTED_TO_BE_SUCCESS, CALL_IS_EXPECTED_TO_BE_SUCCESSFUL end - context "using expect(response).to be_successful" do + context 'using expect(response).to be_successful' do it "does not register an offense" do inspect_source(CALL_EXPECT_TO_BE_SUCCESSFUL) @@ -60,8 +60,8 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do end end - context "using is_expected.to be_successful" do - it "does not register an offense" do + 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) @@ -70,16 +70,16 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do 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 + 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 + 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) -- cgit v1.2.1 From 23da356dd1a048ca86c95960fb0b76df914f9087 Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Fri, 23 Aug 2019 10:57:46 +0200 Subject: Refactor BeSuccessMatcher specs --- spec/rubocop/cop/rspec/be_success_matcher_spec.rb | 61 ++++++++++------------- 1 file changed, 25 insertions(+), 36 deletions(-) diff --git a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb index b55e63ec7f0..9965b8ddf86 100644 --- a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb @@ -10,10 +10,16 @@ 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 + CODE_EXAMPLES = [ + { + bad: %(expect(response).to be_success).freeze, + good: %(expect(response).to be_successful).freeze + }, + { + bad: %(is_expected.to be_success).freeze, + good: %(is_expected.to be_successful).freeze + } + ] let(:source_file) { 'spec/foo_spec.rb' } @@ -42,47 +48,30 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher 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 - it_behaves_like 'an autocorrected be_success call', OFFENSE_CALL_EXPECT_TO_BE_SUCCESS, CALL_EXPECT_TO_BE_SUCCESSFUL - 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 - it_behaves_like 'an autocorrected be_success call', OFFENSE_CALL_IS_EXPECTED_TO_BE_SUCCESS, CALL_IS_EXPECTED_TO_BE_SUCCESSFUL - 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) + CODE_EXAMPLES.each do |code_example| + context "using #{code_example[:bad]} call" do + it_behaves_like 'an offensive be_success call', code_example[:bad] + it_behaves_like 'an autocorrected be_success call', code_example[:bad], code_example[:good] 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) + context "using #{code_example[:good]} call" do + it "does not register an offense" do + inspect_source(code_example[:good]) - expect(cop.offenses.size).to eq(0) + expect(cop.offenses.size).to eq(0) + end 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) + CODE_EXAMPLES.each do |code_example| + context "using #{code_example[:bad]} call" do + it 'does not register an offense' do + inspect_source(code_example[:bad]) - expect(cop.offenses.size).to eq(0) + expect(cop.offenses.size).to eq(0) + end end end end -- cgit v1.2.1 From b61d26f496a2041d7124fd3280031deda3cf5a43 Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Fri, 23 Aug 2019 11:05:50 +0200 Subject: Add support of not_to/to_not to BeSuccessMatcher BeSuccessMatcher now supports following examples: ``` expect(response).to be_success expect(response).to_not be_success expect(response).not_to be_success is_expected.to be_success is_expected.to_not be_success is_expected.not_to be_success ``` --- rubocop/cop/rspec/be_success_matcher.rb | 4 ++-- spec/rubocop/cop/rspec/be_success_matcher_spec.rb | 20 ++++++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/rubocop/cop/rspec/be_success_matcher.rb b/rubocop/cop/rspec/be_success_matcher.rb index 18bcfa6d582..ca937a8ee9f 100644 --- a/rubocop/cop/rspec/be_success_matcher.rb +++ b/rubocop/cop/rspec/be_success_matcher.rb @@ -29,11 +29,11 @@ module RuboCop MESSAGE = 'Do not 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)) + (send (send nil? :expect (send nil? ...)) {:to :not_to :to_not} (send nil? :be_success)) PATTERN def_node_search :is_expected_to_be_success?, <<~PATTERN - (send (send nil? :is_expected) :to (send nil? :be_success)) + (send (send nil? :is_expected) {:to :not_to :to_not} (send nil? :be_success)) PATTERN def be_success_usage?(node) diff --git a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb index 9965b8ddf86..95f08a61557 100644 --- a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb @@ -15,11 +15,27 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do bad: %(expect(response).to be_success).freeze, good: %(expect(response).to be_successful).freeze }, + { + bad: %(expect(response).to_not be_success).freeze, + good: %(expect(response).to_not be_successful).freeze + }, + { + bad: %(expect(response).not_to be_success).freeze, + good: %(expect(response).not_to be_successful).freeze + }, { bad: %(is_expected.to be_success).freeze, good: %(is_expected.to be_successful).freeze + }, + { + bad: %(is_expected.to_not be_success).freeze, + good: %(is_expected.to_not be_successful).freeze + }, + { + bad: %(is_expected.not_to be_success).freeze, + good: %(is_expected.not_to be_successful).freeze } - ] + ].freeze let(:source_file) { 'spec/foo_spec.rb' } @@ -55,7 +71,7 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do end context "using #{code_example[:good]} call" do - it "does not register an offense" do + it 'does not register an offense' do inspect_source(code_example[:good]) expect(cop.offenses.size).to eq(0) -- cgit v1.2.1 From 99b27e69510e83006d4dd2e5ecc5c555409aa4a0 Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Mon, 26 Aug 2019 10:04:54 +0200 Subject: Utilize Rubocop's Include for BeSuccessMatcher Use Rubocop's Include instead of manually checking the matcher in controllers specs. --- .rubocop.yml | 8 ++++++ rubocop/cop/rspec/be_success_matcher.rb | 1 - rubocop/spec_helpers.rb | 16 ----------- spec/rubocop/cop/rspec/be_success_matcher_spec.rb | 34 ++++++----------------- 4 files changed, 16 insertions(+), 43 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 012f4890c33..1319752fc9c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -265,3 +265,11 @@ RSpec/EnvAssignment: - 'ee/spec/**/rails_helper.rb' - 'spec/**/spec_helper.rb' - 'ee/spec/**/spec_helper.rb' +RSpec/BeSuccessMatcher: + Enabled: true + Include: + - 'spec/controllers/**/*' + - 'ee/spec/controllers/**/*' + - 'spec/support/shared_examples/controllers/**/*' + - 'ee/spec/support/shared_examples/controllers/**/*' + - 'spec/support/controllers/**/*' diff --git a/rubocop/cop/rspec/be_success_matcher.rb b/rubocop/cop/rspec/be_success_matcher.rb index ca937a8ee9f..0b7d737f4e2 100644 --- a/rubocop/cop/rspec/be_success_matcher.rb +++ b/rubocop/cop/rspec/be_success_matcher.rb @@ -41,7 +41,6 @@ module RuboCop end def on_send(node) - return unless in_controller_spec?(node) return unless be_success_usage?(node) add_offense(node, location: :expression, message: MESSAGE) diff --git a/rubocop/spec_helpers.rb b/rubocop/spec_helpers.rb index b38dc3f8cdb..ecd77c4351d 100644 --- a/rubocop/spec_helpers.rb +++ b/rubocop/spec_helpers.rb @@ -2,7 +2,6 @@ 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) @@ -27,20 +26,5 @@ module RuboCop 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 index 95f08a61557..77aff7c9dcc 100644 --- a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb @@ -59,35 +59,17 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do end end - context 'in a controller spec file' do - before do - allow(cop).to receive(:in_controller_spec?).and_return(true) + CODE_EXAMPLES.each do |code_example| + context "using #{code_example[:bad]} call" do + it_behaves_like 'an offensive be_success call', code_example[:bad] + it_behaves_like 'an autocorrected be_success call', code_example[:bad], code_example[:good] end - CODE_EXAMPLES.each do |code_example| - context "using #{code_example[:bad]} call" do - it_behaves_like 'an offensive be_success call', code_example[:bad] - it_behaves_like 'an autocorrected be_success call', code_example[:bad], code_example[:good] - end - - context "using #{code_example[:good]} call" do - it 'does not register an offense' do - inspect_source(code_example[:good]) - - expect(cop.offenses.size).to eq(0) - end - end - end - end - - context 'outside of a controller spec file' do - CODE_EXAMPLES.each do |code_example| - context "using #{code_example[:bad]} call" do - it 'does not register an offense' do - inspect_source(code_example[:bad]) + context "using #{code_example[:good]} call" do + it 'does not register an offense' do + inspect_source(code_example[:good]) - expect(cop.offenses.size).to eq(0) - end + expect(cop.offenses.size).to eq(0) end end end -- cgit v1.2.1 From 17385f4dc42d86d7606e3359b9db82cb43d5e534 Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Mon, 26 Aug 2019 15:13:41 +0200 Subject: Refactor BeSuccessMatcher specs for readability --- .rubocop.yml | 1 + rubocop/cop/rspec/be_success_matcher.rb | 2 +- spec/rubocop/cop/rspec/be_success_matcher_spec.rb | 65 +++++++++++------------ 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 1319752fc9c..573f2fbb6c6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -273,3 +273,4 @@ RSpec/BeSuccessMatcher: - 'spec/support/shared_examples/controllers/**/*' - 'ee/spec/support/shared_examples/controllers/**/*' - 'spec/support/controllers/**/*' + - 'ee/spec/support/controllers/**/*' diff --git a/rubocop/cop/rspec/be_success_matcher.rb b/rubocop/cop/rspec/be_success_matcher.rb index 0b7d737f4e2..07dad061613 100644 --- a/rubocop/cop/rspec/be_success_matcher.rb +++ b/rubocop/cop/rspec/be_success_matcher.rb @@ -26,7 +26,7 @@ module RuboCop class BeSuccessMatcher < RuboCop::Cop::Cop include SpecHelpers - MESSAGE = 'Do not use deprecated `success?` method, use `successful?` instead.'.freeze + MESSAGE = 'Do not use deprecated `success?` method, use `successful?` instead.' def_node_search :expect_to_be_success?, <<~PATTERN (send (send nil? :expect (send nil? ...)) {:to :not_to :to_not} (send nil? :be_success)) diff --git a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb index 77aff7c9dcc..0b4e9853dae 100644 --- a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb @@ -10,33 +10,6 @@ require_relative '../../../../rubocop/cop/rspec/be_success_matcher' describe RuboCop::Cop::RSpec::BeSuccessMatcher do include CopHelper - CODE_EXAMPLES = [ - { - bad: %(expect(response).to be_success).freeze, - good: %(expect(response).to be_successful).freeze - }, - { - bad: %(expect(response).to_not be_success).freeze, - good: %(expect(response).to_not be_successful).freeze - }, - { - bad: %(expect(response).not_to be_success).freeze, - good: %(expect(response).not_to be_successful).freeze - }, - { - bad: %(is_expected.to be_success).freeze, - good: %(is_expected.to be_successful).freeze - }, - { - bad: %(is_expected.to_not be_success).freeze, - good: %(is_expected.to_not be_successful).freeze - }, - { - bad: %(is_expected.not_to be_success).freeze, - good: %(is_expected.not_to be_successful).freeze - } - ].freeze - let(:source_file) { 'spec/foo_spec.rb' } subject(:cop) { described_class.new } @@ -59,18 +32,44 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do end end - CODE_EXAMPLES.each do |code_example| - context "using #{code_example[:bad]} call" do - it_behaves_like 'an offensive be_success call', code_example[:bad] - it_behaves_like 'an autocorrected be_success call', code_example[:bad], code_example[:good] + shared_examples 'cop' do |good:, bad:| + context "using #{bad} call" do + it_behaves_like 'an offensive be_success call', bad + it_behaves_like 'an autocorrected be_success call', bad, good end - context "using #{code_example[:good]} call" do + context "using #{good} call" do it 'does not register an offense' do - inspect_source(code_example[:good]) + inspect_source(good) expect(cop.offenses.size).to eq(0) end end end + + describe 'using different code examples' do + it_behaves_like 'cop', + bad: 'expect(response).to be_success', + good: 'expect(response).to be_successful' + + it_behaves_like 'cop', + bad: 'expect(response).to_not be_success', + good: 'expect(response).to_not be_successful' + + it_behaves_like 'cop', + bad: 'expect(response).not_to be_success', + good: 'expect(response).not_to be_successful' + + it_behaves_like 'cop', + bad: 'is_expected.to be_success', + good: 'is_expected.to be_successful' + + it_behaves_like 'cop', + bad: 'is_expected.to_not be_success', + good: 'is_expected.to_not be_successful' + + it_behaves_like 'cop', + bad: 'is_expected.not_to be_success', + good: 'is_expected.not_to be_successful' + end end -- cgit v1.2.1 From 0fee944741ab24071417230d0acc5dbc93e96a0b Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Tue, 27 Aug 2019 07:30:17 +0200 Subject: Inline shared examples for BeSuccessMatcher Former shared examples were used only once. Inlining them makes tests more clear. --- spec/rubocop/cop/rspec/be_success_matcher_spec.rb | 71 ++++++++++------------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb index 0b4e9853dae..a8512ae97ad 100644 --- a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb @@ -14,62 +14,53 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do 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 + shared_examples 'cop' do |good:, bad:| + context "using #{bad} call" do + it "registers an offense for `#{bad}`" do + inspect_source(bad, source_file) - shared_examples 'an autocorrected be_success call' do |content, autocorrected_content| - it "registers an offense for `#{content}` and autocorrects it to `#{autocorrected_content}`" do - autocorrected = autocorrect_source(content, source_file) + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + expect(cop.highlights).to eq([bad]) + end - expect(autocorrected).to eql(autocorrected_content) - end - end + it "registers an offense for `#{bad}` and autocorrects it to `#{good}`" do + autocorrected = autocorrect_source(bad, source_file) - shared_examples 'cop' do |good:, bad:| - context "using #{bad} call" do - it_behaves_like 'an offensive be_success call', bad - it_behaves_like 'an autocorrected be_success call', bad, good + expect(autocorrected).to eql(good) + end end context "using #{good} call" do it 'does not register an offense' do inspect_source(good) - expect(cop.offenses.size).to eq(0) + expect(cop.offenses).to be_empty end end end - describe 'using different code examples' do - it_behaves_like 'cop', - bad: 'expect(response).to be_success', - good: 'expect(response).to be_successful' + it_behaves_like 'cop', + bad: 'expect(response).to be_success', + good: 'expect(response).to be_successful' - it_behaves_like 'cop', - bad: 'expect(response).to_not be_success', - good: 'expect(response).to_not be_successful' + it_behaves_like 'cop', + bad: 'expect(response).to_not be_success', + good: 'expect(response).to_not be_successful' - it_behaves_like 'cop', - bad: 'expect(response).not_to be_success', - good: 'expect(response).not_to be_successful' + it_behaves_like 'cop', + bad: 'expect(response).not_to be_success', + good: 'expect(response).not_to be_successful' - it_behaves_like 'cop', - bad: 'is_expected.to be_success', - good: 'is_expected.to be_successful' + it_behaves_like 'cop', + bad: 'is_expected.to be_success', + good: 'is_expected.to be_successful' - it_behaves_like 'cop', - bad: 'is_expected.to_not be_success', - good: 'is_expected.to_not be_successful' + it_behaves_like 'cop', + bad: 'is_expected.to_not be_success', + good: 'is_expected.to_not be_successful' - it_behaves_like 'cop', - bad: 'is_expected.not_to be_success', - good: 'is_expected.not_to be_successful' - end + it_behaves_like 'cop', + bad: 'is_expected.not_to be_success', + good: 'is_expected.not_to be_successful' end -- cgit v1.2.1 From 69dbc5a5270a2b00bc2e359af37d6176748db400 Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Wed, 28 Aug 2019 08:48:14 +0200 Subject: Remove Rubocop::SpecHelper file --- rubocop/cop/rspec/be_success_matcher.rb | 4 --- rubocop/spec_helpers.rb | 30 ----------------------- spec/rubocop/cop/rspec/be_success_matcher_spec.rb | 3 --- 3 files changed, 37 deletions(-) delete mode 100644 rubocop/spec_helpers.rb diff --git a/rubocop/cop/rspec/be_success_matcher.rb b/rubocop/cop/rspec/be_success_matcher.rb index 07dad061613..dce9604b3d7 100644 --- a/rubocop/cop/rspec/be_success_matcher.rb +++ b/rubocop/cop/rspec/be_success_matcher.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require_relative '../../spec_helpers' - module RuboCop module Cop module RSpec @@ -24,8 +22,6 @@ module RuboCop # it { is_expected.to be_successful } # class BeSuccessMatcher < RuboCop::Cop::Cop - include SpecHelpers - MESSAGE = 'Do not use deprecated `success?` method, use `successful?` instead.' def_node_search :expect_to_be_success?, <<~PATTERN diff --git a/rubocop/spec_helpers.rb b/rubocop/spec_helpers.rb deleted file mode 100644 index ecd77c4351d..00000000000 --- a/rubocop/spec_helpers.rb +++ /dev/null @@ -1,30 +0,0 @@ -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 - - # 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 - end -end diff --git a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb index a8512ae97ad..17408a696de 100644 --- a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb @@ -2,9 +2,6 @@ require 'spec_helper' -require 'rubocop' -require 'rubocop/rspec/support' - require_relative '../../../../rubocop/cop/rspec/be_success_matcher' describe RuboCop::Cop::RSpec::BeSuccessMatcher do -- cgit v1.2.1 From 24493c50232b6109bbcc680ada32dfe832201a67 Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Wed, 28 Aug 2019 08:55:46 +0200 Subject: Replace it_behaves_like with include_examples Improve specs output readability by replacing `it_behaves_like` with `include_examples` in `BeSuccessMatcher` --- spec/rubocop/cop/rspec/be_success_matcher_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb index 17408a696de..12aa7d1643e 100644 --- a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb @@ -13,7 +13,7 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do shared_examples 'cop' do |good:, bad:| context "using #{bad} call" do - it "registers an offense for `#{bad}`" do + it 'registers an offense' do inspect_source(bad, source_file) expect(cop.offenses.size).to eq(1) @@ -21,7 +21,7 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do expect(cop.highlights).to eq([bad]) end - it "registers an offense for `#{bad}` and autocorrects it to `#{good}`" do + it "autocorrects it to `#{good}`" do autocorrected = autocorrect_source(bad, source_file) expect(autocorrected).to eql(good) @@ -37,27 +37,27 @@ describe RuboCop::Cop::RSpec::BeSuccessMatcher do end end - it_behaves_like 'cop', + include_examples 'cop', bad: 'expect(response).to be_success', good: 'expect(response).to be_successful' - it_behaves_like 'cop', + include_examples 'cop', bad: 'expect(response).to_not be_success', good: 'expect(response).to_not be_successful' - it_behaves_like 'cop', + include_examples 'cop', bad: 'expect(response).not_to be_success', good: 'expect(response).not_to be_successful' - it_behaves_like 'cop', + include_examples 'cop', bad: 'is_expected.to be_success', good: 'is_expected.to be_successful' - it_behaves_like 'cop', + include_examples 'cop', bad: 'is_expected.to_not be_success', good: 'is_expected.to_not be_successful' - it_behaves_like 'cop', + include_examples 'cop', bad: 'is_expected.not_to be_success', good: 'is_expected.not_to be_successful' end -- cgit v1.2.1