diff options
Diffstat (limited to 'spec/rubocop/cop/migration')
24 files changed, 343 insertions, 524 deletions
diff --git a/spec/rubocop/cop/migration/add_column_with_default_spec.rb b/spec/rubocop/cop/migration/add_column_with_default_spec.rb index cf476ae55d6..3f47613280f 100644 --- a/spec/rubocop/cop/migration/add_column_with_default_spec.rb +++ b/spec/rubocop/cop/migration/add_column_with_default_spec.rb @@ -1,15 +1,12 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/add_column_with_default' RSpec.describe RuboCop::Cop::Migration::AddColumnWithDefault do - include CopHelper - let(:cop) { described_class.new } - context 'outside of a migration' do + context 'when outside of a migration' do it 'does not register any offenses' do expect_no_offenses(<<~RUBY) def up @@ -19,18 +16,16 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnWithDefault do end end - context 'in a migration' do + context 'when in a migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end - let(:offense) { '`add_column_with_default` is deprecated, use `add_column` instead' } - it 'registers an offense' do expect_offense(<<~RUBY) def up add_column_with_default(:merge_request_diff_files, :artifacts, :boolean, default: true, allow_null: false) - ^^^^^^^^^^^^^^^^^^^^^^^ #{offense} + ^^^^^^^^^^^^^^^^^^^^^^^ `add_column_with_default` is deprecated, use `add_column` instead end RUBY end diff --git a/spec/rubocop/cop/migration/add_columns_to_wide_tables_spec.rb b/spec/rubocop/cop/migration/add_columns_to_wide_tables_spec.rb index 92863c45b1a..b78ec971245 100644 --- a/spec/rubocop/cop/migration/add_columns_to_wide_tables_spec.rb +++ b/spec/rubocop/cop/migration/add_columns_to_wide_tables_spec.rb @@ -1,15 +1,12 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/add_columns_to_wide_tables' RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do - include CopHelper - let(:cop) { described_class.new } - context 'outside of a migration' do + context 'when outside of a migration' do it 'does not register any offenses' do expect_no_offenses(<<~RUBY) def up @@ -19,14 +16,14 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do end end - context 'in a migration' do + context 'when in a migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end context 'with wide tables' do it 'registers an offense when adding a column to a wide table' do - offense = '`projects` is a wide table with several columns, addig more should be avoided unless absolutely necessary. Consider storing the column in a different table or creating a new one.' + offense = '`projects` is a wide table with several columns, [...]' expect_offense(<<~RUBY) def up @@ -37,7 +34,7 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do end it 'registers an offense when adding a column with default to a wide table' do - offense = '`users` is a wide table with several columns, addig more should be avoided unless absolutely necessary. Consider storing the column in a different table or creating a new one.' + offense = '`users` is a wide table with several columns, [...]' expect_offense(<<~RUBY) def up @@ -48,7 +45,7 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do end it 'registers an offense when adding a reference' do - offense = '`ci_builds` is a wide table with several columns, addig more should be avoided unless absolutely necessary. Consider storing the column in a different table or creating a new one.' + offense = '`ci_builds` is a wide table with several columns, [...]' expect_offense(<<~RUBY) def up @@ -59,7 +56,7 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do end it 'registers an offense when adding timestamps' do - offense = '`projects` is a wide table with several columns, addig more should be avoided unless absolutely necessary. Consider storing the column in a different table or creating a new one.' + offense = '`projects` is a wide table with several columns, [...]' expect_offense(<<~RUBY) def up diff --git a/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb b/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb index 25350ad1ecb..572c0d414b3 100644 --- a/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb +++ b/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb @@ -1,50 +1,41 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/add_concurrent_foreign_key' RSpec.describe RuboCop::Cop::Migration::AddConcurrentForeignKey do - include CopHelper - let(:cop) { described_class.new } - context 'outside of a migration' do + context 'when outside of a migration' do it 'does not register any offenses' do - inspect_source('def up; add_foreign_key(:projects, :users, column: :user_id); end') - - expect(cop.offenses).to be_empty + expect_no_offenses('def up; add_foreign_key(:projects, :users, column: :user_id); end') end end - context 'in a migration' do + context 'when in a migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end it 'registers an offense when using add_foreign_key' do - inspect_source('def up; add_foreign_key(:projects, :users, column: :user_id); end') - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end + expect_offense(<<~RUBY) + def up + add_foreign_key(:projects, :users, column: :user_id) + ^^^^^^^^^^^^^^^ `add_foreign_key` requires downtime, use `add_concurrent_foreign_key` instead + end + RUBY end it 'does not register an offense when a `NOT VALID` foreign key is added' do - inspect_source('def up; add_foreign_key(:projects, :users, column: :user_id, validate: false); end') - - expect(cop.offenses).to be_empty + expect_no_offenses('def up; add_foreign_key(:projects, :users, column: :user_id, validate: false); end') end it 'does not register an offense when `add_foreign_key` is within `with_lock_retries`' do - inspect_source <<~RUBY + expect_no_offenses(<<~RUBY) with_lock_retries do add_foreign_key :key, :projects, column: :project_id, on_delete: :cascade end RUBY - - expect(cop.offenses).to be_empty end end end diff --git a/spec/rubocop/cop/migration/add_concurrent_index_spec.rb b/spec/rubocop/cop/migration/add_concurrent_index_spec.rb index 351283a230a..52b3a5769ff 100644 --- a/spec/rubocop/cop/migration/add_concurrent_index_spec.rb +++ b/spec/rubocop/cop/migration/add_concurrent_index_spec.rb @@ -1,40 +1,33 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/add_concurrent_index' RSpec.describe RuboCop::Cop::Migration::AddConcurrentIndex do - include CopHelper - subject(:cop) { described_class.new } - context 'in migration' do + context 'when in migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end it 'registers an offense when add_concurrent_index is used inside a change method' do - inspect_source('def change; add_concurrent_index :table, :column; end') - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end + expect_offense(<<~RUBY) + def change + ^^^^^^ `add_concurrent_index` is not reversible[...] + add_concurrent_index :table, :column + end + RUBY end it 'registers no offense when add_concurrent_index is used inside an up method' do - inspect_source('def up; add_concurrent_index :table, :column; end') - - expect(cop.offenses.size).to eq(0) + expect_no_offenses('def up; add_concurrent_index :table, :column; end') end end - context 'outside of migration' do + context 'when outside of migration' do it 'registers no offense' do - inspect_source('def change; add_concurrent_index :table, :column; end') - - expect(cop.offenses.size).to eq(0) + expect_no_offenses('def change; add_concurrent_index :table, :column; end') end end end diff --git a/spec/rubocop/cop/migration/add_index_spec.rb b/spec/rubocop/cop/migration/add_index_spec.rb index 1d083e9f2d2..088bfe434f4 100644 --- a/spec/rubocop/cop/migration/add_index_spec.rb +++ b/spec/rubocop/cop/migration/add_index_spec.rb @@ -1,12 +1,9 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/add_index' RSpec.describe RuboCop::Cop::Migration::AddIndex do - include CopHelper - subject(:cop) { described_class.new } context 'in migration' do 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 149fb0a48eb..f4695ff8d2d 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 @@ -1,15 +1,14 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/add_limit_to_text_columns' RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do - include CopHelper - subject(:cop) { described_class.new } - context 'in migration' do + context 'when in migration' do + let(:msg) { 'Text columns should always have a limit set (255 is suggested)[...]' } + before do allow(cop).to receive(:in_migration?).and_return(true) end @@ -25,31 +24,29 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do create_table :test_text_limits, id: false do |t| t.integer :test_id, null: false t.text :name - ^^^^ #{described_class::MSG} + ^^^^ #{msg} end create_table_with_constraints :test_text_limits_create do |t| t.integer :test_id, null: false t.text :title t.text :description - ^^^^ #{described_class::MSG} + ^^^^ #{msg} t.text_limit :title, 100 end add_column :test_text_limits, :email, :text - ^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^ #{msg} add_column_with_default :test_text_limits, :role, :text, default: 'default' - ^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^^^^^^^^^^^^ #{msg} change_column_type_concurrently :test_text_limits, :test_id, :text - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} end end RUBY - - expect(cop.offenses.map(&:cop_name)).to all(eq('Migration/AddLimitToTextColumns')) end end @@ -111,7 +108,7 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do end # Make sure that the cop is properly checking for an `add_text_limit` - # over the same {table, attribute} as the one that triggered the offence + # over the same {table, attribute} as the one that triggered the offense context 'when the limit is defined for a same name attribute but different table' do it 'registers an offense' do expect_offense(<<~RUBY) @@ -123,17 +120,17 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do create_table :test_text_limits, id: false do |t| t.integer :test_id, null: false t.text :name - ^^^^ #{described_class::MSG} + ^^^^ #{msg} end add_column :test_text_limits, :email, :text - ^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^ #{msg} add_column_with_default :test_text_limits, :role, :text, default: 'default' - ^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^^^^^^^^^^^^ #{msg} change_column_type_concurrently :test_text_limits, :test_id, :text - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} add_text_limit :wrong_table, :name, 255 add_text_limit :wrong_table, :email, 255 @@ -142,8 +139,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do end end RUBY - - expect(cop.offenses.map(&:cop_name)).to all(eq('Migration/AddLimitToTextColumns')) end end @@ -176,18 +171,18 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do DOWNTIME = false def up - drop_table :no_offence_on_down + drop_table :no_offense_on_down end def down - create_table :no_offence_on_down, id: false do |t| + create_table :no_offense_on_down, id: false do |t| t.integer :test_id, null: false t.text :name end - add_column :no_offence_on_down, :email, :text + add_column :no_offense_on_down, :email, :text - add_column_with_default :no_offence_on_down, :role, :text, default: 'default' + add_column_with_default :no_offense_on_down, :role, :text, default: 'default' end end RUBY @@ -195,7 +190,7 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do end end - context 'outside of migration' do + context 'when outside of migration' do it 'registers no offense' do expect_no_offenses(<<~RUBY) class TestTextLimits < ActiveRecord::Migration[6.0] diff --git a/spec/rubocop/cop/migration/add_reference_spec.rb b/spec/rubocop/cop/migration/add_reference_spec.rb index 6e229d3eefc..9445780e9ed 100644 --- a/spec/rubocop/cop/migration/add_reference_spec.rb +++ b/spec/rubocop/cop/migration/add_reference_spec.rb @@ -1,15 +1,12 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/add_reference' RSpec.describe RuboCop::Cop::Migration::AddReference do - include CopHelper - let(:cop) { described_class.new } - context 'outside of a migration' do + context 'when outside of a migration' do it 'does not register any offenses' do expect_no_offenses(<<~RUBY) def up @@ -19,12 +16,12 @@ RSpec.describe RuboCop::Cop::Migration::AddReference do end end - context 'in a migration' do + context 'when in a migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end - let(:offense) { '`add_reference` requires downtime for existing tables, use `add_concurrent_foreign_key` instead. When used for new tables, `index: true` or `index: { options... } is required.`' } + let(:offense) { '`add_reference` requires downtime for existing tables, use `add_concurrent_foreign_key`[...]' } context 'when the table existed before' do it 'registers an offense when using add_reference' do diff --git a/spec/rubocop/cop/migration/add_timestamps_spec.rb b/spec/rubocop/cop/migration/add_timestamps_spec.rb index 83570711ab9..ef5a856722f 100644 --- a/spec/rubocop/cop/migration/add_timestamps_spec.rb +++ b/spec/rubocop/cop/migration/add_timestamps_spec.rb @@ -1,12 +1,9 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/add_timestamps' RSpec.describe RuboCop::Cop::Migration::AddTimestamps do - include CopHelper - subject(:cop) { described_class.new } let(:migration_with_add_timestamps) do @@ -47,44 +44,39 @@ RSpec.describe RuboCop::Cop::Migration::AddTimestamps do ) end - context 'in migration' do + context 'when in migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end it 'registers an offense when the "add_timestamps" method is used' do - inspect_source(migration_with_add_timestamps) - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([7]) - end + expect_offense(<<~RUBY) + class Users < ActiveRecord::Migration[4.2] + DOWNTIME = false + + def change + add_column(:users, :username, :text) + add_timestamps(:users) + ^^^^^^^^^^^^^^ Do not use `add_timestamps`, use `add_timestamps_with_timezone` instead + end + end + RUBY end it 'does not register an offense when the "add_timestamps" method is not used' do - inspect_source(migration_without_add_timestamps) - - aggregate_failures do - expect(cop.offenses.size).to eq(0) - end + expect_no_offenses(migration_without_add_timestamps) end it 'does not register an offense when the "add_timestamps_with_timezone" method is used' do - inspect_source(migration_with_add_timestamps_with_timezone) - - aggregate_failures do - expect(cop.offenses.size).to eq(0) - end + expect_no_offenses(migration_with_add_timestamps_with_timezone) end end - context 'outside of migration' do - it 'registers no offense' do - inspect_source(migration_with_add_timestamps) - inspect_source(migration_without_add_timestamps) - inspect_source(migration_with_add_timestamps_with_timezone) - - expect(cop.offenses.size).to eq(0) + context 'when outside of migration' do + it 'registers no offense', :aggregate_failures do + expect_no_offenses(migration_with_add_timestamps) + expect_no_offenses(migration_without_add_timestamps) + expect_no_offenses(migration_with_add_timestamps_with_timezone) end end end diff --git a/spec/rubocop/cop/migration/complex_indexes_require_name_spec.rb b/spec/rubocop/cop/migration/complex_indexes_require_name_spec.rb index 38ccf546b7c..15e947a1e53 100644 --- a/spec/rubocop/cop/migration/complex_indexes_require_name_spec.rb +++ b/spec/rubocop/cop/migration/complex_indexes_require_name_spec.rb @@ -1,15 +1,14 @@ # frozen_string_literal: true # require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/complex_indexes_require_name' RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do - include CopHelper - subject(:cop) { described_class.new } - context 'in migration' do + context 'when in migration' do + let(:msg) { 'indexes added with custom options must be explicitly named' } + before do allow(cop).to receive(:in_migration?).and_return(true) end @@ -29,9 +28,9 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do t.index :column1, unique: true t.index :column2, where: 'column1 = 0' - ^^^^^ #{described_class::MSG} + ^^^^^ #{msg} t.index :column3, using: :gin - ^^^^^ #{described_class::MSG} + ^^^^^ #{msg} end end @@ -40,8 +39,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do end end RUBY - - expect(cop.offenses.map(&:cop_name)).to all(eq("Migration/#{described_class.name.demodulize}")) end end @@ -85,20 +82,18 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do add_index :test_indexes, :column1 add_index :test_indexes, :column2, where: "column2 = 'value'", order: { column4: :desc } - ^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^ #{msg} end def down add_index :test_indexes, :column4, 'unique' => true, where: 'column4 IS NOT NULL' - ^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^ #{msg} add_concurrent_index :test_indexes, :column6, using: :gin, opclass: :gin_trgm_ops - ^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^^^^^^^^^ #{msg} end end RUBY - - expect(cop.offenses.map(&:cop_name)).to all(eq("Migration/#{described_class.name.demodulize}")) end end @@ -132,7 +127,7 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do end end - context 'outside migration' do + context 'when outside migration' do before do allow(cop).to receive(:in_migration?).and_return(false) end diff --git a/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb b/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb index 2159bad1490..7bcaf36b014 100644 --- a/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb +++ b/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb @@ -1,12 +1,9 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/create_table_with_foreign_keys' RSpec.describe RuboCop::Cop::Migration::CreateTableWithForeignKeys do - include CopHelper - let(:cop) { described_class.new } context 'outside of a migration' do @@ -22,7 +19,7 @@ RSpec.describe RuboCop::Cop::Migration::CreateTableWithForeignKeys do end end - context 'in a migration' do + context 'when in a migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end diff --git a/spec/rubocop/cop/migration/datetime_spec.rb b/spec/rubocop/cop/migration/datetime_spec.rb index a3cccae21e0..3854ddfe99c 100644 --- a/spec/rubocop/cop/migration/datetime_spec.rb +++ b/spec/rubocop/cop/migration/datetime_spec.rb @@ -1,44 +1,11 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/datetime' RSpec.describe RuboCop::Cop::Migration::Datetime do - include CopHelper - subject(:cop) { described_class.new } - let(:create_table_migration_with_datetime) do - %q( - class Users < ActiveRecord::Migration[6.0] - DOWNTIME = false - - def change - create_table :users do |t| - t.string :username, null: false - t.datetime :last_sign_in - end - end - end - ) - end - - let(:create_table_migration_with_timestamp) do - %q( - class Users < ActiveRecord::Migration[6.0] - DOWNTIME = false - - def change - create_table :users do |t| - t.string :username, null: false - t.timestamp :last_sign_in - end - end - end - ) - end - let(:create_table_migration_without_datetime) do %q( class Users < ActiveRecord::Migration[6.0] @@ -120,92 +87,94 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do ) end - context 'in migration' do + context 'when in migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end it 'registers an offense when the ":datetime" data type is used on create_table' do - inspect_source(create_table_migration_with_datetime) - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([8]) - expect(cop.offenses.first.message).to include('`datetime`') - end + expect_offense(<<~RUBY) + class Users < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + create_table :users do |t| + t.string :username, null: false + t.datetime :last_sign_in + ^^^^^^^^ Do not use the `datetime` data type[...] + end + end + end + RUBY end it 'registers an offense when the ":timestamp" data type is used on create_table' do - inspect_source(create_table_migration_with_timestamp) - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([8]) - expect(cop.offenses.first.message).to include('timestamp') - end + expect_offense(<<~RUBY) + class Users < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + create_table :users do |t| + t.string :username, null: false + t.timestamp :last_sign_in + ^^^^^^^^^ Do not use the `timestamp` data type[...] + end + end + end + RUBY end it 'does not register an offense when the ":datetime" data type is not used on create_table' do - inspect_source(create_table_migration_without_datetime) - - aggregate_failures do - expect(cop.offenses.size).to eq(0) - end + expect_no_offenses(create_table_migration_without_datetime) end it 'does not register an offense when the ":datetime_with_timezone" data type is used on create_table' do - inspect_source(create_table_migration_with_datetime_with_timezone) - - aggregate_failures do - expect(cop.offenses.size).to eq(0) - end + expect_no_offenses(create_table_migration_with_datetime_with_timezone) end it 'registers an offense when the ":datetime" data type is used on add_column' do - inspect_source(add_column_migration_with_datetime) - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([7]) - expect(cop.offenses.first.message).to include('`datetime`') - end + expect_offense(<<~RUBY) + class Users < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + add_column(:users, :username, :text) + add_column(:users, :last_sign_in, :datetime) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use the `datetime` data type[...] + end + end + RUBY end it 'registers an offense when the ":timestamp" data type is used on add_column' do - inspect_source(add_column_migration_with_timestamp) - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([7]) - expect(cop.offenses.first.message).to include('timestamp') - end + expect_offense(<<~RUBY) + class Users < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + add_column(:users, :username, :text) + add_column(:users, :last_sign_in, :timestamp) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use the `timestamp` data type[...] + end + end + RUBY end it 'does not register an offense when the ":datetime" data type is not used on add_column' do - inspect_source(add_column_migration_without_datetime) - - aggregate_failures do - expect(cop.offenses.size).to eq(0) - end + expect_no_offenses(add_column_migration_without_datetime) end it 'does not register an offense when the ":datetime_with_timezone" data type is used on add_column' do - inspect_source(add_column_migration_with_datetime_with_timezone) - - aggregate_failures do - expect(cop.offenses.size).to eq(0) - end + expect_no_offenses(add_column_migration_with_datetime_with_timezone) end end - context 'outside of migration' do - it 'registers no offense' do - inspect_source(add_column_migration_with_datetime) - inspect_source(add_column_migration_with_timestamp) - inspect_source(add_column_migration_without_datetime) - inspect_source(add_column_migration_with_datetime_with_timezone) - - expect(cop.offenses.size).to eq(0) + context 'when outside of migration' do + it 'registers no offense', :aggregate_failures do + expect_no_offenses(add_column_migration_with_datetime) + expect_no_offenses(add_column_migration_with_timestamp) + expect_no_offenses(add_column_migration_without_datetime) + expect_no_offenses(add_column_migration_with_datetime_with_timezone) end end end diff --git a/spec/rubocop/cop/migration/drop_table_spec.rb b/spec/rubocop/cop/migration/drop_table_spec.rb index d783cb56203..f1bd710f5e6 100644 --- a/spec/rubocop/cop/migration/drop_table_spec.rb +++ b/spec/rubocop/cop/migration/drop_table_spec.rb @@ -1,15 +1,16 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/drop_table' RSpec.describe RuboCop::Cop::Migration::DropTable do - include CopHelper - subject(:cop) { described_class.new } context 'when in deployment migration' do + let(:msg) do + '`drop_table` in deployment migrations requires downtime. Drop tables in post-deployment migrations instead.' + end + before do allow(cop).to receive(:in_deployment_migration?).and_return(true) end @@ -30,7 +31,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do expect_offense(<<~PATTERN) def up drop_table :table - ^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^ #{msg} end PATTERN end @@ -41,7 +42,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do expect_offense(<<~PATTERN) def change drop_table :table - ^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^ #{msg} end PATTERN end @@ -63,7 +64,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do expect_offense(<<~PATTERN) def up execute "DROP TABLE table" - ^^^^^^^ #{described_class::MSG} + ^^^^^^^ #{msg} end PATTERN end @@ -74,7 +75,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do expect_offense(<<~PATTERN) def change execute "DROP TABLE table" - ^^^^^^^ #{described_class::MSG} + ^^^^^^^ #{msg} end PATTERN end diff --git a/spec/rubocop/cop/migration/hash_index_spec.rb b/spec/rubocop/cop/migration/hash_index_spec.rb index 15f68eb990f..6da27af39b6 100644 --- a/spec/rubocop/cop/migration/hash_index_spec.rb +++ b/spec/rubocop/cop/migration/hash_index_spec.rb @@ -1,52 +1,47 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/hash_index' RSpec.describe RuboCop::Cop::Migration::HashIndex do - include CopHelper - subject(:cop) { described_class.new } - context 'in migration' do + 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 - inspect_source('def change; add_index :table, :column, using: :hash; end') - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end + 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 - inspect_source('def change; add_concurrent_index :table, :column, using: :hash; end') - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end + 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 - inspect_source('def change; t.index :table, :column, using: :hash; end') - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end + expect_offense(<<~RUBY) + def change + t.index :table, :column, using: :hash + ^^^^^^^^^^^^ hash indexes should be avoided at all costs[...] + end + RUBY end end - context 'outside of migration' do + context 'when outside of migration' do it 'registers no offense' do - inspect_source('def change; index :table, :column, using: :hash; end') - - expect(cop.offenses.size).to eq(0) + expect_no_offenses('def change; index :table, :column, using: :hash; end') end end end diff --git a/spec/rubocop/cop/migration/prevent_strings_spec.rb b/spec/rubocop/cop/migration/prevent_strings_spec.rb index 560a485017a..a9b62f23a77 100644 --- a/spec/rubocop/cop/migration/prevent_strings_spec.rb +++ b/spec/rubocop/cop/migration/prevent_strings_spec.rb @@ -1,49 +1,44 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/prevent_strings' RSpec.describe RuboCop::Cop::Migration::PreventStrings do - include CopHelper - subject(:cop) { described_class.new } - context 'in migration' do + context 'when in migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end context 'when the string data type is used' do it 'registers an offense' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, msg: "Do not use the `string` data type, use `text` instead.[...]") class Users < ActiveRecord::Migration[6.0] DOWNTIME = false def up create_table :users do |t| t.string :username, null: false - ^^^^^^ #{described_class::MSG} + ^^^^^^ %{msg} t.timestamps_with_timezone null: true t.string :password - ^^^^^^ #{described_class::MSG} + ^^^^^^ %{msg} end add_column(:users, :bio, :string) - ^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^ %{msg} add_column_with_default(:users, :url, :string, default: '/-/user', allow_null: false, limit: 255) - ^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^^^^^^^^^^^^ %{msg} change_column_type_concurrently :users, :commit_id, :string - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ %{msg} end end RUBY - - expect(cop.offenses.map(&:cop_name)).to all(eq('Migration/PreventStrings')) end end @@ -109,7 +104,7 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do end end - context 'on down' do + context 'when using down method' do it 'registers no offense' do expect_no_offenses(<<~RUBY) class Users < ActiveRecord::Migration[6.0] @@ -138,7 +133,7 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do end end - context 'outside of migration' do + context 'when outside of migration' do it 'registers no offense' do expect_no_offenses(<<~RUBY) class Users < ActiveRecord::Migration[6.0] diff --git a/spec/rubocop/cop/migration/refer_to_index_by_name_spec.rb b/spec/rubocop/cop/migration/refer_to_index_by_name_spec.rb index a25328a56a8..b3e66492d83 100644 --- a/spec/rubocop/cop/migration/refer_to_index_by_name_spec.rb +++ b/spec/rubocop/cop/migration/refer_to_index_by_name_spec.rb @@ -1,22 +1,19 @@ # frozen_string_literal: true # require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/refer_to_index_by_name' RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do - include CopHelper - subject(:cop) { described_class.new } - context 'in migration' do + context 'when in migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end context 'when existing indexes are referred to without an explicit name' do it 'registers an offense' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, msg: 'migration methods that refer to existing indexes must do so by name') class TestReferToIndexByName < ActiveRecord::Migration[6.0] DOWNTIME = false @@ -30,22 +27,22 @@ RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do end if index_exists? :test_indexes, :column2 - ^^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^^ %{msg} remove_index :test_indexes, :column2 - ^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^ %{msg} end remove_index :test_indexes, column: column3 - ^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^ %{msg} remove_index :test_indexes, name: 'index_name_4' end def down if index_exists? :test_indexes, :column4, using: :gin, opclass: :gin_trgm_ops - ^^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^^ %{msg} remove_concurrent_index :test_indexes, :column4, using: :gin, opclass: :gin_trgm_ops - ^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + ^^^^^^^^^^^^^^^^^^^^^^^ %{msg} end if index_exists? :test_indexes, :column3, unique: true, name: 'index_name_3', where: 'column3 = 10' @@ -54,13 +51,11 @@ RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do end end RUBY - - expect(cop.offenses.map(&:cop_name)).to all(eq("Migration/#{described_class.name.demodulize}")) end end end - context 'outside migration' do + context 'when outside migration' do before do allow(cop).to receive(:in_migration?).and_return(false) end diff --git a/spec/rubocop/cop/migration/remove_column_spec.rb b/spec/rubocop/cop/migration/remove_column_spec.rb index 4768093b10d..f72a5b048d5 100644 --- a/spec/rubocop/cop/migration/remove_column_spec.rb +++ b/spec/rubocop/cop/migration/remove_column_spec.rb @@ -1,67 +1,58 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/remove_column' RSpec.describe RuboCop::Cop::Migration::RemoveColumn do - include CopHelper - subject(:cop) { described_class.new } def source(meth = 'change') "def #{meth}; remove_column :table, :column; end" end - context 'in a regular migration' do + context 'when in a regular migration' do before do allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_post_deployment_migration?).and_return(false) end it 'registers an offense when remove_column is used in the change method' do - inspect_source(source('change')) - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end + expect_offense(<<~RUBY) + def change + remove_column :table, :column + ^^^^^^^^^^^^^ `remove_column` must only be used in post-deployment migrations + end + RUBY end it 'registers an offense when remove_column is used in the up method' do - inspect_source(source('up')) - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end + expect_offense(<<~RUBY) + def up + remove_column :table, :column + ^^^^^^^^^^^^^ `remove_column` must only be used in post-deployment migrations + end + RUBY end it 'registers no offense when remove_column is used in the down method' do - inspect_source(source('down')) - - expect(cop.offenses.size).to eq(0) + expect_no_offenses(source('down')) end end - context 'in a post-deployment migration' do + context 'when in a post-deployment migration' do before do allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_post_deployment_migration?).and_return(true) end it 'registers no offense' do - inspect_source(source) - - expect(cop.offenses.size).to eq(0) + expect_no_offenses(source) end end - context 'outside of a migration' do + context 'when outside of a migration' do it 'registers no offense' do - inspect_source(source) - - expect(cop.offenses.size).to eq(0) + expect_no_offenses(source) end end end diff --git a/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb b/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb index 8da368d588c..10ca0353b0f 100644 --- a/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb +++ b/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb @@ -1,12 +1,9 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/remove_concurrent_index' RSpec.describe RuboCop::Cop::Migration::RemoveConcurrentIndex do - include CopHelper - subject(:cop) { described_class.new } context 'in migration' do @@ -15,26 +12,22 @@ RSpec.describe RuboCop::Cop::Migration::RemoveConcurrentIndex do end it 'registers an offense when remove_concurrent_index is used inside a change method' do - inspect_source('def change; remove_concurrent_index :table, :column; end') - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end + expect_offense(<<~RUBY) + def change + ^^^^^^ `remove_concurrent_index` is not reversible [...] + remove_concurrent_index :table, :column + end + RUBY end it 'registers no offense when remove_concurrent_index is used inside an up method' do - inspect_source('def up; remove_concurrent_index :table, :column; end') - - expect(cop.offenses.size).to eq(0) + expect_no_offenses('def up; remove_concurrent_index :table, :column; end') end end context 'outside of migration' do it 'registers no offense' do - inspect_source('def change; remove_concurrent_index :table, :column; end') - - expect(cop.offenses.size).to eq(0) + expect_no_offenses('def change; remove_concurrent_index :table, :column; end') end end end diff --git a/spec/rubocop/cop/migration/remove_index_spec.rb b/spec/rubocop/cop/migration/remove_index_spec.rb index 274c907ac41..5d1ffef2589 100644 --- a/spec/rubocop/cop/migration/remove_index_spec.rb +++ b/spec/rubocop/cop/migration/remove_index_spec.rb @@ -1,34 +1,29 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/remove_index' RSpec.describe RuboCop::Cop::Migration::RemoveIndex do - include CopHelper - subject(:cop) { described_class.new } - context 'in migration' do + context 'when in migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end it 'registers an offense when remove_index is used' do - inspect_source('def change; remove_index :table, :column; end') - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end + expect_offense(<<~RUBY) + def change + remove_index :table, :column + ^^^^^^^^^^^^ `remove_index` requires downtime, use `remove_concurrent_index` instead + end + RUBY end end - context 'outside of migration' do + context 'when outside of migration' do it 'registers no offense' do - inspect_source('def change; remove_index :table, :column; end') - - expect(cop.offenses.size).to eq(0) + expect_no_offenses('def change; remove_index :table, :column; end') end end end diff --git a/spec/rubocop/cop/migration/safer_boolean_column_spec.rb b/spec/rubocop/cop/migration/safer_boolean_column_spec.rb index aa7bb58ab45..cf9bdbeef91 100644 --- a/spec/rubocop/cop/migration/safer_boolean_column_spec.rb +++ b/spec/rubocop/cop/migration/safer_boolean_column_spec.rb @@ -1,12 +1,9 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/safer_boolean_column' RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do - include CopHelper - subject(:cop) { described_class.new } context 'in migration' do @@ -31,11 +28,10 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do sources_and_offense.each do |source, offense| context "given the source \"#{source}\"" do it "registers the offense matching \"#{offense}\"" do - inspect_source(source) - - aggregate_failures do - expect(cop.offenses.first.message).to match(offense) - end + expect_offense(<<~RUBY, node: source, msg: offense) + %{node} + ^{node} Boolean columns on the `#{table}` table %{msg}.[...] + RUBY end end end @@ -48,11 +44,7 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do inoffensive_sources.each do |source| context "given the source \"#{source}\"" do it "registers no offense" do - inspect_source(source) - - aggregate_failures do - expect(cop.offenses).to be_empty - end + expect_no_offenses(source) end end end @@ -60,25 +52,19 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do end it 'registers no offense for tables not listed in SMALL_TABLES' do - inspect_source("add_column :large_table, :column, :boolean") - - expect(cop.offenses).to be_empty + expect_no_offenses("add_column :large_table, :column, :boolean") end it 'registers no offense for non-boolean columns' do table = described_class::SMALL_TABLES.sample - inspect_source("add_column :#{table}, :column, :string") - - expect(cop.offenses).to be_empty + expect_no_offenses("add_column :#{table}, :column, :string") end end context 'outside of migration' do it 'registers no offense' do table = described_class::SMALL_TABLES.sample - inspect_source("add_column :#{table}, :column, :boolean") - - expect(cop.offenses).to be_empty + expect_no_offenses("add_column :#{table}, :column, :boolean") end end end diff --git a/spec/rubocop/cop/migration/schedule_async_spec.rb b/spec/rubocop/cop/migration/schedule_async_spec.rb index a7246dfa73a..b89acb6db41 100644 --- a/spec/rubocop/cop/migration/schedule_async_spec.rb +++ b/spec/rubocop/cop/migration/schedule_async_spec.rb @@ -2,14 +2,9 @@ require 'fast_spec_helper' -require 'rubocop' -require 'rubocop/rspec/support' - require_relative '../../../../rubocop/cop/migration/schedule_async' RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do - include CopHelper - let(:cop) { described_class.new } let(:source) do <<~SOURCE @@ -21,9 +16,7 @@ RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do shared_examples 'a disabled cop' do it 'does not register any offenses' do - inspect_source(source) - - expect(cop.offenses).to be_empty + expect_no_offenses(source) end end @@ -50,101 +43,73 @@ RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do end context 'BackgroundMigrationWorker.perform_async' do - it 'adds an offence when calling `BackgroundMigrationWorker.peform_async`' do - inspect_source(source) - - expect(cop.offenses.size).to eq(1) - end - - it 'autocorrects to the right version' do - correct_source = <<~CORRECT - def up - migrate_async(ClazzName, "Bar", "Baz") - end - CORRECT + it 'adds an offense when calling `BackgroundMigrationWorker.peform_async` and corrects', :aggregate_failures do + expect_offense(<<~RUBY) + def up + BackgroundMigrationWorker.perform_async(ClazzName, "Bar", "Baz") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...] + end + RUBY - expect(autocorrect_source(source)).to eq(correct_source) + expect_correction(<<~RUBY) + def up + migrate_async(ClazzName, "Bar", "Baz") + end + RUBY end end context 'BackgroundMigrationWorker.perform_in' do - let(:source) do - <<~SOURCE + it 'adds an offense and corrects', :aggregate_failures do + expect_offense(<<~RUBY) def up BackgroundMigrationWorker + ^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...] .perform_in(delay, ClazzName, "Bar", "Baz") end - SOURCE - end - - it 'adds an offence' do - inspect_source(source) + RUBY - expect(cop.offenses.size).to eq(1) - end - - it 'autocorrects to the right version' do - correct_source = <<~CORRECT + expect_correction(<<~RUBY) def up migrate_in(delay, ClazzName, "Bar", "Baz") end - CORRECT - - expect(autocorrect_source(source)).to eq(correct_source) + RUBY end end context 'BackgroundMigrationWorker.bulk_perform_async' do - let(:source) do - <<~SOURCE + it 'adds an offense and corrects', :aggregate_failures do + expect_offense(<<~RUBY) def up BackgroundMigrationWorker + ^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...] .bulk_perform_async(jobs) end - SOURCE - end - - it 'adds an offence' do - inspect_source(source) - - expect(cop.offenses.size).to eq(1) - end + RUBY - it 'autocorrects to the right version' do - correct_source = <<~CORRECT + expect_correction(<<~RUBY) def up bulk_migrate_async(jobs) end - CORRECT - - expect(autocorrect_source(source)).to eq(correct_source) + RUBY end end context 'BackgroundMigrationWorker.bulk_perform_in' do - let(:source) do - <<~SOURCE + it 'adds an offense and corrects', :aggregate_failures do + expect_offense(<<~RUBY) def up BackgroundMigrationWorker + ^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...] .bulk_perform_in(5.minutes, jobs) end - SOURCE - end - - it 'adds an offence' do - inspect_source(source) + RUBY - expect(cop.offenses.size).to eq(1) - end - - it 'autocorrects to the right version' do - correct_source = <<~CORRECT + expect_correction(<<~RUBY) def up bulk_migrate_in(5.minutes, jobs) end - CORRECT - - expect(autocorrect_source(source)).to eq(correct_source) + RUBY end end end diff --git a/spec/rubocop/cop/migration/timestamps_spec.rb b/spec/rubocop/cop/migration/timestamps_spec.rb index 2f4154907d2..91bb5c1b05b 100644 --- a/spec/rubocop/cop/migration/timestamps_spec.rb +++ b/spec/rubocop/cop/migration/timestamps_spec.rb @@ -1,12 +1,9 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/timestamps' RSpec.describe RuboCop::Cop::Migration::Timestamps do - include CopHelper - subject(:cop) { described_class.new } let(:migration_with_timestamps) do @@ -62,38 +59,36 @@ RSpec.describe RuboCop::Cop::Migration::Timestamps do end it 'registers an offense when the "timestamps" method is used' do - inspect_source(migration_with_timestamps) - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([8]) - end + expect_offense(<<~RUBY) + class Users < ActiveRecord::Migration[4.2] + DOWNTIME = false + + def change + create_table :users do |t| + t.string :username, null: false + t.timestamps null: true + ^^^^^^^^^^ Do not use `timestamps`, use `timestamps_with_timezone` instead + t.string :password + end + end + end + RUBY end it 'does not register an offense when the "timestamps" method is not used' do - inspect_source(migration_without_timestamps) - - aggregate_failures do - expect(cop.offenses.size).to eq(0) - end + expect_no_offenses(migration_without_timestamps) end it 'does not register an offense when the "timestamps_with_timezone" method is used' do - inspect_source(migration_with_timestamps_with_timezone) - - aggregate_failures do - expect(cop.offenses.size).to eq(0) - end + expect_no_offenses(migration_with_timestamps_with_timezone) end end context 'outside of migration' do - it 'registers no offense' do - inspect_source(migration_with_timestamps) - inspect_source(migration_without_timestamps) - inspect_source(migration_with_timestamps_with_timezone) - - expect(cop.offenses.size).to eq(0) + it 'registers no offense', :aggregate_failures do + expect_no_offenses(migration_with_timestamps) + expect_no_offenses(migration_without_timestamps) + expect_no_offenses(migration_with_timestamps_with_timezone) end end end diff --git a/spec/rubocop/cop/migration/update_column_in_batches_spec.rb b/spec/rubocop/cop/migration/update_column_in_batches_spec.rb index 8049cba12d0..a12ae94c22b 100644 --- a/spec/rubocop/cop/migration/update_column_in_batches_spec.rb +++ b/spec/rubocop/cop/migration/update_column_in_batches_spec.rb @@ -2,9 +2,6 @@ require 'fast_spec_helper' -require 'rubocop' -require 'rubocop/rspec/support' - require_relative '../../../../rubocop/cop/migration/update_column_in_batches' RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do @@ -31,9 +28,7 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do context 'outside of a migration' do it 'does not register any offenses' do - inspect_source(migration_code) - - expect(cop.offenses).to be_empty + expect_no_offenses(migration_code) end end @@ -53,14 +48,14 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do let(:relative_spec_filepath) { Pathname.new(spec_filepath).relative_path_from(tmp_rails_root) } it 'registers an offense when using update_column_in_batches' do - inspect_source(migration_code, @migration_file) - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([2]) - expect(cop.offenses.first.message) - .to include("`#{relative_spec_filepath}`") - end + expect_offense(<<~RUBY, @migration_file) + def up + update_column_in_batches(:projects, :name, "foo") do |table, query| + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migration running `update_column_in_batches` [...] + query.where(table[:name].eq(nil)) + end + end + RUBY end end @@ -76,20 +71,18 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do end it 'does not register any offenses' do - inspect_source(migration_code, @migration_file) - - expect(cop.offenses).to be_empty + expect_no_offenses(migration_code) end end - context 'in a migration' do + context 'when in migration' do let(:migration_filepath) { File.join(tmp_rails_root, 'db', 'migrate', '20121220064453_my_super_migration.rb') } it_behaves_like 'a migration file with no spec file' it_behaves_like 'a migration file with a spec file' end - context 'in a post migration' do + context 'when in a post migration' do let(:migration_filepath) { File.join(tmp_rails_root, 'db', 'post_migrate', '20121220064453_my_super_migration.rb') } it_behaves_like 'a migration file with no spec file' @@ -99,14 +92,14 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do context 'EE migrations' do let(:spec_filepath) { File.join(tmp_rails_root, 'ee', 'spec', 'migrations', 'my_super_migration_spec.rb') } - context 'in a migration' do + context 'when in a migration' do let(:migration_filepath) { File.join(tmp_rails_root, 'ee', 'db', 'migrate', '20121220064453_my_super_migration.rb') } it_behaves_like 'a migration file with no spec file' it_behaves_like 'a migration file with a spec file' end - context 'in a post migration' do + context 'when in a post migration' do let(:migration_filepath) { File.join(tmp_rails_root, 'ee', 'db', 'post_migrate', '20121220064453_my_super_migration.rb') } it_behaves_like 'a migration file with no spec file' diff --git a/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb b/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb index 814d87ea24b..298ca273256 100644 --- a/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb +++ b/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb @@ -1,64 +1,58 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/with_lock_retries_disallowed_method' RSpec.describe RuboCop::Cop::Migration::WithLockRetriesDisallowedMethod do - include CopHelper - subject(:cop) { described_class.new } - context 'in migration' do + context 'when in migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end it 'registers an offense when `with_lock_retries` block has disallowed method' do - inspect_source('def change; with_lock_retries { disallowed_method }; end') - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end + expect_offense(<<~RUBY) + def change + with_lock_retries { disallowed_method } + ^^^^^^^^^^^^^^^^^ The method is not allowed [...] + end + RUBY end it 'registers an offense when `with_lock_retries` block has disallowed methods' do - source = <<~HEREDOC - def change - with_lock_retries do - disallowed_method + expect_offense(<<~RUBY) + def change + with_lock_retries do + disallowed_method + ^^^^^^^^^^^^^^^^^ The method is not allowed [...] - create_table do |t| - t.text :text - end + create_table do |t| + t.text :text + end - other_disallowed_method + other_disallowed_method + ^^^^^^^^^^^^^^^^^^^^^^^ The method is not allowed [...] - add_column :users, :name + add_column :users, :name + end end - end - HEREDOC - - inspect_source(source) - - aggregate_failures do - expect(cop.offenses.size).to eq(2) - expect(cop.offenses.map(&:line)).to eq([3, 9]) - end + RUBY end it 'registers no offense when `with_lock_retries` has only allowed method' do - inspect_source('def up; with_lock_retries { add_foreign_key :foo, :bar }; end') - - expect(cop.offenses.size).to eq(0) + expect_no_offenses(<<~RUBY) + def up + with_lock_retries { add_foreign_key :foo, :bar } + end + RUBY end describe 'for `add_foreign_key`' do it 'registers an offense when more than two FKs are added' do message = described_class::MSG_ONLY_ONE_FK_ALLOWED - expect_offense <<~RUBY + expect_offense(<<~RUBY) with_lock_retries do add_foreign_key :imports, :projects, column: :project_id, on_delete: :cascade ^^^^^^^^^^^^^^^ #{message} @@ -71,11 +65,13 @@ RSpec.describe RuboCop::Cop::Migration::WithLockRetriesDisallowedMethod do end end - context 'outside of migration' do + context 'when outside of migration' do it 'registers no offense' do - inspect_source('def change; with_lock_retries { disallowed_method }; end') - - expect(cop.offenses.size).to eq(0) + expect_no_offenses(<<~RUBY) + def change + with_lock_retries { disallowed_method } + end + RUBY end end end diff --git a/spec/rubocop/cop/migration/with_lock_retries_with_change_spec.rb b/spec/rubocop/cop/migration/with_lock_retries_with_change_spec.rb index f0be14c8ee9..f2e84a8697c 100644 --- a/spec/rubocop/cop/migration/with_lock_retries_with_change_spec.rb +++ b/spec/rubocop/cop/migration/with_lock_retries_with_change_spec.rb @@ -1,40 +1,41 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'rubocop' require_relative '../../../../rubocop/cop/migration/with_lock_retries_with_change' RSpec.describe RuboCop::Cop::Migration::WithLockRetriesWithChange do - include CopHelper - subject(:cop) { described_class.new } - context 'in migration' do + context 'when in migration' do before do allow(cop).to receive(:in_migration?).and_return(true) end it 'registers an offense when `with_lock_retries` is used inside a `change` method' do - inspect_source('def change; with_lock_retries {}; end') - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end + expect_offense(<<~RUBY) + def change + ^^^^^^ `with_lock_retries` cannot be used within `change` [...] + with_lock_retries {} + end + RUBY end it 'registers no offense when `with_lock_retries` is used inside an `up` method' do - inspect_source('def up; with_lock_retries {}; end') - - expect(cop.offenses.size).to eq(0) + expect_no_offenses(<<~RUBY) + def up + with_lock_retries {} + end + RUBY end end - context 'outside of migration' do + context 'when outside of migration' do it 'registers no offense' do - inspect_source('def change; with_lock_retries {}; end') - - expect(cop.offenses.size).to eq(0) + expect_no_offenses(<<~RUBY) + def change + with_lock_retries {} + end + RUBY end end end |