Commit c5ed7ffc authored by Doug Stull's avatar Doug Stull Committed by Luke Duncalfe

Resolve cop helper use exceptions

- align with standards in rubocop specs.
parent 5c5650e9
...@@ -29,31 +29,8 @@ FactoryBot/InlineAssociation: ...@@ -29,31 +29,8 @@ FactoryBot/InlineAssociation:
- 'spec/factories/uploads.rb' - 'spec/factories/uploads.rb'
- 'spec/factories/wiki_pages.rb' - 'spec/factories/wiki_pages.rb'
InternalAffairs/DeprecateCopHelper: InternalAffairs/DeprecateCopHelper: # issue to resolve: https://gitlab.com/gitlab-org/gitlab/-/issues/276734
Exclude: Exclude:
- 'spec/rubocop/cop/scalability/bulk_perform_with_context_spec.rb'
- 'spec/rubocop/cop/scalability/idempotent_worker_spec.rb'
- 'spec/rubocop/cop/scalability/cron_worker_context_spec.rb'
- 'spec/rubocop/cop/scalability/file_uploads_spec.rb'
- 'spec/rubocop/cop/destroy_all_spec.rb'
- 'spec/rubocop/cop/safe_params_spec.rb'
- 'spec/rubocop/cop/include_sidekiq_worker_spec.rb'
- 'spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb'
- 'spec/rubocop/cop/rspec/timecop_freeze_spec.rb'
- 'spec/rubocop/cop/rspec/any_instance_of_spec.rb'
- 'spec/rubocop/cop/rspec/factory_bot/inline_association_spec.rb'
- 'spec/rubocop/cop/rspec/top_level_describe_path_spec.rb'
- 'spec/rubocop/cop/rspec/be_success_matcher_spec.rb'
- 'spec/rubocop/cop/rspec/htt_party_basic_auth_spec.rb'
- 'spec/rubocop/cop/rspec/modify_sidekiq_middleware_spec.rb'
- 'spec/rubocop/cop/rspec/expect_gitlab_tracking_spec.rb'
- 'spec/rubocop/cop/rspec/have_gitlab_http_status_spec.rb'
- 'spec/rubocop/cop/rspec/timecop_travel_spec.rb'
- 'spec/rubocop/cop/rspec/env_assignment_spec.rb'
- 'spec/rubocop/cop/performance/ar_exists_and_present_blank_spec.rb'
- 'spec/rubocop/cop/performance/ar_count_each_spec.rb'
- 'spec/rubocop/cop/performance/readlines_each_spec.rb'
- 'spec/rubocop/cop/project_path_helper_spec.rb'
- 'spec/rubocop/cop/migration/safer_boolean_column_spec.rb' - 'spec/rubocop/cop/migration/safer_boolean_column_spec.rb'
- 'spec/rubocop/cop/migration/remove_index_spec.rb' - 'spec/rubocop/cop/migration/remove_index_spec.rb'
- 'spec/rubocop/cop/migration/add_index_spec.rb' - 'spec/rubocop/cop/migration/add_index_spec.rb'
......
...@@ -2,44 +2,41 @@ ...@@ -2,44 +2,41 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/destroy_all' require_relative '../../../rubocop/cop/destroy_all'
RSpec.describe RuboCop::Cop::DestroyAll do RSpec.describe RuboCop::Cop::DestroyAll do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
it 'flags the use of destroy_all with a send receiver' do it 'flags the use of destroy_all with a send receiver' do
inspect_source('foo.destroy_all # rubocop: disable Cop/DestroyAll') expect_offense(<<~CODE)
foo.destroy_all # rubocop: disable Cop/DestroyAll
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^^^^ Use `delete_all` instead of `destroy_all`. [...]
CODE
end end
it 'flags the use of destroy_all with a constant receiver' do it 'flags the use of destroy_all with a constant receiver' do
inspect_source('User.destroy_all # rubocop: disable Cop/DestroyAll') expect_offense(<<~CODE)
User.destroy_all # rubocop: disable Cop/DestroyAll
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^^^^^ Use `delete_all` instead of `destroy_all`. [...]
CODE
end end
it 'flags the use of destroy_all when passing arguments' do it 'flags the use of destroy_all when passing arguments' do
inspect_source('User.destroy_all([])') expect_offense(<<~CODE)
User.destroy_all([])
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^^^^^^^^^ Use `delete_all` instead of `destroy_all`. [...]
CODE
end end
it 'flags the use of destroy_all with a local variable receiver' do it 'flags the use of destroy_all with a local variable receiver' do
inspect_source(<<~RUBY) expect_offense(<<~CODE)
users = User.all users = User.all
users.destroy_all # rubocop: disable Cop/DestroyAll users.destroy_all # rubocop: disable Cop/DestroyAll
RUBY ^^^^^^^^^^^^^^^^^ Use `delete_all` instead of `destroy_all`. [...]
CODE
expect(cop.offenses.size).to eq(1)
end end
it 'does not flag the use of delete_all' do it 'does not flag the use of delete_all' do
inspect_source('foo.delete_all') expect_no_offenses('foo.delete_all')
expect(cop.offenses).to be_empty
end end
end end
...@@ -3,33 +3,21 @@ ...@@ -3,33 +3,21 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/include_sidekiq_worker' require_relative '../../../rubocop/cop/include_sidekiq_worker'
RSpec.describe RuboCop::Cop::IncludeSidekiqWorker do RSpec.describe RuboCop::Cop::IncludeSidekiqWorker do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'when `Sidekiq::Worker` is included' do context 'when `Sidekiq::Worker` is included' do
let(:source) { 'include Sidekiq::Worker' } it 'registers an offense and corrects', :aggregate_failures do
let(:correct_source) { 'include ApplicationWorker' } expect_offense(<<~CODE)
include Sidekiq::Worker
it 'registers an offense' do ^^^^^^^^^^^^^^^ Include `ApplicationWorker`, not `Sidekiq::Worker`.
inspect_source(source) CODE
aggregate_failures do expect_correction(<<~CODE)
expect(cop.offenses.size).to eq(1) include ApplicationWorker
expect(cop.offenses.map(&:line)).to eq([1]) CODE
expect(cop.highlights).to eq(['Sidekiq::Worker'])
end
end
it 'autocorrects to the right version' do
autocorrected = autocorrect_source(source)
expect(autocorrected).to eq(correct_source)
end end
end end
end end
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/performance/ar_count_each.rb' require_relative '../../../../rubocop/cop/performance/ar_count_each.rb'
RSpec.describe RuboCop::Cop::Performance::ARCountEach do RSpec.describe RuboCop::Cop::Performance::ARCountEach do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'when it is not haml file' do context 'when it is not haml file' do
...@@ -32,8 +30,6 @@ RSpec.describe RuboCop::Cop::Performance::ARCountEach do ...@@ -32,8 +30,6 @@ RSpec.describe RuboCop::Cop::Performance::ARCountEach do
^^^^^^^^^^^^ If @users is AR relation, avoid `@users.count ...; @users.each... `, this will trigger two queries. Use `@users.load.size ...; @users.each... ` instead. If @users is an array, try to use @users.size. ^^^^^^^^^^^^ If @users is AR relation, avoid `@users.count ...; @users.each... `, this will trigger two queries. Use `@users.load.size ...; @users.each... ` instead. If @users is an array, try to use @users.size.
@users.each { |user| display(user) } @users.each { |user| display(user) }
SOURCE SOURCE
expect(cop.offenses.map(&:cop_name)).to contain_exactly('Performance/ARCountEach')
end end
end end
......
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/performance/ar_exists_and_present_blank.rb' require_relative '../../../../rubocop/cop/performance/ar_exists_and_present_blank.rb'
RSpec.describe RuboCop::Cop::Performance::ARExistsAndPresentBlank do RSpec.describe RuboCop::Cop::Performance::ARExistsAndPresentBlank do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'when it is not haml file' do context 'when it is not haml file' do
...@@ -32,8 +30,6 @@ RSpec.describe RuboCop::Cop::Performance::ARExistsAndPresentBlank do ...@@ -32,8 +30,6 @@ RSpec.describe RuboCop::Cop::Performance::ARExistsAndPresentBlank do
show @users if @users.present? show @users if @users.present?
^^^^^^^^^^^^^^^ Avoid `@users.present?`, because it will generate database query 'Select TABLE.*' which is expensive. Suggest to use `@users.any?` to replace `@users.present?` ^^^^^^^^^^^^^^^ Avoid `@users.present?`, because it will generate database query 'Select TABLE.*' which is expensive. Suggest to use `@users.any?` to replace `@users.present?`
SOURCE SOURCE
expect(cop.offenses.map(&:cop_name)).to contain_exactly('Performance/ARExistsAndPresentBlank')
end end
end end
...@@ -44,8 +40,6 @@ RSpec.describe RuboCop::Cop::Performance::ARExistsAndPresentBlank do ...@@ -44,8 +40,6 @@ RSpec.describe RuboCop::Cop::Performance::ARExistsAndPresentBlank do
show @users if @users.blank? show @users if @users.blank?
^^^^^^^^^^^^^ Avoid `@users.blank?`, because it will generate database query 'Select TABLE.*' which is expensive. Suggest to use `@users.empty?` to replace `@users.blank?` ^^^^^^^^^^^^^ Avoid `@users.blank?`, because it will generate database query 'Select TABLE.*' which is expensive. Suggest to use `@users.empty?` to replace `@users.blank?`
SOURCE SOURCE
expect(cop.offenses.map(&:cop_name)).to contain_exactly('Performance/ARExistsAndPresentBlank')
end end
end end
...@@ -58,8 +52,6 @@ RSpec.describe RuboCop::Cop::Performance::ARExistsAndPresentBlank do ...@@ -58,8 +52,6 @@ RSpec.describe RuboCop::Cop::Performance::ARExistsAndPresentBlank do
show @users if @users.present? show @users if @users.present?
^^^^^^^^^^^^^^^ Avoid `@users.present?`, because it will generate database query 'Select TABLE.*' which is expensive. Suggest to use `@users.any?` to replace `@users.present?` ^^^^^^^^^^^^^^^ Avoid `@users.present?`, because it will generate database query 'Select TABLE.*' which is expensive. Suggest to use `@users.any?` to replace `@users.present?`
SOURCE SOURCE
expect(cop.offenses.map(&:cop_name)).to contain_exactly('Performance/ARExistsAndPresentBlank', 'Performance/ARExistsAndPresentBlank')
end end
end end
......
...@@ -5,19 +5,21 @@ require 'rubocop' ...@@ -5,19 +5,21 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/performance/readlines_each' require_relative '../../../../rubocop/cop/performance/readlines_each'
RSpec.describe RuboCop::Cop::Performance::ReadlinesEach do RSpec.describe RuboCop::Cop::Performance::ReadlinesEach do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
let(:message) { 'Avoid `IO.readlines.each`, since it reads contents into memory in full. Use `IO.each_line` or `IO.each` instead.' } let(:message) { 'Avoid `IO.readlines.each`, since it reads contents into memory in full. Use `IO.each_line` or `IO.each` instead.' }
shared_examples_for(:class_read) do |klass| shared_examples_for(:class_read) do |klass|
context "and it is called as a class method on #{klass}" do context "and it is called as a class method on #{klass}" do
# We can't use `expect_offense` here because indentation changes based on `klass`
it 'flags it as an offense' do it 'flags it as an offense' do
inspect_source "#{klass}.readlines(file_path).each { |line| puts line }" leading_readline = "#{klass}.readlines(file_path)."
padding = " " * leading_readline.length
node = "#{leading_readline}each { |line| puts line }"
expect(cop.offenses.map(&:cop_name)).to contain_exactly('Performance/ReadlinesEach') expect_offense(<<~CODE, node: node)
%{node}
#{padding}^^^^ Avoid `IO.readlines.each`, since it reads contents into memory in full. [...]
CODE
end end
end end
...@@ -62,9 +64,14 @@ RSpec.describe RuboCop::Cop::Performance::ReadlinesEach do ...@@ -62,9 +64,14 @@ RSpec.describe RuboCop::Cop::Performance::ReadlinesEach do
end end
it 'autocorrects `readlines.each` to `each_line`' do it 'autocorrects `readlines.each` to `each_line`' do
expect(autocorrect_source('obj.readlines.each { |line| line }')).to( expect_offense(<<~CODE)
eq('obj.each_line { |line| line }') obj.readlines.each { |line| line }
) ^^^^ Avoid `IO.readlines.each`, since it reads contents into memory in full. [...]
CODE
expect_correction(<<~CODE)
obj.each_line { |line| line }
CODE
end end
end end
......
...@@ -3,41 +3,30 @@ ...@@ -3,41 +3,30 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/project_path_helper' require_relative '../../../rubocop/cop/project_path_helper'
RSpec.describe RuboCop::Cop::ProjectPathHelper do RSpec.describe RuboCop::Cop::ProjectPathHelper do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context "when using namespace_project with the project's namespace" do context "when using namespace_project with the project's namespace" do
let(:source) { 'edit_namespace_project_issue_path(@issue.project.namespace, @issue.project, @issue)' } let(:source) { 'edit_namespace_project_issue_path(@issue.project.namespace, @issue.project, @issue)' }
let(:correct_source) { 'edit_project_issue_path(@issue.project, @issue)' } let(:correct_source) { 'edit_project_issue_path(@issue.project, @issue)' }
it 'registers an offense' do it 'registers an offense and corrects', :aggregate_failures do
inspect_source(source) expect_offense(<<~CODE)
#{source}
aggregate_failures do ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use short project path helpers without explicitly passing the namespace[...]
expect(cop.offenses.size).to eq(1) CODE
expect(cop.offenses.map(&:line)).to eq([1])
expect(cop.highlights).to eq(['edit_namespace_project_issue_path'])
end
end
it 'autocorrects to the right version' do
autocorrected = autocorrect_source(source)
expect(autocorrected).to eq(correct_source) expect_correction(<<~CODE)
#{correct_source}
CODE
end end
end end
context 'when using namespace_project with a different namespace' do context 'when using namespace_project with a different namespace' do
it 'registers no offense' do it 'registers no offense' do
inspect_source('edit_namespace_project_issue_path(namespace, project)') expect_no_offenses('edit_namespace_project_issue_path(namespace, project)')
expect(cop.offenses.size).to eq(0)
end end
end end
end end
...@@ -5,59 +5,51 @@ require 'fast_spec_helper' ...@@ -5,59 +5,51 @@ require 'fast_spec_helper'
require_relative '../../../../rubocop/cop/rspec/any_instance_of' require_relative '../../../../rubocop/cop/rspec/any_instance_of'
RSpec.describe RuboCop::Cop::RSpec::AnyInstanceOf do RSpec.describe RuboCop::Cop::RSpec::AnyInstanceOf do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'when calling allow_any_instance_of' do context 'when calling allow_any_instance_of' do
let(:source) do let(:source) do
<<~SRC <<~SRC
allow_any_instance_of(User).to receive(:invalidate_issue_cache_counts) allow_any_instance_of(User).to receive(:invalidate_issue_cache_counts)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `allow_any_instance_of` [...]
SRC SRC
end end
let(:corrected_source) do let(:corrected_source) do
<<~SRC <<~SRC
allow_next_instance_of(User) do |instance| allow_next_instance_of(User) do |instance|
allow(instance).to receive(:invalidate_issue_cache_counts) allow(instance).to receive(:invalidate_issue_cache_counts)
end end
SRC SRC
end end
it 'registers an offence' do it 'registers an offense and corrects', :aggregate_failures do
inspect_source(source) expect_offense(source)
expect(cop.offenses.size).to eq(1)
end
it 'can autocorrect the source' do expect_correction(corrected_source)
expect(autocorrect_source(source)).to eq(corrected_source)
end end
end end
context 'when calling expect_any_instance_of' do context 'when calling expect_any_instance_of' do
let(:source) do let(:source) do
<<~SRC <<~SRC
expect_any_instance_of(User).to receive(:invalidate_issue_cache_counts).with(args).and_return(double) expect_any_instance_of(User).to receive(:invalidate_issue_cache_counts).with(args).and_return(double)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `expect_any_instance_of` [...]
SRC SRC
end end
let(:corrected_source) do let(:corrected_source) do
<<~SRC <<~SRC
expect_next_instance_of(User) do |instance| expect_next_instance_of(User) do |instance|
expect(instance).to receive(:invalidate_issue_cache_counts).with(args).and_return(double) expect(instance).to receive(:invalidate_issue_cache_counts).with(args).and_return(double)
end end
SRC SRC
end end
it 'registers an offence' do it 'registers an offense and corrects', :aggregate_failures do
inspect_source(source) expect_offense(source)
expect(cop.offenses.size).to eq(1)
end
it 'can autocorrect the source' do expect_correction(corrected_source)
expect(autocorrect_source(source)).to eq(corrected_source)
end end
end end
end end
...@@ -5,34 +5,27 @@ require 'rubocop' ...@@ -5,34 +5,27 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/rspec/be_success_matcher' require_relative '../../../../rubocop/cop/rspec/be_success_matcher'
RSpec.describe RuboCop::Cop::RSpec::BeSuccessMatcher do RSpec.describe RuboCop::Cop::RSpec::BeSuccessMatcher do
include CopHelper
let(:source_file) { 'spec/foo_spec.rb' } let(:source_file) { 'spec/foo_spec.rb' }
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
shared_examples 'cop' do |good:, bad:| shared_examples 'cop' do |good:, bad:|
context "using #{bad} call" do context "using #{bad} call" do
it 'registers an offense' do it 'registers an offense and corrects', :aggregate_failures do
inspect_source(bad, source_file) expect_offense(<<~CODE, node: bad)
%{node}
expect(cop.offenses.size).to eq(1) ^{node} Do not use deprecated `success?` method, use `successful?` instead.
expect(cop.offenses.map(&:line)).to eq([1]) CODE
expect(cop.highlights).to eq([bad])
end expect_correction(<<~CODE)
#{good}
it "autocorrects it to `#{good}`" do CODE
autocorrected = autocorrect_source(bad, source_file)
expect(autocorrected).to eql(good)
end end
end end
context "using #{good} call" do context "using #{good} call" do
it 'does not register an offense' do it 'does not register an offense' do
inspect_source(good) expect_no_offenses(good)
expect(cop.offenses).to be_empty
end end
end end
end end
......
...@@ -3,13 +3,9 @@ ...@@ -3,13 +3,9 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/rspec/env_assignment' require_relative '../../../../rubocop/cop/rspec/env_assignment'
RSpec.describe RuboCop::Cop::RSpec::EnvAssignment do RSpec.describe RuboCop::Cop::RSpec::EnvAssignment do
include CopHelper
offense_call_single_quotes_key = %(ENV['FOO'] = 'bar').freeze offense_call_single_quotes_key = %(ENV['FOO'] = 'bar').freeze
offense_call_double_quotes_key = %(ENV["FOO"] = 'bar').freeze offense_call_double_quotes_key = %(ENV["FOO"] = 'bar').freeze
...@@ -17,31 +13,24 @@ RSpec.describe RuboCop::Cop::RSpec::EnvAssignment do ...@@ -17,31 +13,24 @@ RSpec.describe RuboCop::Cop::RSpec::EnvAssignment do
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
shared_examples 'an offensive ENV#[]= call' do |content| shared_examples 'an offensive and correction ENV#[]= call' do |content, autocorrected_content|
it "registers an offense for `#{content}`" do it "registers an offense for `#{content}` and corrects", :aggregate_failures do
inspect_source(content, source_file) expect_offense(<<~CODE)
#{content}
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^^^^^^^ Don't assign to ENV, use `stub_env` instead.
expect(cop.offenses.map(&:line)).to eq([1]) CODE
expect(cop.highlights).to eq([content])
end
end
shared_examples 'an autocorrected ENV#[]= 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) expect_correction(<<~CODE)
#{autocorrected_content}
CODE
end end
end end
context 'with a key using single quotes' do context 'with a key using single quotes' do
it_behaves_like 'an offensive ENV#[]= call', offense_call_single_quotes_key it_behaves_like 'an offensive and correction ENV#[]= call', offense_call_single_quotes_key, %(stub_env('FOO', 'bar'))
it_behaves_like 'an autocorrected ENV#[]= call', offense_call_single_quotes_key, %(stub_env('FOO', 'bar'))
end end
context 'with a key using double quotes' do context 'with a key using double quotes' do
it_behaves_like 'an offensive ENV#[]= call', offense_call_double_quotes_key it_behaves_like 'an offensive and correction ENV#[]= call', offense_call_double_quotes_key, %(stub_env("FOO", 'bar'))
it_behaves_like 'an autocorrected ENV#[]= call', offense_call_double_quotes_key, %(stub_env("FOO", 'bar'))
end end
end end
...@@ -2,13 +2,9 @@ ...@@ -2,13 +2,9 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/rspec/expect_gitlab_tracking' require_relative '../../../../rubocop/cop/rspec/expect_gitlab_tracking'
RSpec.describe RuboCop::Cop::RSpec::ExpectGitlabTracking do RSpec.describe RuboCop::Cop::RSpec::ExpectGitlabTracking do
include CopHelper
let(:source_file) { 'spec/foo_spec.rb' } let(:source_file) { 'spec/foo_spec.rb' }
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
...@@ -36,29 +32,18 @@ RSpec.describe RuboCop::Cop::RSpec::ExpectGitlabTracking do ...@@ -36,29 +32,18 @@ RSpec.describe RuboCop::Cop::RSpec::ExpectGitlabTracking do
good_samples.each do |good| good_samples.each do |good|
context "good: #{good}" do context "good: #{good}" do
it 'does not register an offense' do it 'does not register an offense' do
inspect_source(good) expect_no_offenses(good)
expect(cop.offenses).to be_empty
end end
end end
end end
bad_samples.each do |bad| bad_samples.each do |bad|
context "bad: #{bad}" do context "bad: #{bad}" do
it 'registers an offense', :aggregate_failures do it 'registers an offense' do
inspect_source(bad, source_file) expect_offense(<<~CODE, node: bad)
%{node}
expect(cop.offenses.size).to eq(1) ^{node} Do not expect directly on `Gitlab::Tracking#event`[...]
expect(cop.offenses.map(&:line)).to eq([1]) CODE
expect(cop.highlights).to eq([bad])
msg = cop.offenses.first.message
expect(msg).to match(
/Do not expect directly on `Gitlab::Tracking#event`/
)
expect(msg).to match(/add the `snowplow` annotation/)
expect(msg).to match(/use `expect_snowplow_event` instead/)
end end
end end
end end
......
...@@ -3,13 +3,9 @@ ...@@ -3,13 +3,9 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/rspec/factories_in_migration_specs' require_relative '../../../../rubocop/cop/rspec/factories_in_migration_specs'
RSpec.describe RuboCop::Cop::RSpec::FactoriesInMigrationSpecs do RSpec.describe RuboCop::Cop::RSpec::FactoriesInMigrationSpecs do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
shared_examples 'an offensive factory call' do |namespace| shared_examples 'an offensive factory call' do |namespace|
......
...@@ -7,8 +7,6 @@ require 'rubocop' ...@@ -7,8 +7,6 @@ require 'rubocop'
require_relative '../../../../../rubocop/cop/rspec/factory_bot/inline_association' require_relative '../../../../../rubocop/cop/rspec/factory_bot/inline_association'
RSpec.describe RuboCop::Cop::RSpec::FactoryBot::InlineAssociation do RSpec.describe RuboCop::Cop::RSpec::FactoryBot::InlineAssociation do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
shared_examples 'offense' do |code_snippet, autocorrected| shared_examples 'offense' do |code_snippet, autocorrected|
...@@ -17,27 +15,31 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::InlineAssociation do ...@@ -17,27 +15,31 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::InlineAssociation do
let(:offense_marker) { '^' * code_snippet.size } let(:offense_marker) { '^' * code_snippet.size }
let(:offense_msg) { msg(type) } let(:offense_msg) { msg(type) }
let(:offense) { "#{offense_marker} #{offense_msg}" } let(:offense) { "#{offense_marker} #{offense_msg}" }
let(:pristine_source) { source.sub(offense, '') }
let(:source) do let(:source) do
<<~RUBY <<~RUBY
FactoryBot.define do FactoryBot.define do
factory :project do factory :project do
attribute { #{code_snippet} } attribute { #{code_snippet} }
#{offense} #{offense}
end
end end
end
RUBY RUBY
end end
it 'registers an offense' do let(:corrected_source) do
expect_offense(source) <<~RUBY
FactoryBot.define do
factory :project do
attribute { #{autocorrected} }
end
end
RUBY
end end
it 'autocorrects the source' do it 'registers an offense and corrects', :aggregate_failures do
corrected = autocorrect_source(pristine_source) expect_offense(source)
expect(corrected).not_to include(code_snippet) expect_correction(corrected_source)
expect(corrected).to include(autocorrected)
end end
end end
......
...@@ -4,50 +4,42 @@ require 'fast_spec_helper' ...@@ -4,50 +4,42 @@ require 'fast_spec_helper'
require 'rspec-parameterized' require 'rspec-parameterized'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/rspec/have_gitlab_http_status' require_relative '../../../../rubocop/cop/rspec/have_gitlab_http_status'
RSpec.describe RuboCop::Cop::RSpec::HaveGitlabHttpStatus do RSpec.describe RuboCop::Cop::RSpec::HaveGitlabHttpStatus do
include CopHelper
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
let(:source_file) { 'spec/foo_spec.rb' } let(:source_file) { 'spec/foo_spec.rb' }
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
shared_examples 'offense' do |code| shared_examples 'offense' do |bad, good|
it 'registers an offense' do it 'registers an offense', :aggregate_failures do
inspect_source(code, source_file) expect_offense(<<~CODE, node: bad)
%{node}
^{node} [...]
CODE
expect(cop.offenses.size).to eq(1) expect_correction(<<~CODE)
expect(cop.offenses.map(&:line)).to eq([1]) #{good}
expect(cop.highlights).to eq([code]) CODE
end end
end end
shared_examples 'no offense' do |code| shared_examples 'no offense' do |code|
it 'does not register an offense' do it 'does not register an offense' do
inspect_source(code) expect_no_offenses(code)
expect(cop.offenses).to be_empty
end
end
shared_examples 'autocorrect' do |bad, good|
it 'autocorrects' do
autocorrected = autocorrect_source(bad, source_file)
expect(autocorrected).to eql(good)
end end
end end
shared_examples 'no autocorrect' do |code| shared_examples 'offense with no autocorrect' do |code|
it 'does not autocorrect' do it 'does not autocorrect' do
autocorrected = autocorrect_source(code, source_file) expect_offense(<<~CODE, node: code)
%{node}
^{node} [...]
CODE
expect(autocorrected).to eql(code) expect_no_corrections
end end
end end
...@@ -64,10 +56,8 @@ RSpec.describe RuboCop::Cop::RSpec::HaveGitlabHttpStatus do ...@@ -64,10 +56,8 @@ RSpec.describe RuboCop::Cop::RSpec::HaveGitlabHttpStatus do
end end
with_them do with_them do
include_examples 'offense', params[:bad] include_examples 'offense', params[:bad], params[:good]
include_examples 'no offense', params[:good] include_examples 'no offense', params[:good]
include_examples 'autocorrect', params[:bad], params[:good]
include_examples 'no autocorrect', params[:good]
end end
end end
...@@ -77,10 +67,8 @@ RSpec.describe RuboCop::Cop::RSpec::HaveGitlabHttpStatus do ...@@ -77,10 +67,8 @@ RSpec.describe RuboCop::Cop::RSpec::HaveGitlabHttpStatus do
end end
with_them do with_them do
include_examples 'offense', params[:bad] include_examples 'offense', params[:bad], params[:good]
include_examples 'offense', params[:good] include_examples 'offense with no autocorrect', params[:good]
include_examples 'autocorrect', params[:bad], params[:good]
include_examples 'no autocorrect', params[:good]
end end
end end
...@@ -114,7 +102,6 @@ RSpec.describe RuboCop::Cop::RSpec::HaveGitlabHttpStatus do ...@@ -114,7 +102,6 @@ RSpec.describe RuboCop::Cop::RSpec::HaveGitlabHttpStatus do
with_them do with_them do
include_examples 'no offense', params[:code] include_examples 'no offense', params[:code]
include_examples 'no autocorrect', params[:code]
end end
end end
end end
...@@ -5,12 +5,10 @@ require 'fast_spec_helper' ...@@ -5,12 +5,10 @@ require 'fast_spec_helper'
require_relative '../../../../rubocop/cop/rspec/httparty_basic_auth' require_relative '../../../../rubocop/cop/rspec/httparty_basic_auth'
RSpec.describe RuboCop::Cop::RSpec::HTTPartyBasicAuth do RSpec.describe RuboCop::Cop::RSpec::HTTPartyBasicAuth do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'when passing `basic_auth: { user: ... }`' do context 'when passing `basic_auth: { user: ... }`' do
it 'registers an offence' do it 'registers an offense and corrects', :aggregate_failures do
expect_offense(<<~SOURCE, 'spec/foo.rb') expect_offense(<<~SOURCE, 'spec/foo.rb')
HTTParty.put( HTTParty.put(
url, url,
...@@ -19,17 +17,19 @@ RSpec.describe RuboCop::Cop::RSpec::HTTPartyBasicAuth do ...@@ -19,17 +17,19 @@ RSpec.describe RuboCop::Cop::RSpec::HTTPartyBasicAuth do
body: body body: body
) )
SOURCE SOURCE
end
it 'can autocorrect the source' do expect_correction(<<~SOURCE)
bad = 'HTTParty.put(url, basic_auth: { user: user, password: token })' HTTParty.put(
good = 'HTTParty.put(url, basic_auth: { username: user, password: token })' url,
expect(autocorrect_source(bad)).to eq(good) basic_auth: { username: user, password: token },
body: body
)
SOURCE
end end
end end
context 'when passing `basic_auth: { username: ... }`' do context 'when passing `basic_auth: { username: ... }`' do
it 'does not register an offence' do it 'does not register an offense' do
expect_no_offenses(<<~SOURCE, 'spec/frontend/fixtures/foo.rb') expect_no_offenses(<<~SOURCE, 'spec/frontend/fixtures/foo.rb')
HTTParty.put( HTTParty.put(
url, url,
......
...@@ -5,33 +5,20 @@ require 'rubocop' ...@@ -5,33 +5,20 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/rspec/modify_sidekiq_middleware' require_relative '../../../../rubocop/cop/rspec/modify_sidekiq_middleware'
RSpec.describe RuboCop::Cop::RSpec::ModifySidekiqMiddleware do RSpec.describe RuboCop::Cop::RSpec::ModifySidekiqMiddleware do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
let(:source) do it 'registers an offense and corrects', :aggregate_failures do
<<~SRC expect_offense(<<~CODE)
Sidekiq::Testing.server_middleware do |chain| Sidekiq::Testing.server_middleware do |chain|
chain.add(MyCustomMiddleware) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't modify global sidekiq middleware, [...]
end chain.add(MyCustomMiddleware)
SRC end
end CODE
let(:corrected) do expect_correction(<<~CODE)
<<~SRC with_sidekiq_server_middleware do |chain|
with_sidekiq_server_middleware do |chain| chain.add(MyCustomMiddleware)
chain.add(MyCustomMiddleware) end
end CODE
SRC
end
it 'registers an offence' do
inspect_source(source)
expect(cop.offenses.size).to eq(1)
end
it 'can autocorrect the source' do
expect(autocorrect_source(source)).to eq(corrected)
end end
end end
...@@ -3,50 +3,29 @@ ...@@ -3,50 +3,29 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/rspec/timecop_freeze' require_relative '../../../../rubocop/cop/rspec/timecop_freeze'
RSpec.describe RuboCop::Cop::RSpec::TimecopFreeze do RSpec.describe RuboCop::Cop::RSpec::TimecopFreeze do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'when calling Timecop.freeze' do context 'when calling Timecop.freeze' do
let(:source) do it 'registers an offense and corrects', :aggregate_failures do
<<~SRC expect_offense(<<~CODE)
Timecop.freeze(Time.current) { example.run } Timecop.freeze(Time.current) { example.run }
SRC ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `Timecop.freeze`, use `freeze_time` instead. [...]
end CODE
let(:corrected_source) do expect_correction(<<~CODE)
<<~SRC freeze_time(Time.current) { example.run }
freeze_time(Time.current) { example.run } CODE
SRC
end
it 'registers an offence' do
inspect_source(source)
expect(cop.offenses.size).to eq(1)
end
it 'can autocorrect the source' do
expect(autocorrect_source(source)).to eq(corrected_source)
end end
end end
context 'when calling a different method on Timecop' do context 'when calling a different method on Timecop' do
let(:source) do it 'does not register an offense' do
<<~SRC expect_no_offenses(<<~CODE)
Timecop.travel(Time.current) Timecop.travel(Time.current)
SRC CODE
end
it 'does not register an offence' do
inspect_source(source)
expect(cop.offenses).to be_empty
end end
end end
end end
...@@ -3,50 +3,29 @@ ...@@ -3,50 +3,29 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/rspec/timecop_travel' require_relative '../../../../rubocop/cop/rspec/timecop_travel'
RSpec.describe RuboCop::Cop::RSpec::TimecopTravel do RSpec.describe RuboCop::Cop::RSpec::TimecopTravel do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'when calling Timecop.travel' do context 'when calling Timecop.travel' do
let(:source) do it 'registers an offense and corrects', :aggregate_failures do
<<~SRC expect_offense(<<~CODE)
Timecop.travel(1.day.ago) { create(:issue) } Timecop.travel(1.day.ago) { create(:issue) }
SRC ^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `Timecop.travel`, use `travel_to` instead. [...]
end CODE
let(:corrected_source) do expect_correction(<<~CODE)
<<~SRC travel_to(1.day.ago) { create(:issue) }
travel_to(1.day.ago) { create(:issue) } CODE
SRC
end
it 'registers an offence' do
inspect_source(source)
expect(cop.offenses.size).to eq(1)
end
it 'can autocorrect the source' do
expect(autocorrect_source(source)).to eq(corrected_source)
end end
end end
context 'when calling a different method on Timecop' do context 'when calling a different method on Timecop' do
let(:source) do it 'does not register an offense' do
<<~SRC expect_no_offenses(<<~CODE)
Timecop.freeze { create(:issue) } Timecop.freeze { create(:issue) }
SRC CODE
end
it 'does not register an offence' do
inspect_source(source)
expect(cop.offenses).to be_empty
end end
end end
end end
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/rspec/top_level_describe_path' require_relative '../../../../rubocop/cop/rspec/top_level_describe_path'
RSpec.describe RuboCop::Cop::RSpec::TopLevelDescribePath do RSpec.describe RuboCop::Cop::RSpec::TopLevelDescribePath do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'when the file ends in _spec.rb' do context 'when the file ends in _spec.rb' do
......
...@@ -2,12 +2,9 @@ ...@@ -2,12 +2,9 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/safe_params' require_relative '../../../rubocop/cop/safe_params'
RSpec.describe RuboCop::Cop::SafeParams do RSpec.describe RuboCop::Cop::SafeParams do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
it 'flags the params as an argument of url_for' do it 'flags the params as an argument of url_for' do
......
...@@ -5,54 +5,44 @@ require 'rubocop' ...@@ -5,54 +5,44 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/scalability/bulk_perform_with_context' require_relative '../../../../rubocop/cop/scalability/bulk_perform_with_context'
RSpec.describe RuboCop::Cop::Scalability::BulkPerformWithContext do RSpec.describe RuboCop::Cop::Scalability::BulkPerformWithContext do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
it "adds an offense when calling bulk_perform_async" do it "adds an offense when calling bulk_perform_async" do
inspect_source(<<~CODE) expect_offense(<<~CODE)
Worker.bulk_perform_async(args) Worker.bulk_perform_async(args)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `Worker.bulk_perform_async_with_contexts` [...]
CODE CODE
expect(cop.offenses.size).to eq(1)
end end
it "adds an offense when calling bulk_perform_in" do it "adds an offense when calling bulk_perform_in" do
inspect_source(<<~CODE) expect_offense(<<~CODE)
diffs.each_batch(of: BATCH_SIZE) do |relation, index| diffs.each_batch(of: BATCH_SIZE) do |relation, index|
ids = relation.pluck_primary_key.map { |id| [id] } ids = relation.pluck_primary_key.map { |id| [id] }
DeleteDiffFilesWorker.bulk_perform_in(index * 5.minutes, ids) DeleteDiffFilesWorker.bulk_perform_in(index * 5.minutes, ids)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `Worker.bulk_perform_async_with_contexts` [...]
end end
CODE CODE
expect(cop.offenses.size).to eq(1)
end end
it "does not add an offense for migrations" do it "does not add an offense for migrations" do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
inspect_source(<<~CODE) expect_no_offenses(<<~CODE)
Worker.bulk_perform_in(args) Worker.bulk_perform_in(args)
CODE CODE
expect(cop.offenses.size).to eq(0)
end end
it "does not add an offence for specs" do it "does not add an offence for specs" do
allow(cop).to receive(:in_spec?).and_return(true) allow(cop).to receive(:in_spec?).and_return(true)
inspect_source(<<~CODE) expect_no_offenses(<<~CODE)
Worker.bulk_perform_in(args) Worker.bulk_perform_in(args)
CODE CODE
expect(cop.offenses.size).to eq(0)
end end
it "does not add an offense for scheduling BackgroundMigrations" do it "does not add an offense for scheduling BackgroundMigrations" do
inspect_source(<<~CODE) expect_no_offenses(<<~CODE)
BackgroundMigrationWorker.bulk_perform_in(args) BackgroundMigrationWorker.bulk_perform_in(args)
CODE CODE
expect(cop.offenses.size).to eq(0)
end end
end end
...@@ -5,18 +5,15 @@ require 'rubocop' ...@@ -5,18 +5,15 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/scalability/cron_worker_context' require_relative '../../../../rubocop/cop/scalability/cron_worker_context'
RSpec.describe RuboCop::Cop::Scalability::CronWorkerContext do RSpec.describe RuboCop::Cop::Scalability::CronWorkerContext do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
it 'adds an offense when including CronjobQueue' do it 'adds an offense when including CronjobQueue' do
inspect_source(<<~CODE) expect_offense(<<~CODE)
class SomeWorker class SomeWorker
include CronjobQueue include CronjobQueue
^^^^^^^^^^^^ Manually define an ApplicationContext for cronjob-workers.[...]
end end
CODE CODE
expect(cop.offenses.size).to eq(1)
end end
it 'does not add offenses for other workers' do it 'does not add offenses for other workers' do
......
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/scalability/file_uploads' require_relative '../../../../rubocop/cop/scalability/file_uploads'
RSpec.describe RuboCop::Cop::Scalability::FileUploads do RSpec.describe RuboCop::Cop::Scalability::FileUploads do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
let(:message) { 'Do not upload files without workhorse acceleration. Please refer to https://docs.gitlab.com/ee/development/uploads.html' } let(:message) { 'Do not upload files without workhorse acceleration. Please refer to https://docs.gitlab.com/ee/development/uploads.html' }
......
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/scalability/idempotent_worker' require_relative '../../../../rubocop/cop/scalability/idempotent_worker'
RSpec.describe RuboCop::Cop::Scalability::IdempotentWorker do RSpec.describe RuboCop::Cop::Scalability::IdempotentWorker do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
before do before do
...@@ -16,21 +14,18 @@ RSpec.describe RuboCop::Cop::Scalability::IdempotentWorker do ...@@ -16,21 +14,18 @@ RSpec.describe RuboCop::Cop::Scalability::IdempotentWorker do
end end
it 'adds an offense when not defining idempotent method' do it 'adds an offense when not defining idempotent method' do
inspect_source(<<~CODE) expect_offense(<<~CODE)
class SomeWorker class SomeWorker
^^^^^^^^^^^^^^^^ Avoid adding not idempotent workers.[...]
end end
CODE CODE
expect(cop.offenses.size).to eq(1)
end end
it 'adds an offense when not defining idempotent method' do it 'adds an offense when not defining idempotent method' do
inspect_source(<<~CODE) expect_no_offenses(<<~CODE)
class SomeWorker class SomeWorker
idempotent! idempotent!
end end
CODE CODE
expect(cop.offenses.size).to be_zero
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