From 36a59d088eca61b834191dacea009677a96c052f Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 19 May 2022 07:33:21 +0000 Subject: Add latest changes from gitlab-org/gitlab@15-0-stable-ee --- .../cop/database/multiple_databases_spec.rb | 7 + .../cop/gitlab/mark_used_feature_flags_spec.rb | 1 - spec/rubocop/cop/gitlab/namespaced_class_spec.rb | 143 ++++++++++++++------- .../background_migration_base_class_spec.rb | 104 +++++++++++++++ .../migration/background_migration_record_spec.rb | 59 +++++++++ spec/rubocop/cop/migration/hash_index_spec.rb | 47 ------- .../rubocop/cop/migration/migration_record_spec.rb | 59 +++++++++ 7 files changed, 327 insertions(+), 93 deletions(-) create mode 100644 spec/rubocop/cop/migration/background_migration_base_class_spec.rb create mode 100644 spec/rubocop/cop/migration/background_migration_record_spec.rb delete mode 100644 spec/rubocop/cop/migration/hash_index_spec.rb create mode 100644 spec/rubocop/cop/migration/migration_record_spec.rb (limited to 'spec/rubocop') diff --git a/spec/rubocop/cop/database/multiple_databases_spec.rb b/spec/rubocop/cop/database/multiple_databases_spec.rb index 8bcd4710305..6ee1e7b13ca 100644 --- a/spec/rubocop/cop/database/multiple_databases_spec.rb +++ b/spec/rubocop/cop/database/multiple_databases_spec.rb @@ -13,6 +13,13 @@ RSpec.describe RuboCop::Cop::Database::MultipleDatabases do SOURCE end + it 'flags the use of ::ActiveRecord::Base.connection' do + expect_offense(<<~SOURCE) + ::ActiveRecord::Base.connection.inspect + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use methods from ActiveRecord::Base, [...] + SOURCE + end + described_class::ALLOWED_METHODS.each do |method_name| it "does not flag use of ActiveRecord::Base.#{method_name}" do expect_no_offenses(<<~SOURCE) 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 6b5b07fb357..2ec3ae7aada 100644 --- a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb +++ b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb @@ -13,7 +13,6 @@ RSpec.describe RuboCop::Cop::Gitlab::MarkUsedFeatureFlags do subject(:cop) { described_class.new } before 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 diff --git a/spec/rubocop/cop/gitlab/namespaced_class_spec.rb b/spec/rubocop/cop/gitlab/namespaced_class_spec.rb index 824a1b8cef5..d9209a8672c 100644 --- a/spec/rubocop/cop/gitlab/namespaced_class_spec.rb +++ b/spec/rubocop/cop/gitlab/namespaced_class_spec.rb @@ -1,72 +1,125 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop/rspec/support' require_relative '../../../../rubocop/cop/gitlab/namespaced_class' RSpec.describe RuboCop::Cop::Gitlab::NamespacedClass do subject(:cop) { described_class.new } - it 'flags a class definition without namespace' do - expect_offense(<<~SOURCE) - class MyClass - ^^^^^^^^^^^^^ #{described_class::MSG} - end - SOURCE - end + shared_examples 'enforces namespaced classes' do + def namespaced(code) + return code unless namespace - it 'flags a class definition with inheritance without namespace' do - expect_offense(<<~SOURCE) - class MyClass < ApplicationRecord - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} - def some_method - true + <<~SOURCE + module #{namespace} + #{code} end - end - SOURCE - end + SOURCE + end + + it 'flags a class definition without additional namespace' do + expect_offense(namespaced(<<~SOURCE)) + class MyClass + ^^^^^^^^^^^^^ #{described_class::MSG} + end + SOURCE + end - it 'does not flag the class definition with namespace in separate lines' do - expect_no_offenses(<<~SOURCE) - module MyModule + it 'flags a compact class definition without additional namespace' do + expect_offense(<<~SOURCE, namespace: namespace) + class %{namespace}::MyClass + ^{namespace}^^^^^^^^^^^^^^^ #{described_class::MSG} + end + SOURCE + end + + it 'flags a class definition with inheritance without additional namespace' do + expect_offense(namespaced(<<~SOURCE)) class MyClass < ApplicationRecord + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + def some_method + true + end end + SOURCE + end - class MyOtherClass - def other_method - 1 + 1 + it 'does not flag the class definition with namespace in separate lines' do + expect_no_offenses(namespaced(<<~SOURCE)) + module MyModule + class MyClass < ApplicationRecord + end + + class MyOtherClass + def other_method + 1 + 1 + end end end - end - SOURCE - end + SOURCE + end - it 'does not flag the class definition with nested namespace in separate lines' do - expect_no_offenses(<<~SOURCE) - module TopLevelModule - module NestedModule - class MyClass + it 'does not flag the class definition with nested namespace in separate lines' do + expect_no_offenses(namespaced(<<~SOURCE)) + module TopLevelModule + module NestedModule + class MyClass + end end end - end - SOURCE - end + SOURCE + end + + it 'does not flag the class definition nested inside namespaced class' do + expect_no_offenses(namespaced(<<~SOURCE)) + module TopLevelModule + class TopLevelClass + class MyClass + end + end + end + SOURCE + end - it 'does not flag the class definition nested inside namespaced class' do - expect_no_offenses(<<~SOURCE) - module TopLevelModule - class TopLevelClass + it 'does not flag the class definition nested inside compact namespace' do + expect_no_offenses(<<~SOURCE) + module #{namespace}::TopLevelModule class MyClass end end - end - SOURCE + SOURCE + end + + it 'does not flag a compact namespaced class definition' do + expect_no_offenses(namespaced(<<~SOURCE)) + class MyModule::MyClass < ApplicationRecord + end + SOURCE + end + + it 'does not flag a truly compact namespaced class definition' do + expect_no_offenses(<<~SOURCE, namespace: namespace) + class %{namespace}::MyModule::MyClass < ApplicationRecord + end + SOURCE + end end - it 'does not flag a compact namespaced class definition' do - expect_no_offenses(<<~SOURCE) - class MyModule::MyClass < ApplicationRecord - end - SOURCE + context 'without top-level namespace' do + let(:namespace) { nil } + + it_behaves_like 'enforces namespaced classes' + end + + context 'with Gitlab namespace' do + let(:namespace) { 'Gitlab' } + + it_behaves_like 'enforces namespaced classes' + end + + context 'with ::Gitlab namespace' do + let(:namespace) { '::Gitlab' } + + it_behaves_like 'enforces namespaced classes' end end diff --git a/spec/rubocop/cop/migration/background_migration_base_class_spec.rb b/spec/rubocop/cop/migration/background_migration_base_class_spec.rb new file mode 100644 index 00000000000..0a110418139 --- /dev/null +++ b/spec/rubocop/cop/migration/background_migration_base_class_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require_relative '../../../../rubocop/cop/migration/background_migration_base_class' + +RSpec.describe RuboCop::Cop::Migration::BackgroundMigrationBaseClass do + subject(:cop) { described_class.new } + + context 'when the migration class inherits from BatchedMigrationJob' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + module Gitlab + module BackgroundMigration + class MyJob < BatchedMigrationJob + def perform + connection.execute("select 1") + end + end + end + end + RUBY + end + end + + context 'when the migration class inherits from the namespaced BatchedMigrationJob' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + module Gitlab + module BackgroundMigration + class MyJob < Gitlab::BackgroundMigration::BatchedMigrationJob + def perform + connection.execute("select 1") + end + end + end + end + RUBY + end + end + + context 'when the migration class inherits from the top-level namespaced BatchedMigrationJob' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + module Gitlab + module BackgroundMigration + class MyJob < ::Gitlab::BackgroundMigration::BatchedMigrationJob + def perform + connection.execute("select 1") + end + end + end + end + RUBY + end + end + + context 'when a nested class is used inside the job class' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + module Gitlab + module BackgroundMigration + class MyJob < BatchedMigrationJob + class Project < ApplicationRecord + self.table_name = 'projects' + end + + def perform + Project.update!(name: 'hi') + end + end + end + end + RUBY + end + end + + context 'when the migration class inherits from another class' do + it 'registers an offense' do + expect_offense(<<~RUBY) + module Gitlab + module BackgroundMigration + class MyJob < SomeOtherClass + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + end + end + end + RUBY + end + end + + context 'when the migration class does not inherit from anything' do + it 'registers an offense' do + expect_offense(<<~RUBY) + module Gitlab + module BackgroundMigration + class MyJob + ^^^^^^^^^^^ #{described_class::MSG} + end + end + end + RUBY + end + end +end diff --git a/spec/rubocop/cop/migration/background_migration_record_spec.rb b/spec/rubocop/cop/migration/background_migration_record_spec.rb new file mode 100644 index 00000000000..b5724ef1efd --- /dev/null +++ b/spec/rubocop/cop/migration/background_migration_record_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require_relative '../../../../rubocop/cop/migration/background_migration_record' + +RSpec.describe RuboCop::Cop::Migration::BackgroundMigrationRecord do + subject(:cop) { described_class.new } + + context 'outside of a migration' do + it 'does not register any offenses' do + expect_no_offenses(<<~SOURCE) + class MigrateProjectRecords + class Project < ActiveRecord::Base + end + end + SOURCE + end + end + + context 'in migration' do + before do + allow(cop).to receive(:in_background_migration?).and_return(true) + end + + it 'adds an offense if inheriting from ActiveRecord::Base' do + expect_offense(<<~RUBY) + class MigrateProjectRecords + class Project < ActiveRecord::Base + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use or inherit from ActiveRecord::Base.[...] + end + end + RUBY + end + + it 'adds an offense if create dynamic model from ActiveRecord::Base' do + expect_offense(<<~RUBY) + class MigrateProjectRecords + def define_model(table_name) + Class.new(ActiveRecord::Base) do + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use or inherit from ActiveRecord::Base.[...] + self.table_name = table_name + self.inheritance_column = :_type_disabled + end + end + end + RUBY + end + + it 'adds an offense if inheriting from ::ActiveRecord::Base' do + expect_offense(<<~RUBY) + class MigrateProjectRecords + class Project < ::ActiveRecord::Base + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use or inherit from ActiveRecord::Base.[...] + end + end + RUBY + end + end +end diff --git a/spec/rubocop/cop/migration/hash_index_spec.rb b/spec/rubocop/cop/migration/hash_index_spec.rb deleted file mode 100644 index 6da27af39b6..00000000000 --- a/spec/rubocop/cop/migration/hash_index_spec.rb +++ /dev/null @@ -1,47 +0,0 @@ -# frozen_string_literal: true - -require 'fast_spec_helper' -require_relative '../../../../rubocop/cop/migration/hash_index' - -RSpec.describe RuboCop::Cop::Migration::HashIndex do - subject(:cop) { described_class.new } - - context 'when in migration' do - before do - allow(cop).to receive(:in_migration?).and_return(true) - end - - it 'registers an offense when creating a hash index' do - expect_offense(<<~RUBY) - def change - add_index :table, :column, using: :hash - ^^^^^^^^^^^^ hash indexes should be avoided at all costs[...] - end - RUBY - end - - it 'registers an offense when creating a concurrent hash index' do - expect_offense(<<~RUBY) - def change - add_concurrent_index :table, :column, using: :hash - ^^^^^^^^^^^^ hash indexes should be avoided at all costs[...] - end - RUBY - end - - it 'registers an offense when creating a hash index using t.index' do - expect_offense(<<~RUBY) - def change - t.index :table, :column, using: :hash - ^^^^^^^^^^^^ hash indexes should be avoided at all costs[...] - end - RUBY - end - end - - context 'when outside of migration' do - it 'registers no offense' do - expect_no_offenses('def change; index :table, :column, using: :hash; end') - end - end -end diff --git a/spec/rubocop/cop/migration/migration_record_spec.rb b/spec/rubocop/cop/migration/migration_record_spec.rb new file mode 100644 index 00000000000..bab0ca469df --- /dev/null +++ b/spec/rubocop/cop/migration/migration_record_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require_relative '../../../../rubocop/cop/migration/migration_record' + +RSpec.describe RuboCop::Cop::Migration::MigrationRecord do + subject(:cop) { described_class.new } + + shared_examples 'a disabled cop' do + it 'does not register any offenses' do + expect_no_offenses(<<~SOURCE) + class MyMigration < Gitlab::Database::Migration[2.0] + class Project < ActiveRecord::Base + end + end + SOURCE + 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) + end + + it 'adds an offense if inheriting from ActiveRecord::Base' do + expect_offense(<<~RUBY) + class Project < ActiveRecord::Base + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't inherit from ActiveRecord::Base but use MigrationRecord instead.[...] + end + RUBY + end + + it 'adds an offense if inheriting from ::ActiveRecord::Base' do + expect_offense(<<~RUBY) + class Project < ::ActiveRecord::Base + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't inherit from ActiveRecord::Base but use MigrationRecord instead.[...] + end + RUBY + end + end + end +end -- cgit v1.2.3