diff options
Diffstat (limited to 'spec/rubocop')
4 files changed, 206 insertions, 12 deletions
diff --git a/spec/rubocop/cop/background_migration/dictionary_file_spec.rb b/spec/rubocop/cop/background_migration/dictionary_file_spec.rb index 7becf9c09a4..9b4adc87f78 100644 --- a/spec/rubocop/cop/background_migration/dictionary_file_spec.rb +++ b/spec/rubocop/cop/background_migration/dictionary_file_spec.rb @@ -134,16 +134,20 @@ RSpec.describe RuboCop::Cop::BackgroundMigration::DictionaryFile, feature_catego end context 'with dictionary file' do - let(:introduced_by_url) { 'https://test_url' } + let(:introduced_by_url) { 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132639' } let(:finalize_after) { '20230507160251' } + let(:milestone) { '16.1' } before do + allow(File).to receive(:exist?).and_call_original allow(File).to receive(:exist?).with(dictionary_file_path).and_return(true) - - allow_next_instance_of(RuboCop::BatchedBackgroundMigrationsDictionary) do |dictionary| - allow(dictionary).to receive(:finalize_after).and_return(finalize_after) - allow(dictionary).to receive(:introduced_by_url).and_return(introduced_by_url) - end + allow(::RuboCop::BatchedBackgroundMigrationsDictionary).to receive(:dictionary_data).and_return({ + '20231118100907' => { + finalize_after: finalize_after, + introduced_by_url: introduced_by_url, + milestone: milestone + } + }) end context 'without introduced_by_url' do @@ -158,6 +162,50 @@ RSpec.describe RuboCop::Cop::BackgroundMigration::DictionaryFile, feature_catego end end + context 'when the `introduced_by_url` is not correct' do + let(:introduced_by_url) { 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132639/invalid' } + + it 'throws offense on having a correct url' do + expect_offense(<<~RUBY) + class QueueMyMigration < Gitlab::Database::Migration[2.1] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{format('Invalid `introduced_by_url` url for the dictionary. Please use the following format: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/XXX')} + def up + queue_batched_background_migration( + 'MyMigration', + :users, + :id + ) + end + end + RUBY + end + end + + context 'without milestone' do + it_behaves_like 'migration with missing dictionary keys offense', :milestone do + let(:milestone) { nil } + end + end + + context 'when milestone is a number' do + let(:milestone) { 16.1 } + + it 'throws offense on having an invalid milestone' do + expect_offense(<<~RUBY) + class QueueMyMigration < Gitlab::Database::Migration[2.1] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{format('Invalid `milestone` for the dictionary. It must be a string. Please ensure it is quoted.')} + def up + queue_batched_background_migration( + 'MyMigration', + :users, + :id + ) + end + end + RUBY + end + end + context 'with required dictionary keys' do it 'does not throw offense with appropriate dictionary file' do expect_no_offenses(<<~RUBY) diff --git a/spec/rubocop/cop/database/avoid_using_pluck_without_limit_spec.rb b/spec/rubocop/cop/database/avoid_using_pluck_without_limit_spec.rb new file mode 100644 index 00000000000..801cf449726 --- /dev/null +++ b/spec/rubocop/cop/database/avoid_using_pluck_without_limit_spec.rb @@ -0,0 +1,144 @@ +# frozen_string_literal: true + +require 'rubocop_spec_helper' +require_relative '../../../../rubocop/cop/database/avoid_using_pluck_without_limit' + +RSpec.describe RuboCop::Cop::Database::AvoidUsingPluckWithoutLimit, feature_category: :database do + context 'when using pluck without a limit' do + it 'flags the use of pluck as a model scope' do + expect_offense(<<~RUBY) + class MyModel < ApplicationRecord + scope :all_users, -> { where(user_id: User.pluck(:id)) } + ^^^^^ #{described_class::MSG} + end + RUBY + end + + it 'flags the use of pluck as a regular method' do + expect_offense(<<~RUBY) + class MyModel < ApplicationRecord + def all + self.pluck(:id) + ^^^^^ #{described_class::MSG} + end + end + RUBY + end + + it 'flags the use of pluck inside where' do + expect_offense(<<~RUBY) + class MyModel < ApplicationRecord + def all_projects + Project.where(id: self.pluck(:id)) + ^^^^^ #{described_class::MSG} + end + end + RUBY + end + + it 'flags the use of pluck inside a model class method' do + allow(cop).to receive(:in_model?).and_return(true) + + expect_offense(<<~RUBY) + class MyClass < Model + def all_users + User.where(id: self.pluck(:id)) + ^^^^^ #{described_class::MSG} + end + end + RUBY + end + + it 'flags the use of pluck inside a finder' do + allow(cop).to receive(:in_finder?).and_return(true) + + expect_offense(<<~RUBY) + class MyFinder + def find(path) + Project.where(path: path).pluck(:id) + ^^^^^ #{described_class::MSG} + end + end + RUBY + end + + it 'flags the use of pluck inside a service' do + allow(cop).to receive(:in_service_class?).and_return(true) + + expect_offense(<<~RUBY) + class MyService + def delete_all(project) + delete(project.for_scan_result_policy_read(scan_result_policy_reads.pluck(:id))) + ^^^^^ #{described_class::MSG} + end + end + RUBY + end + end + + context 'when using pluck with a limit' do + it 'does not flags the use of pluck as a model scope' do + expect_no_offenses(<<~RUBY) + class MyModel < ApplicationRecord + scope :all_users, ->(limit) { where(user_id: User.limit(limit).pluck(:id)) } + end + RUBY + end + + it 'does not flags the use of pluck as a regular method' do + expect_no_offenses(<<~RUBY) + class MyModel < ApplicationRecord + def all(limit) + self.limit(limit).pluck(:id) + end + end + RUBY + end + + it 'does not flags the use of pluck inside where' do + expect_no_offenses(<<~RUBY) + class MyModel < ApplicationRecord + def all_projects(limit) + Project.where(id: self.limit(limit).pluck(:id)) + end + end + RUBY + end + + it 'does not flags the use of pluck inside a model class method' do + allow(cop).to receive(:in_model?).and_return(true) + + expect_no_offenses(<<~RUBY) + class MyClass < Model + def all_users + User.where(id: self.limit(100).pluck(:id)) + end + end + RUBY + end + + it 'does not flags the use of pluck inside a finder' do + allow(cop).to receive(:in_finder?).and_return(true) + + expect_no_offenses(<<~RUBY) + class MyFinder + def find(path) + Project.where(path: path).limit(100).pluck(:id) + end + end + RUBY + end + + it 'flags the use of pluck inside a service' do + allow(cop).to receive(:in_service_class?).and_return(true) + + expect_no_offenses(<<~RUBY) + class MyService + def delete_all(project) + delete(project.for_scan_result_policy_read(scan_result_policy_reads.limit(100).pluck(:id))) + end + end + RUBY + end + end +end diff --git a/spec/rubocop/cop/gitlab/avoid_gitlab_instance_checks_spec.rb b/spec/rubocop/cop/gitlab/avoid_gitlab_instance_checks_spec.rb index 2dba6194d44..b3864c5495f 100644 --- a/spec/rubocop/cop/gitlab/avoid_gitlab_instance_checks_spec.rb +++ b/spec/rubocop/cop/gitlab/avoid_gitlab_instance_checks_spec.rb @@ -18,6 +18,8 @@ RSpec.describe RuboCop::Cop::Gitlab::AvoidGitlabInstanceChecks, feature_category ::Gitlab.com? Gitlab::CurrentSettings.should_check_namespace_plan? ::Gitlab::CurrentSettings.should_check_namespace_plan? + Gitlab::Saas.enabled? + ::Gitlab::Saas.enabled? ] end diff --git a/spec/rubocop/cop/migration/versioned_migration_class_spec.rb b/spec/rubocop/cop/migration/versioned_migration_class_spec.rb index b92d9d21498..89657fbfa91 100644 --- a/spec/rubocop/cop/migration/versioned_migration_class_spec.rb +++ b/spec/rubocop/cop/migration/versioned_migration_class_spec.rb @@ -49,7 +49,7 @@ RSpec.describe RuboCop::Cop::Migration::VersionedMigrationClass, feature_categor 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 or old versions of Gitlab::Database::Migration. Use Gitlab::Database::Migration[2.1] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't inherit from ActiveRecord::Migration or old versions of Gitlab::Database::Migration. Use Gitlab::Database::Migration[#{described_class::CURRENT_MIGRATION_VERSION}] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning. end RUBY end @@ -57,23 +57,23 @@ RSpec.describe RuboCop::Cop::Migration::VersionedMigrationClass, feature_categor it 'adds an offence if inheriting from old version of Gitlab::Database::Migration' do expect_offense(<<~RUBY) class MyMigration < Gitlab::Database::Migration[2.0] - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't inherit from ActiveRecord::Migration or old versions of Gitlab::Database::Migration. Use Gitlab::Database::Migration[2.1] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't inherit from ActiveRecord::Migration or old versions of Gitlab::Database::Migration. Use Gitlab::Database::Migration[#{described_class::CURRENT_MIGRATION_VERSION}] 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[2.1] + class MyMigration < Gitlab::Database::Migration[#{described_class::CURRENT_MIGRATION_VERSION}] include Gitlab::Database::MigrationHelpers - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't include migration helper modules directly. Inherit from Gitlab::Database::Migration[2.1] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't include migration helper modules directly. Inherit from Gitlab::Database::Migration[#{described_class::CURRENT_MIGRATION_VERSION}] 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[2.1] + class TestMigration < Gitlab::Database::Migration[#{described_class::CURRENT_MIGRATION_VERSION}] class TestModel < ApplicationRecord end @@ -85,7 +85,7 @@ RSpec.describe RuboCop::Cop::Migration::VersionedMigrationClass, feature_categor it 'excludes parentless classes defined inside the migration' do expect_no_offenses(<<~RUBY) - class TestMigration < Gitlab::Database::Migration[2.1] + class TestMigration < Gitlab::Database::Migration[#{described_class::CURRENT_MIGRATION_VERSION}] class TestClass end end |