diff options
Diffstat (limited to 'spec/rubocop')
30 files changed, 376 insertions, 574 deletions
diff --git a/spec/rubocop/cop/avoid_return_from_blocks_spec.rb b/spec/rubocop/cop/avoid_return_from_blocks_spec.rb index 71311b9df7f..526a362447f 100644 --- a/spec/rubocop/cop/avoid_return_from_blocks_spec.rb +++ b/spec/rubocop/cop/avoid_return_from_blocks_spec.rb @@ -5,8 +5,6 @@ require 'rubocop' require_relative '../../../rubocop/cop/avoid_return_from_blocks' RSpec.describe RuboCop::Cop::AvoidReturnFromBlocks do - include CopHelper - subject(:cop) { described_class.new } it 'flags violation for return inside a block' do @@ -19,20 +17,16 @@ RSpec.describe RuboCop::Cop::AvoidReturnFromBlocks do RUBY end - it "doesn't call add_offense twice for nested blocks" do - source = <<~RUBY + it "doesn't create more than one offense for nested blocks" do + expect_offense(<<~RUBY) call do call do something return if something_else + ^^^^^^ Do not return from a block, use next or break instead. end end RUBY - expect_any_instance_of(described_class) do |instance| - expect(instance).to receive(:add_offense).once - end - - inspect_source(source) end it 'flags violation for return inside included > def > block' do diff --git a/spec/rubocop/cop/avoid_route_redirect_leading_slash_spec.rb b/spec/rubocop/cop/avoid_route_redirect_leading_slash_spec.rb index 9e13a5278e3..c049528523e 100644 --- a/spec/rubocop/cop/avoid_route_redirect_leading_slash_spec.rb +++ b/spec/rubocop/cop/avoid_route_redirect_leading_slash_spec.rb @@ -5,19 +5,21 @@ require 'rubocop' require_relative '../../../rubocop/cop/avoid_route_redirect_leading_slash' RSpec.describe RuboCop::Cop::AvoidRouteRedirectLeadingSlash do - include CopHelper - subject(:cop) { described_class.new } before do allow(cop).to receive(:in_routes?).and_return(true) end - it 'registers an offense when redirect has a leading slash' do + it 'registers an offense when redirect has a leading slash and corrects', :aggregate_failures do expect_offense(<<~PATTERN) root to: redirect("/-/route") ^^^^^^^^^^^^^^^^^^^^ Do not use a leading "/" in route redirects PATTERN + + expect_correction(<<~PATTERN) + root to: redirect("-/route") + PATTERN end it 'does not register an offense when redirect does not have a leading slash' do @@ -25,8 +27,4 @@ RSpec.describe RuboCop::Cop::AvoidRouteRedirectLeadingSlash do root to: redirect("-/route") PATTERN end - - it 'autocorrect `/-/route` to `-/route`' do - expect(autocorrect_source('redirect("/-/route")')).to eq('redirect("-/route")') - end end diff --git a/spec/rubocop/cop/ignored_columns_spec.rb b/spec/rubocop/cop/ignored_columns_spec.rb index 38b4ac0bc1a..37fec7cb62a 100644 --- a/spec/rubocop/cop/ignored_columns_spec.rb +++ b/spec/rubocop/cop/ignored_columns_spec.rb @@ -2,21 +2,17 @@ require 'fast_spec_helper' require 'rubocop' -require 'rubocop/rspec/support' require_relative '../../../rubocop/cop/ignored_columns' RSpec.describe RuboCop::Cop::IgnoredColumns do - include CopHelper - subject(:cop) { described_class.new } - it 'flags the use of destroy_all with a local variable receiver' do - inspect_source(<<~RUBY) + it 'flags direct use of ignored_columns instead of the IgnoredColumns concern' do + expect_offense(<<~RUBY) class Foo < ApplicationRecord self.ignored_columns += %i[id] + ^^^^^^^^^^^^^^^^^^^^ Use `IgnoredColumns` concern instead of adding to `self.ignored_columns`. end RUBY - - expect(cop.offenses.size).to eq(1) end end 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..68f219e01f9 100644 --- a/spec/rubocop/cop/migration/add_column_with_default_spec.rb +++ b/spec/rubocop/cop/migration/add_column_with_default_spec.rb @@ -5,11 +5,9 @@ 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 +17,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..70d9a577728 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 @@ -5,11 +5,9 @@ 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 +17,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 +35,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 +46,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 +57,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..2da29606024 100644 --- a/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb +++ b/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb @@ -5,46 +5,38 @@ 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..bc5fd2de4fd 100644 --- a/spec/rubocop/cop/migration/add_concurrent_index_spec.rb +++ b/spec/rubocop/cop/migration/add_concurrent_index_spec.rb @@ -5,36 +5,30 @@ 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..894fbc25eb8 100644 --- a/spec/rubocop/cop/migration/add_index_spec.rb +++ b/spec/rubocop/cop/migration/add_index_spec.rb @@ -5,8 +5,6 @@ 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..4f107f7600f 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 @@ -5,11 +5,11 @@ 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 +25,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 +109,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 +121,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 +140,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 +172,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 +191,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..35386f14e26 100644 --- a/spec/rubocop/cop/migration/add_reference_spec.rb +++ b/spec/rubocop/cop/migration/add_reference_spec.rb @@ -5,11 +5,9 @@ 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 +17,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..6492c93d4e6 100644 --- a/spec/rubocop/cop/migration/add_timestamps_spec.rb +++ b/spec/rubocop/cop/migration/add_timestamps_spec.rb @@ -5,8 +5,6 @@ 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 +45,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..0a95bd5d78c 100644 --- a/spec/rubocop/cop/migration/complex_indexes_require_name_spec.rb +++ b/spec/rubocop/cop/migration/complex_indexes_require_name_spec.rb @@ -5,11 +5,11 @@ 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 +29,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 +40,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 +83,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 +128,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..d046fd913d4 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 @@ -5,8 +5,6 @@ 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 +20,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..24be3bea8c3 100644 --- a/spec/rubocop/cop/migration/datetime_spec.rb +++ b/spec/rubocop/cop/migration/datetime_spec.rb @@ -5,40 +5,8 @@ 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 +88,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..7a2e7929f11 100644 --- a/spec/rubocop/cop/migration/drop_table_spec.rb +++ b/spec/rubocop/cop/migration/drop_table_spec.rb @@ -5,11 +5,13 @@ 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 +32,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 +43,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 +65,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 +76,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..08c803ee79e 100644 --- a/spec/rubocop/cop/migration/hash_index_spec.rb +++ b/spec/rubocop/cop/migration/hash_index_spec.rb @@ -5,48 +5,44 @@ 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..d8d4058363e 100644 --- a/spec/rubocop/cop/migration/prevent_strings_spec.rb +++ b/spec/rubocop/cop/migration/prevent_strings_spec.rb @@ -5,45 +5,41 @@ 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 +105,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 +134,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..864d2325d53 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 @@ -5,18 +5,16 @@ 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 +28,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 +52,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..a8a828bc62a 100644 --- a/spec/rubocop/cop/migration/remove_column_spec.rb +++ b/spec/rubocop/cop/migration/remove_column_spec.rb @@ -5,63 +5,55 @@ 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..8e29e51992c 100644 --- a/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb +++ b/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb @@ -5,8 +5,6 @@ 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 +13,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..c9797443276 100644 --- a/spec/rubocop/cop/migration/remove_index_spec.rb +++ b/spec/rubocop/cop/migration/remove_index_spec.rb @@ -5,30 +5,26 @@ 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..11661be2bb6 100644 --- a/spec/rubocop/cop/migration/safer_boolean_column_spec.rb +++ b/spec/rubocop/cop/migration/safer_boolean_column_spec.rb @@ -5,8 +5,6 @@ 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 +29,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 +45,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 +53,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..008a23c7b8f 100644 --- a/spec/rubocop/cop/migration/schedule_async_spec.rb +++ b/spec/rubocop/cop/migration/schedule_async_spec.rb @@ -3,13 +3,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 +17,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 +44,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..4da3dffbfc6 100644 --- a/spec/rubocop/cop/migration/timestamps_spec.rb +++ b/spec/rubocop/cop/migration/timestamps_spec.rb @@ -5,8 +5,6 @@ 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 +60,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..5e33e11b13f 100644 --- a/spec/rubocop/cop/migration/update_column_in_batches_spec.rb +++ b/spec/rubocop/cop/migration/update_column_in_batches_spec.rb @@ -3,8 +3,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 +29,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 +49,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 +72,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 +93,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..429490294be 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 @@ -5,60 +5,55 @@ 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 +66,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..cf2d46d52c9 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 @@ -5,36 +5,38 @@ 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 diff --git a/spec/rubocop/cop/prefer_class_methods_over_module_spec.rb b/spec/rubocop/cop/prefer_class_methods_over_module_spec.rb index dc665f9dd25..5a557ec26d6 100644 --- a/spec/rubocop/cop/prefer_class_methods_over_module_spec.rb +++ b/spec/rubocop/cop/prefer_class_methods_over_module_spec.rb @@ -2,15 +2,12 @@ require 'fast_spec_helper' require 'rubocop' -require 'rubocop/rspec/support' require_relative '../../../rubocop/cop/prefer_class_methods_over_module' RSpec.describe RuboCop::Cop::PreferClassMethodsOverModule do - include CopHelper - subject(:cop) { described_class.new } - it 'flags violation when using module ClassMethods' do + it 'flags violation when using module ClassMethods and corrects', :aggregate_failures do expect_offense(<<~RUBY) module Foo extend ActiveSupport::Concern @@ -22,6 +19,17 @@ RSpec.describe RuboCop::Cop::PreferClassMethodsOverModule do end end RUBY + + expect_correction(<<~RUBY) + module Foo + extend ActiveSupport::Concern + + class_methods do + def a_class_method + end + end + end + RUBY end it "doesn't flag violation when using class_methods" do @@ -69,30 +77,4 @@ RSpec.describe RuboCop::Cop::PreferClassMethodsOverModule do end RUBY end - - it 'autocorrects ClassMethods into class_methods' do - source = <<~RUBY - module Foo - extend ActiveSupport::Concern - - module ClassMethods - def a_class_method - end - end - end - RUBY - autocorrected = autocorrect_source(source) - - expected_source = <<~RUBY - module Foo - extend ActiveSupport::Concern - - class_methods do - def a_class_method - end - end - end - RUBY - expect(autocorrected).to eq(expected_source) - end end diff --git a/spec/rubocop/cop/put_group_routes_under_scope_spec.rb b/spec/rubocop/cop/put_group_routes_under_scope_spec.rb index 46b50d7690b..f9e64d4d68f 100644 --- a/spec/rubocop/cop/put_group_routes_under_scope_spec.rb +++ b/spec/rubocop/cop/put_group_routes_under_scope_spec.rb @@ -5,8 +5,6 @@ require 'rubocop' require_relative '../../../rubocop/cop/put_group_routes_under_scope' RSpec.describe RuboCop::Cop::PutGroupRoutesUnderScope do - include CopHelper - subject(:cop) { described_class.new } %w[resource resources get post put patch delete].each do |route_method| @@ -15,12 +13,12 @@ RSpec.describe RuboCop::Cop::PutGroupRoutesUnderScope do marker = '^' * offense.size expect_offense(<<~PATTERN) - scope(path: 'groups/*group_id/-', module: :groups) do - resource :issues - end + scope(path: 'groups/*group_id/-', module: :groups) do + resource :issues + end - #{offense} - #{marker} Put new group routes under /-/ scope + #{offense} + #{marker} Put new group routes under /-/ scope PATTERN end end diff --git a/spec/rubocop/cop/sidekiq_options_queue_spec.rb b/spec/rubocop/cop/sidekiq_options_queue_spec.rb index 306cbcf62b5..e65118ca192 100644 --- a/spec/rubocop/cop/sidekiq_options_queue_spec.rb +++ b/spec/rubocop/cop/sidekiq_options_queue_spec.rb @@ -3,28 +3,19 @@ require 'fast_spec_helper' require 'rubocop' -require 'rubocop/rspec/support' - require_relative '../../../rubocop/cop/sidekiq_options_queue' RSpec.describe RuboCop::Cop::SidekiqOptionsQueue do - include CopHelper - subject(:cop) { described_class.new } it 'registers an offense when `sidekiq_options` is used with the `queue` option' do - inspect_source('sidekiq_options queue: "some_queue"') - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - expect(cop.highlights).to eq(['queue: "some_queue"']) - end + expect_offense(<<~CODE) + sidekiq_options queue: "some_queue" + ^^^^^^^^^^^^^^^^^^^ Do not manually set a queue; `ApplicationWorker` sets one automatically. + CODE end it 'does not register an offense when `sidekiq_options` is used with another option' do - inspect_source('sidekiq_options retry: false') - - expect(cop.offenses).to be_empty + expect_no_offenses('sidekiq_options retry: false') end end |