summaryrefslogtreecommitdiff
path: root/spec/rubocop/cop
diff options
context:
space:
mode:
Diffstat (limited to 'spec/rubocop/cop')
-rw-r--r--spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb1
-rw-r--r--spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb36
-rw-r--r--spec/rubocop/cop/migration/prevent_index_creation_spec.rb86
-rw-r--r--spec/rubocop/cop/migration/versioned_migration_class_spec.rb81
-rw-r--r--spec/rubocop/cop/performance/active_record_subtransaction_methods_spec.rb29
-rw-r--r--spec/rubocop/cop/performance/active_record_subtransactions_spec.rb62
-rw-r--r--spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_spec.rb (renamed from spec/rubocop/cop/worker_data_consistency_spec.rb)4
-rw-r--r--spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication_spec.rb166
8 files changed, 444 insertions, 21 deletions
diff --git a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb
index 35b21477d80..6b5b07fb357 100644
--- a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb
+++ b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb
@@ -16,6 +16,7 @@ RSpec.describe RuboCop::Cop::Gitlab::MarkUsedFeatureFlags do
stub_const("#{described_class}::DYNAMIC_FEATURE_FLAGS", [])
allow(cop).to receive(:defined_feature_flags).and_return(defined_feature_flags)
allow(cop).to receive(:usage_data_counters_known_event_feature_flags).and_return([])
+ described_class.feature_flags_already_tracked = false
end
def feature_flag_path(feature_flag_name)
diff --git a/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb b/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb
index 899872859a9..f6bed0d74fb 100644
--- a/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb
+++ b/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb
@@ -11,6 +11,7 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
before do
allow(cop).to receive(:in_migration?).and_return(true)
+ allow(cop).to receive(:version).and_return(described_class::TEXT_LIMIT_ATTRIBUTE_ALLOWED_SINCE + 5)
end
context 'when text columns are defined without a limit' do
@@ -26,7 +27,7 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
^^^^ #{msg}
end
- create_table_with_constraints :test_text_limits_create do |t|
+ create_table :test_text_limits_create do |t|
t.integer :test_id, null: false
t.text :title
t.text :description
@@ -61,13 +62,10 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
t.text :name
end
- create_table_with_constraints :test_text_limits_create do |t|
+ create_table :test_text_limits_create do |t|
t.integer :test_id, null: false
- t.text :title
- t.text :description
-
- t.text_limit :title, 100
- t.text_limit :description, 255
+ t.text :title, limit: 100
+ t.text :description, limit: 255
end
add_column :test_text_limits, :email, :text
@@ -82,6 +80,30 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
end
RUBY
end
+
+ context 'for migrations before 2021_09_10_00_00_00' do
+ it 'when limit: attribute is used (which is not supported yet for this version): registers an offense' do
+ allow(cop).to receive(:version).and_return(described_class::TEXT_LIMIT_ATTRIBUTE_ALLOWED_SINCE - 5)
+
+ expect_offense(<<~RUBY)
+ class TestTextLimits < ActiveRecord::Migration[6.0]
+ def up
+ create_table :test_text_limit_attribute do |t|
+ t.integer :test_id, null: false
+ t.text :name, limit: 100
+ ^^^^ Text columns should always have a limit set (255 is suggested). Using limit: is not supported in this version. You can add a limit to a `text` column by using `add_text_limit` or `.text_limit` inside `create_table`
+ end
+
+ create_table_with_constraints :test_text_limit_attribute do |t|
+ t.integer :test_id, null: false
+ t.text :name, limit: 100
+ ^^^^ Text columns should always have a limit set (255 is suggested). Using limit: is not supported in this version. You can add a limit to a `text` column by using `add_text_limit` or `.text_limit` inside `create_table`
+ end
+ end
+ end
+ RUBY
+ end
+ end
end
context 'when text array columns are defined without a limit' do
diff --git a/spec/rubocop/cop/migration/prevent_index_creation_spec.rb b/spec/rubocop/cop/migration/prevent_index_creation_spec.rb
index a3965f54bbd..ed7c8974d8d 100644
--- a/spec/rubocop/cop/migration/prevent_index_creation_spec.rb
+++ b/spec/rubocop/cop/migration/prevent_index_creation_spec.rb
@@ -6,28 +6,76 @@ require_relative '../../../../rubocop/cop/migration/prevent_index_creation'
RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do
subject(:cop) { described_class.new }
+ let(:forbidden_tables) { %w(ci_builds) }
+ let(:forbidden_tables_list) { forbidden_tables.join(', ') }
+
context 'when in migration' do
before do
allow(cop).to receive(:in_migration?).and_return(true)
end
context 'when adding an index to a forbidden table' do
- it 'registers an offense when add_index is used' do
- expect_offense(<<~RUBY)
- def change
- add_index :ci_builds, :protected
- ^^^^^^^^^ Adding new index to ci_builds is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
+ context 'when table_name is a symbol' do
+ it "registers an offense when add_index is used", :aggregate_failures do
+ forbidden_tables.each do |table_name|
+ expect_offense(<<~RUBY)
+ def change
+ add_index :#{table_name}, :protected
+ ^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
+ end
+ RUBY
end
- RUBY
+ end
+
+ it "registers an offense when add_concurrent_index is used", :aggregate_failures do
+ forbidden_tables.each do |table_name|
+ expect_offense(<<~RUBY)
+ def change
+ add_concurrent_index :#{table_name}, :protected
+ ^^^^^^^^^^^^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
+ end
+ RUBY
+ end
+ end
end
- it 'registers an offense when add_concurrent_index is used' do
- expect_offense(<<~RUBY)
- def change
- add_concurrent_index :ci_builds, :protected
- ^^^^^^^^^^^^^^^^^^^^ Adding new index to ci_builds is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
+ context 'when table_name is a string' do
+ it "registers an offense when add_index is used", :aggregate_failures do
+ forbidden_tables.each do |table_name|
+ expect_offense(<<~RUBY)
+ def change
+ add_index "#{table_name}", :protected
+ ^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
+ end
+ RUBY
end
- RUBY
+ end
+
+ it "registers an offense when add_concurrent_index is used", :aggregate_failures do
+ forbidden_tables.each do |table_name|
+ expect_offense(<<~RUBY)
+ def change
+ add_concurrent_index "#{table_name}", :protected
+ ^^^^^^^^^^^^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
+ end
+ RUBY
+ end
+ end
+ end
+
+ context 'when table_name is a constant' do
+ it "registers an offense when add_concurrent_index is used", :aggregate_failures do
+ expect_offense(<<~RUBY)
+ INDEX_NAME = "index_name"
+ TABLE_NAME = :ci_builds
+ disable_ddl_transaction!
+
+ def change
+ add_concurrent_index TABLE_NAME, :protected
+ ^^^^^^^^^^^^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
+ end
+ RUBY
+ end
end
end
@@ -39,6 +87,20 @@ RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do
end
RUBY
end
+
+ context 'when using a constant' do
+ it 'does not register an offense' do
+ expect_no_offenses(<<~RUBY)
+ disable_ddl_transaction!
+
+ TABLE_NAME = "not_forbidden"
+
+ def up
+ add_concurrent_index TABLE_NAME, :protected
+ end
+ RUBY
+ end
+ end
end
end
diff --git a/spec/rubocop/cop/migration/versioned_migration_class_spec.rb b/spec/rubocop/cop/migration/versioned_migration_class_spec.rb
new file mode 100644
index 00000000000..d9b0cd4546c
--- /dev/null
+++ b/spec/rubocop/cop/migration/versioned_migration_class_spec.rb
@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require_relative '../../../../rubocop/cop/migration/versioned_migration_class'
+
+RSpec.describe RuboCop::Cop::Migration::VersionedMigrationClass do
+ subject(:cop) { described_class.new }
+
+ let(:migration) do
+ <<~SOURCE
+ class TestMigration < Gitlab::Database::Migration[1.0]
+ def up
+ execute 'select 1'
+ end
+
+ def down
+ execute 'select 1'
+ end
+ end
+ SOURCE
+ end
+
+ shared_examples 'a disabled cop' do
+ it 'does not register any offenses' do
+ expect_no_offenses(migration)
+ end
+ end
+
+ context 'outside of a migration' do
+ it_behaves_like 'a disabled cop'
+ end
+
+ context 'in migration' do
+ before do
+ allow(cop).to receive(:in_migration?).and_return(true)
+ end
+
+ context 'in an old migration' do
+ before do
+ allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE - 5)
+ end
+
+ it_behaves_like 'a disabled cop'
+ end
+
+ context 'that is recent' do
+ before do
+ allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE + 5)
+ end
+
+ it 'adds an offence if inheriting from ActiveRecord::Migration' do
+ expect_offense(<<~RUBY)
+ class MyMigration < ActiveRecord::Migration[6.1]
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't inherit from ActiveRecord::Migration but use Gitlab::Database::Migration[1.0] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.
+ end
+ RUBY
+ end
+
+ it 'adds an offence if including Gitlab::Database::MigrationHelpers directly' do
+ expect_offense(<<~RUBY)
+ class MyMigration < Gitlab::Database::Migration[1.0]
+ include Gitlab::Database::MigrationHelpers
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't include migration helper modules directly. Inherit from Gitlab::Database::Migration[1.0] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.
+ end
+ RUBY
+ end
+
+ it 'excludes ActiveRecord classes defined inside the migration' do
+ expect_no_offenses(<<~RUBY)
+ class TestMigration < Gitlab::Database::Migration[1.0]
+ class TestModel < ApplicationRecord
+ end
+
+ class AnotherTestModel < ActiveRecord::Base
+ end
+ end
+ RUBY
+ end
+ end
+ end
+end
diff --git a/spec/rubocop/cop/performance/active_record_subtransaction_methods_spec.rb b/spec/rubocop/cop/performance/active_record_subtransaction_methods_spec.rb
new file mode 100644
index 00000000000..df18121e2df
--- /dev/null
+++ b/spec/rubocop/cop/performance/active_record_subtransaction_methods_spec.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rspec-parameterized'
+
+require_relative '../../../../rubocop/cop/performance/active_record_subtransaction_methods'
+
+RSpec.describe RuboCop::Cop::Performance::ActiveRecordSubtransactionMethods do
+ subject(:cop) { described_class.new }
+
+ let(:message) { described_class::MSG }
+
+ shared_examples 'a method that uses a subtransaction' do |method_name|
+ it 'registers an offense' do
+ expect_offense(<<~RUBY, method_name: method_name, message: message)
+ Project.%{method_name}
+ ^{method_name} %{message}
+ RUBY
+ end
+ end
+
+ context 'when the method uses a subtransaction' do
+ where(:method) { described_class::DISALLOWED_METHODS.to_a }
+
+ with_them do
+ include_examples 'a method that uses a subtransaction', params[:method]
+ end
+ end
+end
diff --git a/spec/rubocop/cop/performance/active_record_subtransactions_spec.rb b/spec/rubocop/cop/performance/active_record_subtransactions_spec.rb
new file mode 100644
index 00000000000..0da2e30062a
--- /dev/null
+++ b/spec/rubocop/cop/performance/active_record_subtransactions_spec.rb
@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require_relative '../../../../rubocop/cop/performance/active_record_subtransactions'
+
+RSpec.describe RuboCop::Cop::Performance::ActiveRecordSubtransactions do
+ subject(:cop) { described_class.new }
+
+ let(:message) { described_class::MSG }
+
+ context 'when calling #transaction with only requires_new: true' do
+ it 'registers an offense' do
+ expect_offense(<<~RUBY)
+ ApplicationRecord.transaction(requires_new: true) do
+ ^^^^^^^^^^^^^^^^^^ #{message}
+ Project.create!(name: 'MyProject')
+ end
+ RUBY
+ end
+ end
+
+ context 'when passing multiple arguments to #transaction, including requires_new: true' do
+ it 'registers an offense' do
+ expect_offense(<<~RUBY)
+ ApplicationRecord.transaction(isolation: :read_committed, requires_new: true) do
+ ^^^^^^^^^^^^^^^^^^ #{message}
+ Project.create!(name: 'MyProject')
+ end
+ RUBY
+ end
+ end
+
+ context 'when calling #transaction with requires_new: false' do
+ it 'does not register an offense' do
+ expect_no_offenses(<<~RUBY)
+ ApplicationRecord.transaction(requires_new: false) do
+ Project.create!(name: 'MyProject')
+ end
+ RUBY
+ end
+ end
+
+ context 'when calling #transaction with other options' do
+ it 'does not register an offense' do
+ expect_no_offenses(<<~RUBY)
+ ApplicationRecord.transaction(isolation: :read_committed) do
+ Project.create!(name: 'MyProject')
+ end
+ RUBY
+ end
+ end
+
+ context 'when calling #transaction with no arguments' do
+ it 'does not register an offense' do
+ expect_no_offenses(<<~RUBY)
+ ApplicationRecord.transaction do
+ Project.create!(name: 'MyProject')
+ end
+ RUBY
+ end
+ end
+end
diff --git a/spec/rubocop/cop/worker_data_consistency_spec.rb b/spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_spec.rb
index 5fa42bf2b87..cf8d0d1b66f 100644
--- a/spec/rubocop/cop/worker_data_consistency_spec.rb
+++ b/spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_spec.rb
@@ -1,9 +1,9 @@
# frozen_string_literal: true
require 'fast_spec_helper'
-require_relative '../../../rubocop/cop/worker_data_consistency'
+require_relative '../../../../rubocop/cop/sidekiq_load_balancing/worker_data_consistency'
-RSpec.describe RuboCop::Cop::WorkerDataConsistency do
+RSpec.describe RuboCop::Cop::SidekiqLoadBalancing::WorkerDataConsistency do
subject(:cop) { described_class.new }
before do
diff --git a/spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication_spec.rb b/spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication_spec.rb
new file mode 100644
index 00000000000..6e7212b1002
--- /dev/null
+++ b/spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication_spec.rb
@@ -0,0 +1,166 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rspec-parameterized'
+require_relative '../../../../rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication'
+
+RSpec.describe RuboCop::Cop::SidekiqLoadBalancing::WorkerDataConsistencyWithDeduplication do
+ using RSpec::Parameterized::TableSyntax
+
+ subject(:cop) { described_class.new }
+
+ before do
+ allow(cop)
+ .to receive(:in_worker?)
+ .and_return(true)
+ end
+
+ where(:data_consistency) { %i[delayed sticky] }
+
+ with_them do
+ let(:strategy) { described_class::DEFAULT_STRATEGY }
+ let(:corrected) do
+ <<~CORRECTED
+ class SomeWorker
+ include ApplicationWorker
+
+ data_consistency :#{data_consistency}
+
+ deduplicate #{strategy}, including_scheduled: true
+ idempotent!
+ end
+ CORRECTED
+ end
+
+ context 'when deduplication strategy is not explicitly set' do
+ it 'registers an offense and corrects using default strategy' do
+ expect_offense(<<~CODE)
+ class SomeWorker
+ include ApplicationWorker
+
+ data_consistency :#{data_consistency}
+
+ idempotent!
+ ^^^^^^^^^^^ Workers that declare either `:sticky` or `:delayed` data consistency [...]
+ end
+ CODE
+
+ expect_correction(corrected)
+ end
+
+ context 'when identation is different' do
+ let(:corrected) do
+ <<~CORRECTED
+ class SomeWorker
+ include ApplicationWorker
+
+ data_consistency :#{data_consistency}
+
+ deduplicate #{strategy}, including_scheduled: true
+ idempotent!
+ end
+ CORRECTED
+ end
+
+ it 'registers an offense and corrects with correct identation' do
+ expect_offense(<<~CODE)
+ class SomeWorker
+ include ApplicationWorker
+
+ data_consistency :#{data_consistency}
+
+ idempotent!
+ ^^^^^^^^^^^ Workers that declare either `:sticky` or `:delayed` data consistency [...]
+ end
+ CODE
+
+ expect_correction(corrected)
+ end
+ end
+ end
+
+ context 'when deduplication strategy does not include including_scheduling option' do
+ let(:strategy) { ':until_executed' }
+
+ it 'registers an offense and corrects' do
+ expect_offense(<<~CODE)
+ class SomeWorker
+ include ApplicationWorker
+
+ data_consistency :#{data_consistency}
+
+ deduplicate :until_executed
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Workers that declare either `:sticky` or `:delayed` data consistency [...]
+ idempotent!
+ end
+ CODE
+
+ expect_correction(corrected)
+ end
+ end
+
+ context 'when deduplication strategy has including_scheduling option disabled' do
+ let(:strategy) { ':until_executed' }
+
+ it 'registers an offense and corrects' do
+ expect_offense(<<~CODE)
+ class SomeWorker
+ include ApplicationWorker
+
+ data_consistency :#{data_consistency}
+
+ deduplicate :until_executed, including_scheduled: false
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Workers that declare either `:sticky` or `:delayed` data consistency [...]
+ idempotent!
+ end
+ CODE
+
+ expect_correction(corrected)
+ end
+ end
+
+ context "when deduplication strategy is :none" do
+ it 'does not register an offense' do
+ expect_no_offenses(<<~CODE)
+ class SomeWorker
+ include ApplicationWorker
+
+ data_consistency :always
+
+ deduplicate :none
+ idempotent!
+ end
+ CODE
+ end
+ end
+
+ context "when deduplication strategy has including_scheduling option enabled" do
+ it 'does not register an offense' do
+ expect_no_offenses(<<~CODE)
+ class SomeWorker
+ include ApplicationWorker
+
+ data_consistency :always
+
+ deduplicate :until_executing, including_scheduled: true
+ idempotent!
+ end
+ CODE
+ end
+ end
+ end
+
+ context "data_consistency: :always" do
+ it 'does not register an offense' do
+ expect_no_offenses(<<~CODE)
+ class SomeWorker
+ include ApplicationWorker
+
+ data_consistency :always
+
+ idempotent!
+ end
+ CODE
+ end
+ end
+end