Commit 40cd5b8a authored by Doug Stull's avatar Doug Stull Committed by Douglas Barbosa Alexandre

Resolve cop_helper use offenses p4

- follow internal affairs cop.
parent c1de8637
...@@ -28,39 +28,6 @@ FactoryBot/InlineAssociation: ...@@ -28,39 +28,6 @@ FactoryBot/InlineAssociation:
- 'spec/factories/uploads.rb' - 'spec/factories/uploads.rb'
- 'spec/factories/wiki_pages.rb' - 'spec/factories/wiki_pages.rb'
InternalAffairs/DeprecateCopHelper: # issue to resolve: https://gitlab.com/gitlab-org/gitlab/-/issues/276734
Exclude:
- 'spec/rubocop/cop/migration/safer_boolean_column_spec.rb'
- 'spec/rubocop/cop/migration/remove_index_spec.rb'
- 'spec/rubocop/cop/migration/add_index_spec.rb'
- 'spec/rubocop/cop/migration/drop_table_spec.rb'
- 'spec/rubocop/cop/migration/hash_index_spec.rb'
- 'spec/rubocop/cop/migration/datetime_spec.rb'
- 'spec/rubocop/cop/migration/add_column_with_default_spec.rb'
- 'spec/rubocop/cop/migration/prevent_strings_spec.rb'
- 'spec/rubocop/cop/migration/add_timestamps_spec.rb'
- 'spec/rubocop/cop/migration/add_concurrent_index_spec.rb'
- 'spec/rubocop/cop/migration/update_column_in_batches_spec.rb'
- 'spec/rubocop/cop/migration/complex_indexes_require_name_spec.rb'
- 'spec/rubocop/cop/migration/refer_to_index_by_name_spec.rb'
- 'spec/rubocop/cop/migration/schedule_async_spec.rb'
- 'spec/rubocop/cop/migration/timestamps_spec.rb'
- 'spec/rubocop/cop/migration/remove_concurrent_index_spec.rb'
- 'spec/rubocop/cop/migration/add_columns_to_wide_tables_spec.rb'
- 'spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb'
- 'spec/rubocop/cop/migration/add_reference_spec.rb'
- 'spec/rubocop/cop/migration/remove_column_spec.rb'
- 'spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb'
- 'spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb'
- 'spec/rubocop/cop/migration/with_lock_retries_with_change_spec.rb'
- 'spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb'
- 'spec/rubocop/cop/avoid_return_from_blocks_spec.rb'
- 'spec/rubocop/cop/avoid_route_redirect_leading_slash_spec.rb'
- 'spec/rubocop/cop/put_group_routes_under_scope_spec.rb'
- 'spec/rubocop/cop/sidekiq_options_queue_spec.rb'
- 'spec/rubocop/cop/ignored_columns_spec.rb'
- 'spec/rubocop/cop/prefer_class_methods_over_module_spec.rb'
Rails/SaveBang: Rails/SaveBang:
Exclude: Exclude:
- 'ee/spec/controllers/projects/merge_requests_controller_spec.rb' - 'ee/spec/controllers/projects/merge_requests_controller_spec.rb'
......
...@@ -9,8 +9,8 @@ module RuboCop ...@@ -9,8 +9,8 @@ module RuboCop
class AddColumnsToWideTables < RuboCop::Cop::Cop class AddColumnsToWideTables < RuboCop::Cop::Cop
include MigrationHelpers include MigrationHelpers
MSG = '`%s` is a wide table with several columns, addig more should be avoided unless absolutely necessary.' \ MSG = '`%s` is a wide table with several columns, adding more should be avoided unless absolutely necessary.' \
' Consider storing the column in a different table or creating a new one.'.freeze ' Consider storing the column in a different table or creating a new one.'
BLACKLISTED_METHODS = %i[ BLACKLISTED_METHODS = %i[
add_column add_column
......
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../rubocop/cop/avoid_return_from_blocks' require_relative '../../../rubocop/cop/avoid_return_from_blocks'
RSpec.describe RuboCop::Cop::AvoidReturnFromBlocks do RSpec.describe RuboCop::Cop::AvoidReturnFromBlocks do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
it 'flags violation for return inside a block' do it 'flags violation for return inside a block' do
...@@ -19,20 +17,16 @@ RSpec.describe RuboCop::Cop::AvoidReturnFromBlocks do ...@@ -19,20 +17,16 @@ RSpec.describe RuboCop::Cop::AvoidReturnFromBlocks do
RUBY RUBY
end end
it "doesn't call add_offense twice for nested blocks" do it "doesn't create more than one offense for nested blocks" do
source = <<~RUBY expect_offense(<<~RUBY)
call do call do
call do call do
something something
return if something_else return if something_else
^^^^^^ Do not return from a block, use next or break instead.
end end
end end
RUBY RUBY
expect_any_instance_of(described_class) do |instance|
expect(instance).to receive(:add_offense).once
end
inspect_source(source)
end end
it 'flags violation for return inside included > def > block' do it 'flags violation for return inside included > def > block' do
......
...@@ -5,19 +5,21 @@ require 'rubocop' ...@@ -5,19 +5,21 @@ require 'rubocop'
require_relative '../../../rubocop/cop/avoid_route_redirect_leading_slash' require_relative '../../../rubocop/cop/avoid_route_redirect_leading_slash'
RSpec.describe RuboCop::Cop::AvoidRouteRedirectLeadingSlash do RSpec.describe RuboCop::Cop::AvoidRouteRedirectLeadingSlash do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
before do before do
allow(cop).to receive(:in_routes?).and_return(true) allow(cop).to receive(:in_routes?).and_return(true)
end 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) expect_offense(<<~PATTERN)
root to: redirect("/-/route") root to: redirect("/-/route")
^^^^^^^^^^^^^^^^^^^^ Do not use a leading "/" in route redirects ^^^^^^^^^^^^^^^^^^^^ Do not use a leading "/" in route redirects
PATTERN PATTERN
expect_correction(<<~PATTERN)
root to: redirect("-/route")
PATTERN
end end
it 'does not register an offense when redirect does not have a leading slash' do 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 ...@@ -25,8 +27,4 @@ RSpec.describe RuboCop::Cop::AvoidRouteRedirectLeadingSlash do
root to: redirect("-/route") root to: redirect("-/route")
PATTERN PATTERN
end end
it 'autocorrect `/-/route` to `-/route`' do
expect(autocorrect_source('redirect("/-/route")')).to eq('redirect("-/route")')
end
end end
...@@ -2,21 +2,17 @@ ...@@ -2,21 +2,17 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/ignored_columns' require_relative '../../../rubocop/cop/ignored_columns'
RSpec.describe RuboCop::Cop::IgnoredColumns do RSpec.describe RuboCop::Cop::IgnoredColumns do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
it 'flags the use of destroy_all with a local variable receiver' do it 'flags direct use of ignored_columns instead of the IgnoredColumns concern' do
inspect_source(<<~RUBY) expect_offense(<<~RUBY)
class Foo < ApplicationRecord class Foo < ApplicationRecord
self.ignored_columns += %i[id] self.ignored_columns += %i[id]
^^^^^^^^^^^^^^^^^^^^ Use `IgnoredColumns` concern instead of adding to `self.ignored_columns`.
end end
RUBY RUBY
expect(cop.offenses.size).to eq(1)
end end
end end
...@@ -5,11 +5,9 @@ require 'rubocop' ...@@ -5,11 +5,9 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/add_column_with_default' require_relative '../../../../rubocop/cop/migration/add_column_with_default'
RSpec.describe RuboCop::Cop::Migration::AddColumnWithDefault do RSpec.describe RuboCop::Cop::Migration::AddColumnWithDefault do
include CopHelper
let(:cop) { described_class.new } 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 it 'does not register any offenses' do
expect_no_offenses(<<~RUBY) expect_no_offenses(<<~RUBY)
def up def up
...@@ -19,18 +17,16 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnWithDefault do ...@@ -19,18 +17,16 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnWithDefault do
end end
end end
context 'in a migration' do context 'when in a migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
let(:offense) { '`add_column_with_default` is deprecated, use `add_column` instead' }
it 'registers an offense' do it 'registers an offense' do
expect_offense(<<~RUBY) expect_offense(<<~RUBY)
def up def up
add_column_with_default(:merge_request_diff_files, :artifacts, :boolean, default: true, allow_null: false) 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 end
RUBY RUBY
end end
......
...@@ -5,11 +5,9 @@ require 'rubocop' ...@@ -5,11 +5,9 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/add_columns_to_wide_tables' require_relative '../../../../rubocop/cop/migration/add_columns_to_wide_tables'
RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do
include CopHelper
let(:cop) { described_class.new } 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 it 'does not register any offenses' do
expect_no_offenses(<<~RUBY) expect_no_offenses(<<~RUBY)
def up def up
...@@ -19,14 +17,14 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do ...@@ -19,14 +17,14 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do
end end
end end
context 'in a migration' do context 'when in a migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
context 'with wide tables' do context 'with wide tables' do
it 'registers an offense when adding a column to a wide table' 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) expect_offense(<<~RUBY)
def up def up
...@@ -37,7 +35,7 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do ...@@ -37,7 +35,7 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do
end end
it 'registers an offense when adding a column with default to a wide table' do 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) expect_offense(<<~RUBY)
def up def up
...@@ -48,7 +46,7 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do ...@@ -48,7 +46,7 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do
end end
it 'registers an offense when adding a reference' do 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) expect_offense(<<~RUBY)
def up def up
...@@ -59,7 +57,7 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do ...@@ -59,7 +57,7 @@ RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do
end end
it 'registers an offense when adding timestamps' do 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) expect_offense(<<~RUBY)
def up def up
......
...@@ -5,46 +5,38 @@ require 'rubocop' ...@@ -5,46 +5,38 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/add_concurrent_foreign_key' require_relative '../../../../rubocop/cop/migration/add_concurrent_foreign_key'
RSpec.describe RuboCop::Cop::Migration::AddConcurrentForeignKey do RSpec.describe RuboCop::Cop::Migration::AddConcurrentForeignKey do
include CopHelper
let(:cop) { described_class.new } 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 it 'does not register any offenses' do
inspect_source('def up; add_foreign_key(:projects, :users, column: :user_id); end') expect_no_offenses('def up; add_foreign_key(:projects, :users, column: :user_id); end')
expect(cop.offenses).to be_empty
end end
end end
context 'in a migration' do context 'when in a migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
it 'registers an offense when using add_foreign_key' do it 'registers an offense when using add_foreign_key' do
inspect_source('def up; add_foreign_key(:projects, :users, column: :user_id); end') expect_offense(<<~RUBY)
def up
aggregate_failures do add_foreign_key(:projects, :users, column: :user_id)
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^^^^ `add_foreign_key` requires downtime, use `add_concurrent_foreign_key` instead
expect(cop.offenses.map(&:line)).to eq([1])
end end
RUBY
end end
it 'does not register an offense when a `NOT VALID` foreign key is added' do 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_no_offenses('def up; add_foreign_key(:projects, :users, column: :user_id, validate: false); end')
expect(cop.offenses).to be_empty
end end
it 'does not register an offense when `add_foreign_key` is within `with_lock_retries`' do 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 with_lock_retries do
add_foreign_key :key, :projects, column: :project_id, on_delete: :cascade add_foreign_key :key, :projects, column: :project_id, on_delete: :cascade
end end
RUBY RUBY
expect(cop.offenses).to be_empty
end end
end end
end end
...@@ -5,36 +5,30 @@ require 'rubocop' ...@@ -5,36 +5,30 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/add_concurrent_index' require_relative '../../../../rubocop/cop/migration/add_concurrent_index'
RSpec.describe RuboCop::Cop::Migration::AddConcurrentIndex do RSpec.describe RuboCop::Cop::Migration::AddConcurrentIndex do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'in migration' do context 'when in migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
it 'registers an offense when add_concurrent_index is used inside a change method' do 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') expect_offense(<<~RUBY)
def change
aggregate_failures do ^^^^^^ `add_concurrent_index` is not reversible[...]
expect(cop.offenses.size).to eq(1) add_concurrent_index :table, :column
expect(cop.offenses.map(&:line)).to eq([1])
end end
RUBY
end end
it 'registers no offense when add_concurrent_index is used inside an up method' do 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_no_offenses('def up; add_concurrent_index :table, :column; end')
expect(cop.offenses.size).to eq(0)
end end
end end
context 'outside of migration' do context 'when outside of migration' do
it 'registers no offense' do it 'registers no offense' do
inspect_source('def change; add_concurrent_index :table, :column; end') expect_no_offenses('def change; add_concurrent_index :table, :column; end')
expect(cop.offenses.size).to eq(0)
end end
end end
end end
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/add_index' require_relative '../../../../rubocop/cop/migration/add_index'
RSpec.describe RuboCop::Cop::Migration::AddIndex do RSpec.describe RuboCop::Cop::Migration::AddIndex do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'in migration' do context 'in migration' do
......
...@@ -5,11 +5,11 @@ require 'rubocop' ...@@ -5,11 +5,11 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/add_limit_to_text_columns' require_relative '../../../../rubocop/cop/migration/add_limit_to_text_columns'
RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
include CopHelper
subject(:cop) { described_class.new } 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 before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
...@@ -25,31 +25,29 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do ...@@ -25,31 +25,29 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
create_table :test_text_limits, id: false do |t| create_table :test_text_limits, id: false do |t|
t.integer :test_id, null: false t.integer :test_id, null: false
t.text :name t.text :name
^^^^ #{described_class::MSG} ^^^^ #{msg}
end end
create_table_with_constraints :test_text_limits_create do |t| create_table_with_constraints :test_text_limits_create do |t|
t.integer :test_id, null: false t.integer :test_id, null: false
t.text :title t.text :title
t.text :description t.text :description
^^^^ #{described_class::MSG} ^^^^ #{msg}
t.text_limit :title, 100 t.text_limit :title, 100
end end
add_column :test_text_limits, :email, :text add_column :test_text_limits, :email, :text
^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^ #{msg}
add_column_with_default :test_text_limits, :role, :text, default: 'default' 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 change_column_type_concurrently :test_text_limits, :test_id, :text
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg}
end end
end end
RUBY RUBY
expect(cop.offenses.map(&:cop_name)).to all(eq('Migration/AddLimitToTextColumns'))
end end
end end
...@@ -111,7 +109,7 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do ...@@ -111,7 +109,7 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
end end
# Make sure that the cop is properly checking for an `add_text_limit` # 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 context 'when the limit is defined for a same name attribute but different table' do
it 'registers an offense' do it 'registers an offense' do
expect_offense(<<~RUBY) expect_offense(<<~RUBY)
...@@ -123,17 +121,17 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do ...@@ -123,17 +121,17 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
create_table :test_text_limits, id: false do |t| create_table :test_text_limits, id: false do |t|
t.integer :test_id, null: false t.integer :test_id, null: false
t.text :name t.text :name
^^^^ #{described_class::MSG} ^^^^ #{msg}
end end
add_column :test_text_limits, :email, :text add_column :test_text_limits, :email, :text
^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^ #{msg}
add_column_with_default :test_text_limits, :role, :text, default: 'default' 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 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, :name, 255
add_text_limit :wrong_table, :email, 255 add_text_limit :wrong_table, :email, 255
...@@ -142,8 +140,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do ...@@ -142,8 +140,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
end end
end end
RUBY RUBY
expect(cop.offenses.map(&:cop_name)).to all(eq('Migration/AddLimitToTextColumns'))
end end
end end
...@@ -176,18 +172,18 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do ...@@ -176,18 +172,18 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
DOWNTIME = false DOWNTIME = false
def up def up
drop_table :no_offence_on_down drop_table :no_offense_on_down
end end
def down 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.integer :test_id, null: false
t.text :name t.text :name
end 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
end end
RUBY RUBY
...@@ -195,7 +191,7 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do ...@@ -195,7 +191,7 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
end end
end end
context 'outside of migration' do context 'when outside of migration' do
it 'registers no offense' do it 'registers no offense' do
expect_no_offenses(<<~RUBY) expect_no_offenses(<<~RUBY)
class TestTextLimits < ActiveRecord::Migration[6.0] class TestTextLimits < ActiveRecord::Migration[6.0]
......
...@@ -5,11 +5,9 @@ require 'rubocop' ...@@ -5,11 +5,9 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/add_reference' require_relative '../../../../rubocop/cop/migration/add_reference'
RSpec.describe RuboCop::Cop::Migration::AddReference do RSpec.describe RuboCop::Cop::Migration::AddReference do
include CopHelper
let(:cop) { described_class.new } 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 it 'does not register any offenses' do
expect_no_offenses(<<~RUBY) expect_no_offenses(<<~RUBY)
def up def up
...@@ -19,12 +17,12 @@ RSpec.describe RuboCop::Cop::Migration::AddReference do ...@@ -19,12 +17,12 @@ RSpec.describe RuboCop::Cop::Migration::AddReference do
end end
end end
context 'in a migration' do context 'when in a migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end 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 context 'when the table existed before' do
it 'registers an offense when using add_reference' do it 'registers an offense when using add_reference' do
......
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/add_timestamps' require_relative '../../../../rubocop/cop/migration/add_timestamps'
RSpec.describe RuboCop::Cop::Migration::AddTimestamps do RSpec.describe RuboCop::Cop::Migration::AddTimestamps do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
let(:migration_with_add_timestamps) do let(:migration_with_add_timestamps) do
...@@ -47,44 +45,39 @@ RSpec.describe RuboCop::Cop::Migration::AddTimestamps do ...@@ -47,44 +45,39 @@ RSpec.describe RuboCop::Cop::Migration::AddTimestamps do
) )
end end
context 'in migration' do context 'when in migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
it 'registers an offense when the "add_timestamps" method is used' do it 'registers an offense when the "add_timestamps" method is used' do
inspect_source(migration_with_add_timestamps) expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[4.2]
DOWNTIME = false
aggregate_failures do def change
expect(cop.offenses.size).to eq(1) add_column(:users, :username, :text)
expect(cop.offenses.map(&:line)).to eq([7]) add_timestamps(:users)
^^^^^^^^^^^^^^ Do not use `add_timestamps`, use `add_timestamps_with_timezone` instead
end end
end end
RUBY
end
it 'does not register an offense when the "add_timestamps" method is not used' do it 'does not register an offense when the "add_timestamps" method is not used' do
inspect_source(migration_without_add_timestamps) expect_no_offenses(migration_without_add_timestamps)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end
end end
it 'does not register an offense when the "add_timestamps_with_timezone" method is used' do it 'does not register an offense when the "add_timestamps_with_timezone" method is used' do
inspect_source(migration_with_add_timestamps_with_timezone) expect_no_offenses(migration_with_add_timestamps_with_timezone)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end
end end
end end
context 'outside of migration' do context 'when outside of migration' do
it 'registers no offense' do it 'registers no offense', :aggregate_failures do
inspect_source(migration_with_add_timestamps) expect_no_offenses(migration_with_add_timestamps)
inspect_source(migration_without_add_timestamps) expect_no_offenses(migration_without_add_timestamps)
inspect_source(migration_with_add_timestamps_with_timezone) expect_no_offenses(migration_with_add_timestamps_with_timezone)
expect(cop.offenses.size).to eq(0)
end end
end end
end end
...@@ -5,11 +5,11 @@ require 'rubocop' ...@@ -5,11 +5,11 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/complex_indexes_require_name' require_relative '../../../../rubocop/cop/migration/complex_indexes_require_name'
RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
include CopHelper
subject(:cop) { described_class.new } 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 before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
...@@ -29,9 +29,9 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do ...@@ -29,9 +29,9 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
t.index :column1, unique: true t.index :column1, unique: true
t.index :column2, where: 'column1 = 0' t.index :column2, where: 'column1 = 0'
^^^^^ #{described_class::MSG} ^^^^^ #{msg}
t.index :column3, using: :gin t.index :column3, using: :gin
^^^^^ #{described_class::MSG} ^^^^^ #{msg}
end end
end end
...@@ -40,8 +40,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do ...@@ -40,8 +40,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
end end
end end
RUBY RUBY
expect(cop.offenses.map(&:cop_name)).to all(eq("Migration/#{described_class.name.demodulize}"))
end end
end end
...@@ -85,20 +83,18 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do ...@@ -85,20 +83,18 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
add_index :test_indexes, :column1 add_index :test_indexes, :column1
add_index :test_indexes, :column2, where: "column2 = 'value'", order: { column4: :desc } add_index :test_indexes, :column2, where: "column2 = 'value'", order: { column4: :desc }
^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^ #{msg}
end end
def down def down
add_index :test_indexes, :column4, 'unique' => true, where: 'column4 IS NOT NULL' 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 add_concurrent_index :test_indexes, :column6, using: :gin, opclass: :gin_trgm_ops
^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^^^^^^^^^^^ #{msg}
end end
end end
RUBY RUBY
expect(cop.offenses.map(&:cop_name)).to all(eq("Migration/#{described_class.name.demodulize}"))
end end
end end
...@@ -132,7 +128,7 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do ...@@ -132,7 +128,7 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
end end
end end
context 'outside migration' do context 'when outside migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(false) allow(cop).to receive(:in_migration?).and_return(false)
end end
......
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/create_table_with_foreign_keys' require_relative '../../../../rubocop/cop/migration/create_table_with_foreign_keys'
RSpec.describe RuboCop::Cop::Migration::CreateTableWithForeignKeys do RSpec.describe RuboCop::Cop::Migration::CreateTableWithForeignKeys do
include CopHelper
let(:cop) { described_class.new } let(:cop) { described_class.new }
context 'outside of a migration' do context 'outside of a migration' do
...@@ -22,7 +20,7 @@ RSpec.describe RuboCop::Cop::Migration::CreateTableWithForeignKeys do ...@@ -22,7 +20,7 @@ RSpec.describe RuboCop::Cop::Migration::CreateTableWithForeignKeys do
end end
end end
context 'in a migration' do context 'when in a migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
......
...@@ -5,40 +5,8 @@ require 'rubocop' ...@@ -5,40 +5,8 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/datetime' require_relative '../../../../rubocop/cop/migration/datetime'
RSpec.describe RuboCop::Cop::Migration::Datetime do RSpec.describe RuboCop::Cop::Migration::Datetime do
include CopHelper
subject(:cop) { described_class.new } 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 let(:create_table_migration_without_datetime) do
%q( %q(
class Users < ActiveRecord::Migration[6.0] class Users < ActiveRecord::Migration[6.0]
...@@ -120,92 +88,94 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do ...@@ -120,92 +88,94 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
) )
end end
context 'in migration' do context 'when in migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
it 'registers an offense when the ":datetime" data type is used on create_table' do it 'registers an offense when the ":datetime" data type is used on create_table' do
inspect_source(create_table_migration_with_datetime) expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
aggregate_failures do def change
expect(cop.offenses.size).to eq(1) create_table :users do |t|
expect(cop.offenses.map(&:line)).to eq([8]) t.string :username, null: false
expect(cop.offenses.first.message).to include('`datetime`') t.datetime :last_sign_in
^^^^^^^^ Do not use the `datetime` data type[...]
end
end end
end end
RUBY
end
it 'registers an offense when the ":timestamp" data type is used on create_table' do it 'registers an offense when the ":timestamp" data type is used on create_table' do
inspect_source(create_table_migration_with_timestamp) expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
aggregate_failures do def change
expect(cop.offenses.size).to eq(1) create_table :users do |t|
expect(cop.offenses.map(&:line)).to eq([8]) t.string :username, null: false
expect(cop.offenses.first.message).to include('timestamp') t.timestamp :last_sign_in
^^^^^^^^^ Do not use the `timestamp` data type[...]
end
end end
end end
RUBY
end
it 'does not register an offense when the ":datetime" data type is not used on create_table' do 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) expect_no_offenses(create_table_migration_without_datetime)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end
end end
it 'does not register an offense when the ":datetime_with_timezone" data type is used on create_table' do 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) expect_no_offenses(create_table_migration_with_datetime_with_timezone)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end
end end
it 'registers an offense when the ":datetime" data type is used on add_column' do it 'registers an offense when the ":datetime" data type is used on add_column' do
inspect_source(add_column_migration_with_datetime) expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
aggregate_failures do def change
expect(cop.offenses.size).to eq(1) add_column(:users, :username, :text)
expect(cop.offenses.map(&:line)).to eq([7]) add_column(:users, :last_sign_in, :datetime)
expect(cop.offenses.first.message).to include('`datetime`') ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use the `datetime` data type[...]
end end
end end
RUBY
end
it 'registers an offense when the ":timestamp" data type is used on add_column' do it 'registers an offense when the ":timestamp" data type is used on add_column' do
inspect_source(add_column_migration_with_timestamp) expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
aggregate_failures do def change
expect(cop.offenses.size).to eq(1) add_column(:users, :username, :text)
expect(cop.offenses.map(&:line)).to eq([7]) add_column(:users, :last_sign_in, :timestamp)
expect(cop.offenses.first.message).to include('timestamp') ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use the `timestamp` data type[...]
end end
end end
RUBY
end
it 'does not register an offense when the ":datetime" data type is not used on add_column' do 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) expect_no_offenses(add_column_migration_without_datetime)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end
end end
it 'does not register an offense when the ":datetime_with_timezone" data type is used on add_column' do 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) expect_no_offenses(add_column_migration_with_datetime_with_timezone)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end end
end 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 end
end end
...@@ -5,11 +5,13 @@ require 'rubocop' ...@@ -5,11 +5,13 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/drop_table' require_relative '../../../../rubocop/cop/migration/drop_table'
RSpec.describe RuboCop::Cop::Migration::DropTable do RSpec.describe RuboCop::Cop::Migration::DropTable do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'when in deployment migration' do 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 before do
allow(cop).to receive(:in_deployment_migration?).and_return(true) allow(cop).to receive(:in_deployment_migration?).and_return(true)
end end
...@@ -30,7 +32,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do ...@@ -30,7 +32,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do
expect_offense(<<~PATTERN) expect_offense(<<~PATTERN)
def up def up
drop_table :table drop_table :table
^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^ #{msg}
end end
PATTERN PATTERN
end end
...@@ -41,7 +43,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do ...@@ -41,7 +43,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do
expect_offense(<<~PATTERN) expect_offense(<<~PATTERN)
def change def change
drop_table :table drop_table :table
^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^ #{msg}
end end
PATTERN PATTERN
end end
...@@ -63,7 +65,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do ...@@ -63,7 +65,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do
expect_offense(<<~PATTERN) expect_offense(<<~PATTERN)
def up def up
execute "DROP TABLE table" execute "DROP TABLE table"
^^^^^^^ #{described_class::MSG} ^^^^^^^ #{msg}
end end
PATTERN PATTERN
end end
...@@ -74,7 +76,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do ...@@ -74,7 +76,7 @@ RSpec.describe RuboCop::Cop::Migration::DropTable do
expect_offense(<<~PATTERN) expect_offense(<<~PATTERN)
def change def change
execute "DROP TABLE table" execute "DROP TABLE table"
^^^^^^^ #{described_class::MSG} ^^^^^^^ #{msg}
end end
PATTERN PATTERN
end end
......
...@@ -5,48 +5,44 @@ require 'rubocop' ...@@ -5,48 +5,44 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/hash_index' require_relative '../../../../rubocop/cop/migration/hash_index'
RSpec.describe RuboCop::Cop::Migration::HashIndex do RSpec.describe RuboCop::Cop::Migration::HashIndex do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'in migration' do context 'when in migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
it 'registers an offense when creating a hash index' do it 'registers an offense when creating a hash index' do
inspect_source('def change; add_index :table, :column, using: :hash; end') expect_offense(<<~RUBY)
def change
aggregate_failures do add_index :table, :column, using: :hash
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^ hash indexes should be avoided at all costs[...]
expect(cop.offenses.map(&:line)).to eq([1])
end end
RUBY
end end
it 'registers an offense when creating a concurrent hash index' do it 'registers an offense when creating a concurrent hash index' do
inspect_source('def change; add_concurrent_index :table, :column, using: :hash; end') expect_offense(<<~RUBY)
def change
aggregate_failures do add_concurrent_index :table, :column, using: :hash
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^ hash indexes should be avoided at all costs[...]
expect(cop.offenses.map(&:line)).to eq([1])
end end
RUBY
end end
it 'registers an offense when creating a hash index using t.index' do it 'registers an offense when creating a hash index using t.index' do
inspect_source('def change; t.index :table, :column, using: :hash; end') expect_offense(<<~RUBY)
def change
aggregate_failures do t.index :table, :column, using: :hash
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^ hash indexes should be avoided at all costs[...]
expect(cop.offenses.map(&:line)).to eq([1])
end end
RUBY
end end
end end
context 'outside of migration' do context 'when outside of migration' do
it 'registers no offense' do it 'registers no offense' do
inspect_source('def change; index :table, :column, using: :hash; end') expect_no_offenses('def change; index :table, :column, using: :hash; end')
expect(cop.offenses.size).to eq(0)
end end
end end
end end
...@@ -5,45 +5,41 @@ require 'rubocop' ...@@ -5,45 +5,41 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/prevent_strings' require_relative '../../../../rubocop/cop/migration/prevent_strings'
RSpec.describe RuboCop::Cop::Migration::PreventStrings do RSpec.describe RuboCop::Cop::Migration::PreventStrings do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'in migration' do context 'when in migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
context 'when the string data type is used' do context 'when the string data type is used' do
it 'registers an offense' 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] class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false DOWNTIME = false
def up def up
create_table :users do |t| create_table :users do |t|
t.string :username, null: false t.string :username, null: false
^^^^^^ #{described_class::MSG} ^^^^^^ %{msg}
t.timestamps_with_timezone null: true t.timestamps_with_timezone null: true
t.string :password t.string :password
^^^^^^ #{described_class::MSG} ^^^^^^ %{msg}
end end
add_column(:users, :bio, :string) add_column(:users, :bio, :string)
^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^ %{msg}
add_column_with_default(:users, :url, :string, default: '/-/user', allow_null: false, limit: 255) 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 change_column_type_concurrently :users, :commit_id, :string
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ %{msg}
end end
end end
RUBY RUBY
expect(cop.offenses.map(&:cop_name)).to all(eq('Migration/PreventStrings'))
end end
end end
...@@ -109,7 +105,7 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do ...@@ -109,7 +105,7 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
end end
end end
context 'on down' do context 'when using down method' do
it 'registers no offense' do it 'registers no offense' do
expect_no_offenses(<<~RUBY) expect_no_offenses(<<~RUBY)
class Users < ActiveRecord::Migration[6.0] class Users < ActiveRecord::Migration[6.0]
...@@ -138,7 +134,7 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do ...@@ -138,7 +134,7 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
end end
end end
context 'outside of migration' do context 'when outside of migration' do
it 'registers no offense' do it 'registers no offense' do
expect_no_offenses(<<~RUBY) expect_no_offenses(<<~RUBY)
class Users < ActiveRecord::Migration[6.0] class Users < ActiveRecord::Migration[6.0]
......
...@@ -5,18 +5,16 @@ require 'rubocop' ...@@ -5,18 +5,16 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/refer_to_index_by_name' require_relative '../../../../rubocop/cop/migration/refer_to_index_by_name'
RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'in migration' do context 'when in migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
context 'when existing indexes are referred to without an explicit name' do context 'when existing indexes are referred to without an explicit name' do
it 'registers an offense' 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] class TestReferToIndexByName < ActiveRecord::Migration[6.0]
DOWNTIME = false DOWNTIME = false
...@@ -30,22 +28,22 @@ RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do ...@@ -30,22 +28,22 @@ RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do
end end
if index_exists? :test_indexes, :column2 if index_exists? :test_indexes, :column2
^^^^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^^^^ %{msg}
remove_index :test_indexes, :column2 remove_index :test_indexes, :column2
^^^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^^^ %{msg}
end end
remove_index :test_indexes, column: column3 remove_index :test_indexes, column: column3
^^^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^^^ %{msg}
remove_index :test_indexes, name: 'index_name_4' remove_index :test_indexes, name: 'index_name_4'
end end
def down def down
if index_exists? :test_indexes, :column4, using: :gin, opclass: :gin_trgm_ops 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 remove_concurrent_index :test_indexes, :column4, using: :gin, opclass: :gin_trgm_ops
^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^^^^^^^^^^^^^^ %{msg}
end end
if index_exists? :test_indexes, :column3, unique: true, name: 'index_name_3', where: 'column3 = 10' 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 ...@@ -54,13 +52,11 @@ RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do
end end
end end
RUBY RUBY
expect(cop.offenses.map(&:cop_name)).to all(eq("Migration/#{described_class.name.demodulize}"))
end end
end end
end end
context 'outside migration' do context 'when outside migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(false) allow(cop).to receive(:in_migration?).and_return(false)
end end
......
...@@ -5,63 +5,55 @@ require 'rubocop' ...@@ -5,63 +5,55 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/remove_column' require_relative '../../../../rubocop/cop/migration/remove_column'
RSpec.describe RuboCop::Cop::Migration::RemoveColumn do RSpec.describe RuboCop::Cop::Migration::RemoveColumn do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
def source(meth = 'change') def source(meth = 'change')
"def #{meth}; remove_column :table, :column; end" "def #{meth}; remove_column :table, :column; end"
end end
context 'in a regular migration' do context 'when in a regular migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
allow(cop).to receive(:in_post_deployment_migration?).and_return(false) allow(cop).to receive(:in_post_deployment_migration?).and_return(false)
end end
it 'registers an offense when remove_column is used in the change method' do it 'registers an offense when remove_column is used in the change method' do
inspect_source(source('change')) expect_offense(<<~RUBY)
def change
aggregate_failures do remove_column :table, :column
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^^ `remove_column` must only be used in post-deployment migrations
expect(cop.offenses.map(&:line)).to eq([1])
end end
RUBY
end end
it 'registers an offense when remove_column is used in the up method' do it 'registers an offense when remove_column is used in the up method' do
inspect_source(source('up')) expect_offense(<<~RUBY)
def up
aggregate_failures do remove_column :table, :column
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^^ `remove_column` must only be used in post-deployment migrations
expect(cop.offenses.map(&:line)).to eq([1])
end end
RUBY
end end
it 'registers no offense when remove_column is used in the down method' do it 'registers no offense when remove_column is used in the down method' do
inspect_source(source('down')) expect_no_offenses(source('down'))
expect(cop.offenses.size).to eq(0)
end end
end end
context 'in a post-deployment migration' do context 'when in a post-deployment migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
allow(cop).to receive(:in_post_deployment_migration?).and_return(true) allow(cop).to receive(:in_post_deployment_migration?).and_return(true)
end end
it 'registers no offense' do it 'registers no offense' do
inspect_source(source) expect_no_offenses(source)
expect(cop.offenses.size).to eq(0)
end end
end end
context 'outside of a migration' do context 'when outside of a migration' do
it 'registers no offense' do it 'registers no offense' do
inspect_source(source) expect_no_offenses(source)
expect(cop.offenses.size).to eq(0)
end end
end end
end end
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/remove_concurrent_index' require_relative '../../../../rubocop/cop/migration/remove_concurrent_index'
RSpec.describe RuboCop::Cop::Migration::RemoveConcurrentIndex do RSpec.describe RuboCop::Cop::Migration::RemoveConcurrentIndex do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'in migration' do context 'in migration' do
...@@ -15,26 +13,22 @@ RSpec.describe RuboCop::Cop::Migration::RemoveConcurrentIndex do ...@@ -15,26 +13,22 @@ RSpec.describe RuboCop::Cop::Migration::RemoveConcurrentIndex do
end end
it 'registers an offense when remove_concurrent_index is used inside a change method' do 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') expect_offense(<<~RUBY)
def change
aggregate_failures do ^^^^^^ `remove_concurrent_index` is not reversible [...]
expect(cop.offenses.size).to eq(1) remove_concurrent_index :table, :column
expect(cop.offenses.map(&:line)).to eq([1])
end end
RUBY
end end
it 'registers no offense when remove_concurrent_index is used inside an up method' do 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_no_offenses('def up; remove_concurrent_index :table, :column; end')
expect(cop.offenses.size).to eq(0)
end end
end end
context 'outside of migration' do context 'outside of migration' do
it 'registers no offense' do it 'registers no offense' do
inspect_source('def change; remove_concurrent_index :table, :column; end') expect_no_offenses('def change; remove_concurrent_index :table, :column; end')
expect(cop.offenses.size).to eq(0)
end end
end end
end end
...@@ -5,30 +5,26 @@ require 'rubocop' ...@@ -5,30 +5,26 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/remove_index' require_relative '../../../../rubocop/cop/migration/remove_index'
RSpec.describe RuboCop::Cop::Migration::RemoveIndex do RSpec.describe RuboCop::Cop::Migration::RemoveIndex do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'in migration' do context 'when in migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
it 'registers an offense when remove_index is used' do it 'registers an offense when remove_index is used' do
inspect_source('def change; remove_index :table, :column; end') expect_offense(<<~RUBY)
def change
aggregate_failures do remove_index :table, :column
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^ `remove_index` requires downtime, use `remove_concurrent_index` instead
expect(cop.offenses.map(&:line)).to eq([1])
end end
RUBY
end end
end end
context 'outside of migration' do context 'when outside of migration' do
it 'registers no offense' do it 'registers no offense' do
inspect_source('def change; remove_index :table, :column; end') expect_no_offenses('def change; remove_index :table, :column; end')
expect(cop.offenses.size).to eq(0)
end end
end end
end end
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/safer_boolean_column' require_relative '../../../../rubocop/cop/migration/safer_boolean_column'
RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'in migration' do context 'in migration' do
...@@ -31,11 +29,10 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do ...@@ -31,11 +29,10 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do
sources_and_offense.each do |source, offense| sources_and_offense.each do |source, offense|
context "given the source \"#{source}\"" do context "given the source \"#{source}\"" do
it "registers the offense matching \"#{offense}\"" do it "registers the offense matching \"#{offense}\"" do
inspect_source(source) expect_offense(<<~RUBY, node: source, msg: offense)
%{node}
aggregate_failures do ^{node} Boolean columns on the `#{table}` table %{msg}.[...]
expect(cop.offenses.first.message).to match(offense) RUBY
end
end end
end end
end end
...@@ -48,11 +45,7 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do ...@@ -48,11 +45,7 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do
inoffensive_sources.each do |source| inoffensive_sources.each do |source|
context "given the source \"#{source}\"" do context "given the source \"#{source}\"" do
it "registers no offense" do it "registers no offense" do
inspect_source(source) expect_no_offenses(source)
aggregate_failures do
expect(cop.offenses).to be_empty
end
end end
end end
end end
...@@ -60,25 +53,19 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do ...@@ -60,25 +53,19 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn do
end end
it 'registers no offense for tables not listed in SMALL_TABLES' do it 'registers no offense for tables not listed in SMALL_TABLES' do
inspect_source("add_column :large_table, :column, :boolean") expect_no_offenses("add_column :large_table, :column, :boolean")
expect(cop.offenses).to be_empty
end end
it 'registers no offense for non-boolean columns' do it 'registers no offense for non-boolean columns' do
table = described_class::SMALL_TABLES.sample table = described_class::SMALL_TABLES.sample
inspect_source("add_column :#{table}, :column, :string") expect_no_offenses("add_column :#{table}, :column, :string")
expect(cop.offenses).to be_empty
end end
end end
context 'outside of migration' do context 'outside of migration' do
it 'registers no offense' do it 'registers no offense' do
table = described_class::SMALL_TABLES.sample table = described_class::SMALL_TABLES.sample
inspect_source("add_column :#{table}, :column, :boolean") expect_no_offenses("add_column :#{table}, :column, :boolean")
expect(cop.offenses).to be_empty
end end
end end
end end
...@@ -3,13 +3,9 @@ ...@@ -3,13 +3,9 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/migration/schedule_async' require_relative '../../../../rubocop/cop/migration/schedule_async'
RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do
include CopHelper
let(:cop) { described_class.new } let(:cop) { described_class.new }
let(:source) do let(:source) do
<<~SOURCE <<~SOURCE
...@@ -21,9 +17,7 @@ RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do ...@@ -21,9 +17,7 @@ RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do
shared_examples 'a disabled cop' do shared_examples 'a disabled cop' do
it 'does not register any offenses' do it 'does not register any offenses' do
inspect_source(source) expect_no_offenses(source)
expect(cop.offenses).to be_empty
end end
end end
...@@ -50,101 +44,73 @@ RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do ...@@ -50,101 +44,73 @@ RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do
end end
context 'BackgroundMigrationWorker.perform_async' do context 'BackgroundMigrationWorker.perform_async' do
it 'adds an offence when calling `BackgroundMigrationWorker.peform_async`' do it 'adds an offense when calling `BackgroundMigrationWorker.peform_async` and corrects', :aggregate_failures do
inspect_source(source) expect_offense(<<~RUBY)
def up
expect(cop.offenses.size).to eq(1) BackgroundMigrationWorker.perform_async(ClazzName, "Bar", "Baz")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...]
end end
RUBY
it 'autocorrects to the right version' do expect_correction(<<~RUBY)
correct_source = <<~CORRECT
def up def up
migrate_async(ClazzName, "Bar", "Baz") migrate_async(ClazzName, "Bar", "Baz")
end end
CORRECT RUBY
expect(autocorrect_source(source)).to eq(correct_source)
end end
end end
context 'BackgroundMigrationWorker.perform_in' do context 'BackgroundMigrationWorker.perform_in' do
let(:source) do it 'adds an offense and corrects', :aggregate_failures do
<<~SOURCE expect_offense(<<~RUBY)
def up def up
BackgroundMigrationWorker BackgroundMigrationWorker
^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...]
.perform_in(delay, ClazzName, "Bar", "Baz") .perform_in(delay, ClazzName, "Bar", "Baz")
end end
SOURCE RUBY
end
it 'adds an offence' do
inspect_source(source)
expect(cop.offenses.size).to eq(1)
end
it 'autocorrects to the right version' do expect_correction(<<~RUBY)
correct_source = <<~CORRECT
def up def up
migrate_in(delay, ClazzName, "Bar", "Baz") migrate_in(delay, ClazzName, "Bar", "Baz")
end end
CORRECT RUBY
expect(autocorrect_source(source)).to eq(correct_source)
end end
end end
context 'BackgroundMigrationWorker.bulk_perform_async' do context 'BackgroundMigrationWorker.bulk_perform_async' do
let(:source) do it 'adds an offense and corrects', :aggregate_failures do
<<~SOURCE expect_offense(<<~RUBY)
def up def up
BackgroundMigrationWorker BackgroundMigrationWorker
^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...]
.bulk_perform_async(jobs) .bulk_perform_async(jobs)
end end
SOURCE RUBY
end
it 'adds an offence' do
inspect_source(source)
expect(cop.offenses.size).to eq(1)
end
it 'autocorrects to the right version' do expect_correction(<<~RUBY)
correct_source = <<~CORRECT
def up def up
bulk_migrate_async(jobs) bulk_migrate_async(jobs)
end end
CORRECT RUBY
expect(autocorrect_source(source)).to eq(correct_source)
end end
end end
context 'BackgroundMigrationWorker.bulk_perform_in' do context 'BackgroundMigrationWorker.bulk_perform_in' do
let(:source) do it 'adds an offense and corrects', :aggregate_failures do
<<~SOURCE expect_offense(<<~RUBY)
def up def up
BackgroundMigrationWorker BackgroundMigrationWorker
^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...]
.bulk_perform_in(5.minutes, jobs) .bulk_perform_in(5.minutes, jobs)
end end
SOURCE RUBY
end
it 'adds an offence' do
inspect_source(source)
expect(cop.offenses.size).to eq(1) expect_correction(<<~RUBY)
end
it 'autocorrects to the right version' do
correct_source = <<~CORRECT
def up def up
bulk_migrate_in(5.minutes, jobs) bulk_migrate_in(5.minutes, jobs)
end end
CORRECT RUBY
expect(autocorrect_source(source)).to eq(correct_source)
end end
end end
end end
......
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/timestamps' require_relative '../../../../rubocop/cop/migration/timestamps'
RSpec.describe RuboCop::Cop::Migration::Timestamps do RSpec.describe RuboCop::Cop::Migration::Timestamps do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
let(:migration_with_timestamps) do let(:migration_with_timestamps) do
...@@ -62,38 +60,36 @@ RSpec.describe RuboCop::Cop::Migration::Timestamps do ...@@ -62,38 +60,36 @@ RSpec.describe RuboCop::Cop::Migration::Timestamps do
end end
it 'registers an offense when the "timestamps" method is used' do it 'registers an offense when the "timestamps" method is used' do
inspect_source(migration_with_timestamps) expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[4.2]
DOWNTIME = false
aggregate_failures do def change
expect(cop.offenses.size).to eq(1) create_table :users do |t|
expect(cop.offenses.map(&:line)).to eq([8]) t.string :username, null: false
t.timestamps null: true
^^^^^^^^^^ Do not use `timestamps`, use `timestamps_with_timezone` instead
t.string :password
end end
end end
end
RUBY
end
it 'does not register an offense when the "timestamps" method is not used' do it 'does not register an offense when the "timestamps" method is not used' do
inspect_source(migration_without_timestamps) expect_no_offenses(migration_without_timestamps)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end
end end
it 'does not register an offense when the "timestamps_with_timezone" method is used' do it 'does not register an offense when the "timestamps_with_timezone" method is used' do
inspect_source(migration_with_timestamps_with_timezone) expect_no_offenses(migration_with_timestamps_with_timezone)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end
end end
end end
context 'outside of migration' do context 'outside of migration' do
it 'registers no offense' do it 'registers no offense', :aggregate_failures do
inspect_source(migration_with_timestamps) expect_no_offenses(migration_with_timestamps)
inspect_source(migration_without_timestamps) expect_no_offenses(migration_without_timestamps)
inspect_source(migration_with_timestamps_with_timezone) expect_no_offenses(migration_with_timestamps_with_timezone)
expect(cop.offenses.size).to eq(0)
end end
end end
end end
...@@ -3,8 +3,6 @@ ...@@ -3,8 +3,6 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/migration/update_column_in_batches' require_relative '../../../../rubocop/cop/migration/update_column_in_batches'
RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do
...@@ -31,9 +29,7 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do ...@@ -31,9 +29,7 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do
context 'outside of a migration' do context 'outside of a migration' do
it 'does not register any offenses' do it 'does not register any offenses' do
inspect_source(migration_code) expect_no_offenses(migration_code)
expect(cop.offenses).to be_empty
end end
end end
...@@ -53,15 +49,15 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do ...@@ -53,15 +49,15 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do
let(:relative_spec_filepath) { Pathname.new(spec_filepath).relative_path_from(tmp_rails_root) } 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 it 'registers an offense when using update_column_in_batches' do
inspect_source(migration_code, @migration_file) expect_offense(<<~RUBY, @migration_file)
def up
aggregate_failures do update_column_in_batches(:projects, :name, "foo") do |table, query|
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migration running `update_column_in_batches` [...]
expect(cop.offenses.map(&:line)).to eq([2]) query.where(table[:name].eq(nil))
expect(cop.offenses.first.message)
.to include("`#{relative_spec_filepath}`")
end end
end end
RUBY
end
end end
shared_examples 'a migration file with a spec file' do shared_examples 'a migration file with a spec file' do
...@@ -76,20 +72,18 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do ...@@ -76,20 +72,18 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do
end end
it 'does not register any offenses' do it 'does not register any offenses' do
inspect_source(migration_code, @migration_file) expect_no_offenses(migration_code)
expect(cop.offenses).to be_empty
end end
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') } 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 no spec file'
it_behaves_like 'a migration file with a spec file' it_behaves_like 'a migration file with a spec file'
end 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') } 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' it_behaves_like 'a migration file with no spec file'
...@@ -99,14 +93,14 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do ...@@ -99,14 +93,14 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do
context 'EE migrations' do context 'EE migrations' do
let(:spec_filepath) { File.join(tmp_rails_root, 'ee', 'spec', 'migrations', 'my_super_migration_spec.rb') } 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') } 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 no spec file'
it_behaves_like 'a migration file with a spec file' it_behaves_like 'a migration file with a spec file'
end 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') } 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' it_behaves_like 'a migration file with no spec file'
......
...@@ -5,60 +5,55 @@ require 'rubocop' ...@@ -5,60 +5,55 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/with_lock_retries_disallowed_method' require_relative '../../../../rubocop/cop/migration/with_lock_retries_disallowed_method'
RSpec.describe RuboCop::Cop::Migration::WithLockRetriesDisallowedMethod do RSpec.describe RuboCop::Cop::Migration::WithLockRetriesDisallowedMethod do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'in migration' do context 'when in migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
it 'registers an offense when `with_lock_retries` block has disallowed method' do it 'registers an offense when `with_lock_retries` block has disallowed method' do
inspect_source('def change; with_lock_retries { disallowed_method }; end') expect_offense(<<~RUBY)
def change
aggregate_failures do with_lock_retries { disallowed_method }
expect(cop.offenses.size).to eq(1) ^^^^^^^^^^^^^^^^^ The method is not allowed [...]
expect(cop.offenses.map(&:line)).to eq([1])
end end
RUBY
end end
it 'registers an offense when `with_lock_retries` block has disallowed methods' do it 'registers an offense when `with_lock_retries` block has disallowed methods' do
source = <<~HEREDOC expect_offense(<<~RUBY)
def change def change
with_lock_retries do with_lock_retries do
disallowed_method disallowed_method
^^^^^^^^^^^^^^^^^ The method is not allowed [...]
create_table do |t| create_table do |t|
t.text :text t.text :text
end end
other_disallowed_method other_disallowed_method
^^^^^^^^^^^^^^^^^^^^^^^ The method is not allowed [...]
add_column :users, :name add_column :users, :name
end end
end end
HEREDOC RUBY
inspect_source(source)
aggregate_failures do
expect(cop.offenses.size).to eq(2)
expect(cop.offenses.map(&:line)).to eq([3, 9])
end
end end
it 'registers no offense when `with_lock_retries` has only allowed method' do 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_no_offenses(<<~RUBY)
def up
expect(cop.offenses.size).to eq(0) with_lock_retries { add_foreign_key :foo, :bar }
end
RUBY
end end
describe 'for `add_foreign_key`' do describe 'for `add_foreign_key`' do
it 'registers an offense when more than two FKs are added' do it 'registers an offense when more than two FKs are added' do
message = described_class::MSG_ONLY_ONE_FK_ALLOWED message = described_class::MSG_ONLY_ONE_FK_ALLOWED
expect_offense <<~RUBY expect_offense(<<~RUBY)
with_lock_retries do with_lock_retries do
add_foreign_key :imports, :projects, column: :project_id, on_delete: :cascade add_foreign_key :imports, :projects, column: :project_id, on_delete: :cascade
^^^^^^^^^^^^^^^ #{message} ^^^^^^^^^^^^^^^ #{message}
...@@ -71,11 +66,13 @@ RSpec.describe RuboCop::Cop::Migration::WithLockRetriesDisallowedMethod do ...@@ -71,11 +66,13 @@ RSpec.describe RuboCop::Cop::Migration::WithLockRetriesDisallowedMethod do
end end
end end
context 'outside of migration' do context 'when outside of migration' do
it 'registers no offense' do it 'registers no offense' do
inspect_source('def change; with_lock_retries { disallowed_method }; end') expect_no_offenses(<<~RUBY)
def change
expect(cop.offenses.size).to eq(0) with_lock_retries { disallowed_method }
end
RUBY
end end
end end
end end
...@@ -5,36 +5,38 @@ require 'rubocop' ...@@ -5,36 +5,38 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/migration/with_lock_retries_with_change' require_relative '../../../../rubocop/cop/migration/with_lock_retries_with_change'
RSpec.describe RuboCop::Cop::Migration::WithLockRetriesWithChange do RSpec.describe RuboCop::Cop::Migration::WithLockRetriesWithChange do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'in migration' do context 'when in migration' do
before do before do
allow(cop).to receive(:in_migration?).and_return(true) allow(cop).to receive(:in_migration?).and_return(true)
end end
it 'registers an offense when `with_lock_retries` is used inside a `change` method' do it 'registers an offense when `with_lock_retries` is used inside a `change` method' do
inspect_source('def change; with_lock_retries {}; end') expect_offense(<<~RUBY)
def change
aggregate_failures do ^^^^^^ `with_lock_retries` cannot be used within `change` [...]
expect(cop.offenses.size).to eq(1) with_lock_retries {}
expect(cop.offenses.map(&:line)).to eq([1])
end end
RUBY
end end
it 'registers no offense when `with_lock_retries` is used inside an `up` method' do it 'registers no offense when `with_lock_retries` is used inside an `up` method' do
inspect_source('def up; with_lock_retries {}; end') expect_no_offenses(<<~RUBY)
def up
expect(cop.offenses.size).to eq(0) with_lock_retries {}
end
RUBY
end end
end end
context 'outside of migration' do context 'when outside of migration' do
it 'registers no offense' do it 'registers no offense' do
inspect_source('def change; with_lock_retries {}; end') expect_no_offenses(<<~RUBY)
def change
expect(cop.offenses.size).to eq(0) with_lock_retries {}
end
RUBY
end end
end end
end end
...@@ -2,15 +2,12 @@ ...@@ -2,15 +2,12 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/prefer_class_methods_over_module' require_relative '../../../rubocop/cop/prefer_class_methods_over_module'
RSpec.describe RuboCop::Cop::PreferClassMethodsOverModule do RSpec.describe RuboCop::Cop::PreferClassMethodsOverModule do
include CopHelper
subject(:cop) { described_class.new } 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) expect_offense(<<~RUBY)
module Foo module Foo
extend ActiveSupport::Concern extend ActiveSupport::Concern
...@@ -22,6 +19,17 @@ RSpec.describe RuboCop::Cop::PreferClassMethodsOverModule do ...@@ -22,6 +19,17 @@ RSpec.describe RuboCop::Cop::PreferClassMethodsOverModule do
end end
end end
RUBY RUBY
expect_correction(<<~RUBY)
module Foo
extend ActiveSupport::Concern
class_methods do
def a_class_method
end
end
end
RUBY
end end
it "doesn't flag violation when using class_methods" do it "doesn't flag violation when using class_methods" do
...@@ -69,30 +77,4 @@ RSpec.describe RuboCop::Cop::PreferClassMethodsOverModule do ...@@ -69,30 +77,4 @@ RSpec.describe RuboCop::Cop::PreferClassMethodsOverModule do
end end
RUBY RUBY
end 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 end
...@@ -5,8 +5,6 @@ require 'rubocop' ...@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../rubocop/cop/put_group_routes_under_scope' require_relative '../../../rubocop/cop/put_group_routes_under_scope'
RSpec.describe RuboCop::Cop::PutGroupRoutesUnderScope do RSpec.describe RuboCop::Cop::PutGroupRoutesUnderScope do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
%w[resource resources get post put patch delete].each do |route_method| %w[resource resources get post put patch delete].each do |route_method|
......
...@@ -3,28 +3,19 @@ ...@@ -3,28 +3,19 @@
require 'fast_spec_helper' require 'fast_spec_helper'
require 'rubocop' require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/sidekiq_options_queue' require_relative '../../../rubocop/cop/sidekiq_options_queue'
RSpec.describe RuboCop::Cop::SidekiqOptionsQueue do RSpec.describe RuboCop::Cop::SidekiqOptionsQueue do
include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
it 'registers an offense when `sidekiq_options` is used with the `queue` option' do it 'registers an offense when `sidekiq_options` is used with the `queue` option' do
inspect_source('sidekiq_options queue: "some_queue"') expect_offense(<<~CODE)
sidekiq_options queue: "some_queue"
aggregate_failures do ^^^^^^^^^^^^^^^^^^^ Do not manually set a queue; `ApplicationWorker` sets one automatically.
expect(cop.offenses.size).to eq(1) CODE
expect(cop.offenses.map(&:line)).to eq([1])
expect(cop.highlights).to eq(['queue: "some_queue"'])
end
end end
it 'does not register an offense when `sidekiq_options` is used with another option' do it 'does not register an offense when `sidekiq_options` is used with another option' do
inspect_source('sidekiq_options retry: false') expect_no_offenses('sidekiq_options retry: false')
expect(cop.offenses).to be_empty
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment