diff options
Diffstat (limited to 'spec/rubocop/cop/migration')
3 files changed, 184 insertions, 19 deletions
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 |