diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-19 01:45:44 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-19 01:45:44 +0000 |
commit | 85dc423f7090da0a52c73eb66faf22ddb20efff9 (patch) | |
tree | 9160f299afd8c80c038f08e1545be119f5e3f1e1 /spec/rubocop | |
parent | 15c2c8c66dbe422588e5411eee7e68f1fa440bb8 (diff) | |
download | gitlab-ce-85dc423f7090da0a52c73eb66faf22ddb20efff9.tar.gz |
Add latest changes from gitlab-org/gitlab@13-4-stable-ee
Diffstat (limited to 'spec/rubocop')
16 files changed, 678 insertions, 34 deletions
diff --git a/spec/rubocop/cop/active_record_association_reload_spec.rb b/spec/rubocop/cop/active_record_association_reload_spec.rb index 79053a79c5a..e8d46064b49 100644 --- a/spec/rubocop/cop/active_record_association_reload_spec.rb +++ b/spec/rubocop/cop/active_record_association_reload_spec.rb @@ -15,7 +15,7 @@ RSpec.describe RuboCop::Cop::ActiveRecordAssociationReload, type: :rubocop do users = User.all users.reload ^^^^^^ Use reset instead of reload. For more details check the https://gitlab.com/gitlab-org/gitlab-foss/issues/60218. - PATTERN + PATTERN end it 'does not register an offense on reset usage' do diff --git a/spec/rubocop/cop/gitlab/avoid_uploaded_file_from_params_spec.rb b/spec/rubocop/cop/gitlab/avoid_uploaded_file_from_params_spec.rb new file mode 100644 index 00000000000..8341b0cab3a --- /dev/null +++ b/spec/rubocop/cop/gitlab/avoid_uploaded_file_from_params_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/gitlab/avoid_uploaded_file_from_params' + +RSpec.describe RuboCop::Cop::Gitlab::AvoidUploadedFileFromParams, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + context 'UploadedFile.from_params' do + it 'flags its call' do + expect_offense(<<~SOURCE) + UploadedFile.from_params(params) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `UploadedFile` set by `multipart.rb` instead of calling `UploadedFile.from_params` directly. See https://docs.gitlab.com/ee/development/uploads.html#how-to-add-a-new-upload-route + SOURCE + end + end +end diff --git a/spec/rubocop/cop/gitlab/bulk_insert_spec.rb b/spec/rubocop/cop/gitlab/bulk_insert_spec.rb index 2766e4f1982..d1236865897 100644 --- a/spec/rubocop/cop/gitlab/bulk_insert_spec.rb +++ b/spec/rubocop/cop/gitlab/bulk_insert_spec.rb @@ -13,7 +13,14 @@ RSpec.describe RuboCop::Cop::Gitlab::BulkInsert, type: :rubocop do it 'flags the use of Gitlab::Database.bulk_insert' do expect_offense(<<~SOURCE) Gitlab::Database.bulk_insert('merge_request_diff_files', rows) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `BulkInsertSafe` concern, instead of using `Gitlab::Database.bulk_insert`. See https://docs.gitlab.com/ee/development/insert_into_tables_in_batches.html + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{RuboCop::Cop::Gitlab::BulkInsert::MSG} + SOURCE + end + + it 'flags the use of ::Gitlab::Database.bulk_insert' do + expect_offense(<<~SOURCE) + ::Gitlab::Database.bulk_insert('merge_request_diff_files', rows) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{RuboCop::Cop::Gitlab::BulkInsert::MSG} SOURCE end end diff --git a/spec/rubocop/cop/gitlab/except_spec.rb b/spec/rubocop/cop/gitlab/except_spec.rb new file mode 100644 index 00000000000..50277d15a57 --- /dev/null +++ b/spec/rubocop/cop/gitlab/except_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/gitlab/except' + +RSpec.describe RuboCop::Cop::Gitlab::Except, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + it 'flags the use of Gitlab::SQL::Except.new' do + expect_offense(<<~SOURCE) + Gitlab::SQL::Except.new([foo]) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `FromExcept` concern, instead of using `Gitlab::SQL::Except` directly + SOURCE + end +end diff --git a/spec/rubocop/cop/gitlab/intersect_spec.rb b/spec/rubocop/cop/gitlab/intersect_spec.rb new file mode 100644 index 00000000000..351033d0ed2 --- /dev/null +++ b/spec/rubocop/cop/gitlab/intersect_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/gitlab/intersect' + +RSpec.describe RuboCop::Cop::Gitlab::Intersect, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + it 'flags the use of Gitlab::SQL::Intersect.new' do + expect_offense(<<~SOURCE) + Gitlab::SQL::Intersect.new([foo]) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `FromIntersect` concern, instead of using `Gitlab::SQL::Intersect` directly + SOURCE + end +end diff --git a/spec/rubocop/cop/gitlab/rails_logger_spec.rb b/spec/rubocop/cop/gitlab/rails_logger_spec.rb index 0583079136b..70d208b31ec 100644 --- a/spec/rubocop/cop/gitlab/rails_logger_spec.rb +++ b/spec/rubocop/cop/gitlab/rails_logger_spec.rb @@ -10,22 +10,12 @@ RSpec.describe RuboCop::Cop::Gitlab::RailsLogger, type: :rubocop do subject(:cop) { described_class.new } - it 'flags the use of Rails.logger.error with a constant receiver' do - inspect_source("Rails.logger.error('some error')") + described_class::LOG_METHODS.each do |method| + it "flags the use of Rails.logger.#{method} with a constant receiver" do + inspect_source("Rails.logger.#{method}('some error')") - expect(cop.offenses.size).to eq(1) - end - - it 'flags the use of Rails.logger.info with a constant receiver' do - inspect_source("Rails.logger.info('some info')") - - expect(cop.offenses.size).to eq(1) - end - - it 'flags the use of Rails.logger.warn with a constant receiver' do - inspect_source("Rails.logger.warn('some warning')") - - expect(cop.offenses.size).to eq(1) + expect(cop.offenses.size).to eq(1) + end end it 'does not flag the use of Rails.logger with a constant that is not Rails' do @@ -39,4 +29,10 @@ RSpec.describe RuboCop::Cop::Gitlab::RailsLogger, type: :rubocop do expect(cop.offenses.size).to eq(0) end + + it 'does not flag the use of Rails.logger.level' do + inspect_source("Rails.logger.level") + + expect(cop.offenses.size).to eq(0) + 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 new file mode 100644 index 00000000000..b769109057e --- /dev/null +++ b/spec/rubocop/cop/migration/complex_indexes_require_name_spec.rb @@ -0,0 +1,164 @@ +# frozen_string_literal: true +# +require 'fast_spec_helper' +require 'rubocop' +require_relative '../../../../rubocop/cop/migration/complex_indexes_require_name' + +RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + context 'when creating complex indexes as part of create_table' do + context 'when indexes are configured with an options hash, but no name' do + it 'registers an offense' do + expect_offense(<<~RUBY) + class TestComplexIndexes < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def up + create_table :test_table do |t| + t.integer :column1, null: false + t.integer :column2, null: false + t.jsonb :column3 + + t.index :column1, unique: true + t.index :column2, where: 'column1 = 0' + ^^^^^ #{described_class::MSG} + t.index :column3, using: :gin + ^^^^^ #{described_class::MSG} + end + end + + def down + drop_table :test_table + end + end + RUBY + + expect(cop.offenses.map(&:cop_name)).to all(eq("Migration/#{described_class.name.demodulize}")) + end + end + + context 'when indexes are configured with an options hash and name' do + it 'registers no offense' do + expect_no_offenses(<<~RUBY) + class TestComplexIndexes < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def up + create_table :test_table do |t| + t.integer :column1, null: false + t.integer :column2, null: false + t.jsonb :column3 + + t.index :column1, unique: true + t.index :column2, where: 'column1 = 0', name: 'my_index_1' + t.index :column3, using: :gin, name: 'my_gin_index' + end + end + + def down + drop_table :test_table + end + end + RUBY + end + end + end + + context 'when indexes are added to an existing table' do + context 'when indexes are configured with an options hash, but no name' do + it 'registers an offense' do + expect_offense(<<~RUBY) + class TestComplexIndexes < ActiveRecord::Migration[6.0] + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_index :test_indexes, :column1 + + add_index :test_indexes, :column2, where: "column2 = 'value'", order: { column4: :desc } + ^^^^^^^^^ #{described_class::MSG} + end + + def down + add_index :test_indexes, :column4, 'unique' => true, where: 'column4 IS NOT NULL' + ^^^^^^^^^ #{described_class::MSG} + + add_concurrent_index :test_indexes, :column6, using: :gin, opclass: :gin_trgm_ops + ^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + end + end + RUBY + + expect(cop.offenses.map(&:cop_name)).to all(eq("Migration/#{described_class.name.demodulize}")) + end + end + + context 'when indexes are configured with an options hash and a name' do + it 'registers no offenses' do + expect_no_offenses(<<~RUBY) + class TestComplexIndexes < ActiveRecord::Migration[6.0] + DOWNTIME = false + + INDEX_NAME = 'my_test_name' + + disable_ddl_transaction! + + def up + add_index :test_indexes, :column1 + + add_index :test_indexes, :column2, where: "column2 = 'value'", order: { column4: :desc }, name: 'my_index_1' + + add_concurrent_index :test_indexes, :column3, where: 'column3 = 10', name: 'idx_equal_to_10' + end + + def down + add_index :test_indexes, :column4, 'unique' => true, where: 'column4 IS NOT NULL', name: 'my_index_2' + + add_concurrent_index :test_indexes, :column6, using: :gin, opclass: :gin_trgm_ops, name: INDEX_NAME + end + end + RUBY + end + end + end + end + + context 'outside migration' do + before do + allow(cop).to receive(:in_migration?).and_return(false) + end + + it 'registers no offenses' do + expect_no_offenses(<<~RUBY) + class TestComplexIndexes < ActiveRecord::Migration[6.0] + DOWNTIME = false + + disable_ddl_transaction! + + def up + create_table :test_table do |t| + t.integer :column1 + + t.index :column1, where: 'column2 IS NOT NULL' + end + + add_index :test_indexes, :column1, where: "some_column = 'value'" + end + + def down + add_concurrent_index :test_indexes, :column2, unique: true + end + end + RUBY + end + end +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 new file mode 100644 index 00000000000..fa4acc62226 --- /dev/null +++ b/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb @@ -0,0 +1,205 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require_relative '../../../../rubocop/cop/migration/create_table_with_foreign_keys' + +RSpec.describe RuboCop::Cop::Migration::CreateTableWithForeignKeys, type: :rubocop do + include CopHelper + + let(:cop) { described_class.new } + + context 'outside of a migration' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + def up + create_table(:foo) do |t| + t.references :bar, foreign_key: { on_delete: 'cascade' } + t.references :zoo, foreign_key: { on_delete: 'cascade' } + end + end + RUBY + end + end + + context 'in a migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + context 'without foreign key' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + def up + create_table(:foo) do |t| + t.references :bar + end + end + RUBY + end + end + + context 'with foreign key' do + context 'with just one foreign key' do + context 'when the foreign_key targets a high traffic table' do + context 'when the foreign_key has to_table option set' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + def up + create_table(:foo) do |t| + t.references :project, "foreign_key" => { on_delete: 'cascade', to_table: 'projects' } + end + end + RUBY + end + end + + context 'when the foreign_key does not have to_table option set' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + def up + create_table(:foo) do |t| + t.references :project, foreign_key: { on_delete: 'cascade' } + end + end + RUBY + end + end + end + + context 'when the foreign_key does not target a high traffic table' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + def up + create_table(:foo) do |t| + t.references :bar, foreign_key: { on_delete: 'cascade' } + end + end + RUBY + end + end + end + + context 'with more than one foreign keys' do + let(:offense) do + 'Creating a table with more than one foreign key at once violates our migration style guide. ' \ + 'For more details check the https://docs.gitlab.com/ce/development/migration_style_guide.html#examples' + end + + shared_examples 'target to high traffic table' do |dsl_method, table_name| + context 'when the target is defined as option' do + it 'registers an offense ' do + expect_offense(<<~RUBY) + def up + create_table(:foo) do |t| + ^^^^^^^^^^^^^^^^^^ #{offense} + t.#{dsl_method} :#{table_name.singularize} #{explicit_target_opts} + t.#{dsl_method} :zoo #{implicit_target_opts} + end + end + RUBY + end + end + + context 'when the target has implicit definition' do + it 'registers an offense ' do + expect_offense(<<~RUBY) + def up + create_table(:foo) do |t| + ^^^^^^^^^^^^^^^^^^ #{offense} + t.#{dsl_method} :#{table_name.singularize} #{implicit_target_opts} + t.#{dsl_method} :zoo #{implicit_target_opts} + end + end + RUBY + end + end + end + + shared_context 'when there is a target to a high traffic table' do |dsl_method| + %w[ + audit_events + ci_build_trace_sections + ci_builds + ci_builds_metadata + ci_job_artifacts + ci_pipeline_variables + ci_pipelines + ci_stages + deployments + events + gitlab_subscriptions + issues + merge_request_diff_commits + merge_request_diff_files + merge_request_diffs + merge_request_metrics + merge_requests + namespaces + note_diff_files + notes + project_authorizations + projects + project_ci_cd_settings + project_features + push_event_payloads + resource_label_events + routes + sent_notifications + system_note_metadata + taggings + todos + users + web_hook_logs + ].each do |table| + let(:table_name) { table } + + it_behaves_like 'target to high traffic table', dsl_method, table + end + end + + context 'when the foreign keys are defined as options' do + context 'when there is no target to a high traffic table' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + def up + create_table(:foo) do |t| + t.references :bar, foreign_key: { on_delete: 'cascade', to_table: :bars } + t.references :zoo, 'foreign_key' => { on_delete: 'cascade' } + t.references :john, 'foreign_key' => { on_delete: 'cascade' } + t.references :doe, 'foreign_key' => { on_delete: 'cascade', to_table: 'doe' } + end + end + RUBY + end + end + + include_context 'when there is a target to a high traffic table', :references do + let(:explicit_target_opts) { ", foreign_key: { to_table: :#{table_name} }" } + let(:implicit_target_opts) { ", foreign_key: true" } + end + end + + context 'when the foreign keys are defined by standlone migration helper' do + context 'when there is no target to a high traffic table' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + def up + create_table(:foo) do |t| + t.foreign_key :bar + t.foreign_key :zoo, to_table: 'zoos' + end + end + RUBY + end + end + + include_context 'when there is a target to a high traffic table', :foreign_key do + let(:explicit_target_opts) { ", to_table: :#{table_name}" } + let(:implicit_target_opts) { } + end + end + end + end + end +end 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 new file mode 100644 index 00000000000..76554d7446c --- /dev/null +++ b/spec/rubocop/cop/migration/refer_to_index_by_name_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true +# +require 'fast_spec_helper' +require 'rubocop' +require_relative '../../../../rubocop/cop/migration/refer_to_index_by_name' + +RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + context '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) + class TestReferToIndexByName < ActiveRecord::Migration[6.0] + DOWNTIME = false + + INDEX_NAME = 'my_test_name' + + disable_ddl_transaction! + + def up + if index_exists? :test_indexes, :column1, name: 'index_name_1' + remove_index :test_indexes, column: :column1, name: 'index_name_1' + end + + if index_exists? :test_indexes, :column2 + ^^^^^^^^^^^^^ #{described_class::MSG} + remove_index :test_indexes, :column2 + ^^^^^^^^^^^^ #{described_class::MSG} + end + + remove_index :test_indexes, column: column3 + ^^^^^^^^^^^^ #{described_class::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} + remove_concurrent_index :test_indexes, :column4, using: :gin, opclass: :gin_trgm_ops + ^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG} + end + + if index_exists? :test_indexes, :column3, unique: true, name: 'index_name_3', where: 'column3 = 10' + remove_concurrent_index :test_indexes, :column3, unique: true, name: 'index_name_3', where: 'column3 = 10' + end + end + end + RUBY + + expect(cop.offenses.map(&:cop_name)).to all(eq("Migration/#{described_class.name.demodulize}")) + end + end + end + + context 'outside migration' do + before do + allow(cop).to receive(:in_migration?).and_return(false) + end + + it 'registers no offenses' do + expect_no_offenses(<<~RUBY) + class TestReferToIndexByName < ActiveRecord::Migration[6.0] + DOWNTIME = false + + disable_ddl_transaction! + + def up + if index_exists? :test_indexes, :column1 + remove_index :test_indexes, :column1 + end + end + + def down + if index_exists? :test_indexes, :column1 + remove_concurrent_index :test_indexes, :column1 + end + end + end + RUBY + 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 013f2edc5e9..72b817fde12 100644 --- a/spec/rubocop/cop/migration/safer_boolean_column_spec.rb +++ b/spec/rubocop/cop/migration/safer_boolean_column_spec.rb @@ -14,7 +14,7 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn, type: :rubocop do allow(cop).to receive(:in_migration?).and_return(true) end - described_class::WHITELISTED_TABLES.each do |table| + described_class::SMALL_TABLES.each do |table| context "for the #{table} table" do sources_and_offense = [ ["add_column :#{table}, :column, :boolean, default: true", 'should disallow nulls'], @@ -59,14 +59,14 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn, type: :rubocop do end end - it 'registers no offense for tables not listed in WHITELISTED_TABLES' do + 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 end it 'registers no offense for non-boolean columns' do - table = described_class::WHITELISTED_TABLES.sample + table = described_class::SMALL_TABLES.sample inspect_source("add_column :#{table}, :column, :string") expect(cop.offenses).to be_empty @@ -75,7 +75,7 @@ RSpec.describe RuboCop::Cop::Migration::SaferBooleanColumn, type: :rubocop do context 'outside of migration' do it 'registers no offense' do - table = described_class::WHITELISTED_TABLES.sample + table = described_class::SMALL_TABLES.sample inspect_source("add_column :#{table}, :column, :boolean") expect(cop.offenses).to be_empty 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 5d96e8048bf..1d50d8c675e 100644 --- a/spec/rubocop/cop/migration/update_column_in_batches_spec.rb +++ b/spec/rubocop/cop/migration/update_column_in_batches_spec.rb @@ -1,15 +1,15 @@ # frozen_string_literal: true -require 'spec_helper' +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 +RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches, type: :rubocop do let(:cop) { described_class.new } - let(:tmp_rails_root) { Rails.root.join('tmp', 'rails_root') } + let(:tmp_rails_root) { rails_root_join('tmp', 'rails_root') } let(:migration_code) do <<-END def up @@ -27,6 +27,8 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do FileUtils.rm_rf(tmp_rails_root) end + let(:spec_filepath) { File.join(tmp_rails_root, 'spec', 'migrations', 'my_super_migration_spec.rb') } + context 'outside of a migration' do it 'does not register any offenses' do inspect_source(migration_code) @@ -35,8 +37,6 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do end end - let(:spec_filepath) { tmp_rails_root.join('spec', 'migrations', 'my_super_migration_spec.rb') } - shared_context 'with a migration file' do before do FileUtils.mkdir_p(File.dirname(migration_filepath)) @@ -83,31 +83,31 @@ RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do end context 'in a migration' do - let(:migration_filepath) { tmp_rails_root.join('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 a spec file' end context 'in a post migration' do - let(:migration_filepath) { tmp_rails_root.join('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 a spec file' end context 'EE migrations' do - let(:spec_filepath) { tmp_rails_root.join('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 - let(:migration_filepath) { tmp_rails_root.join('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 a spec file' end context 'in a post migration' do - let(:migration_filepath) { tmp_rails_root.join('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 a spec file' 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 c55d9bf22d6..888d1b6a2ba 100644 --- a/spec/rubocop/cop/put_group_routes_under_scope_spec.rb +++ b/spec/rubocop/cop/put_group_routes_under_scope_spec.rb @@ -46,4 +46,18 @@ RSpec.describe RuboCop::Cop::PutGroupRoutesUnderScope, type: :rubocop do end PATTERN end + + it 'does not register an offense for the root route' do + expect_no_offenses(<<~PATTERN) + get '/' + PATTERN + end + + it 'does not register an offense for the root route within scope' do + expect_no_offenses(<<~PATTERN) + scope(path: 'groups/*group_id/-', module: :groups) do + get '/' + end + PATTERN + end end diff --git a/spec/rubocop/cop/put_project_routes_under_scope_spec.rb b/spec/rubocop/cop/put_project_routes_under_scope_spec.rb index 05e1cd7b693..eebb7f3eb61 100644 --- a/spec/rubocop/cop/put_project_routes_under_scope_spec.rb +++ b/spec/rubocop/cop/put_project_routes_under_scope_spec.rb @@ -46,4 +46,18 @@ RSpec.describe RuboCop::Cop::PutProjectRoutesUnderScope, type: :rubocop do end PATTERN end + + it 'does not register an offense for the root route' do + expect_no_offenses(<<~PATTERN) + get '/' + PATTERN + end + + it 'does not register an offense for the root route within scope' do + expect_no_offenses(<<~PATTERN) + scope '-' do + get '/' + end + PATTERN + end end diff --git a/spec/rubocop/cop/rspec/timecop_freeze_spec.rb b/spec/rubocop/cop/rspec/timecop_freeze_spec.rb new file mode 100644 index 00000000000..3809431a2fc --- /dev/null +++ b/spec/rubocop/cop/rspec/timecop_freeze_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/rspec/timecop_freeze' + +RSpec.describe RuboCop::Cop::RSpec::TimecopFreeze, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + context 'when calling Timecop.freeze' do + let(:source) do + <<~SRC + Timecop.freeze(Time.current) { example.run } + SRC + end + + let(:corrected_source) do + <<~SRC + freeze_time(Time.current) { example.run } + SRC + end + + it 'registers an offence' do + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + end + + it 'can autocorrect the source' do + expect(autocorrect_source(source)).to eq(corrected_source) + end + end + + context 'when calling a different method on Timecop' do + let(:source) do + <<~SRC + Timecop.travel(Time.current) + SRC + end + + it 'does not register an offence' do + inspect_source(source) + + expect(cop.offenses).to be_empty + end + end +end diff --git a/spec/rubocop/cop/static_translation_definition_spec.rb b/spec/rubocop/cop/static_translation_definition_spec.rb index b6c9f6a25df..f3185def3d7 100644 --- a/spec/rubocop/cop/static_translation_definition_spec.rb +++ b/spec/rubocop/cop/static_translation_definition_spec.rb @@ -40,6 +40,17 @@ RSpec.describe RuboCop::Cop::StaticTranslationDefinition, type: :rubocop do ['C = n_("c")', 'n_("c")', 1], [ <<~CODE, + class MyClass + def self.translations + @cache ||= { hello: _("hello") } + end + end + CODE + '_("hello")', + 3 + ], + [ + <<~CODE, module MyModule A = { b: { @@ -79,6 +90,20 @@ RSpec.describe RuboCop::Cop::StaticTranslationDefinition, type: :rubocop do 'CONSTANT_2 = s__("a")', 'CONSTANT_3 = n__("a")', <<~CODE, + class MyClass + def self.method + @cache ||= { hello: -> { _("hello") } } + end + end + CODE + <<~CODE, + class MyClass + def method + @cache ||= { hello: _("hello") } + end + end + CODE + <<~CODE, def method s_('a') end diff --git a/spec/rubocop/cop/usage_data/distinct_count_by_large_foreign_key_spec.rb b/spec/rubocop/cop/usage_data/distinct_count_by_large_foreign_key_spec.rb index db931c50bdf..8b6a2eac349 100644 --- a/spec/rubocop/cop/usage_data/distinct_count_by_large_foreign_key_spec.rb +++ b/spec/rubocop/cop/usage_data/distinct_count_by_large_foreign_key_spec.rb @@ -10,7 +10,7 @@ require_relative '../../../../rubocop/cop/usage_data/distinct_count_by_large_for RSpec.describe RuboCop::Cop::UsageData::DistinctCountByLargeForeignKey, type: :rubocop do include CopHelper - let(:allowed_foreign_keys) { %i[author_id user_id] } + let(:allowed_foreign_keys) { [:author_id, :user_id, :'merge_requests.target_project_id'] } let(:config) do RuboCop::Config.new('UsageData/DistinctCountByLargeForeignKey' => { @@ -21,18 +21,36 @@ RSpec.describe RuboCop::Cop::UsageData::DistinctCountByLargeForeignKey, type: :r subject(:cop) { described_class.new(config) } context 'when counting by disallowed key' do - it 'register an offence' do + it 'registers an offence' do inspect_source('distinct_count(Issue, :creator_id)') expect(cop.offenses.size).to eq(1) end + + it 'does not register an offence when batch is false' do + inspect_source('distinct_count(Issue, :creator_id, batch: false)') + + expect(cop.offenses).to be_empty + end + + it 'register an offence when batch is true' do + inspect_source('distinct_count(Issue, :creator_id, batch: true)') + + expect(cop.offenses.size).to eq(1) + end end context 'when calling by allowed key' do - it 'does not register an offence' do + it 'does not register an offence with symbol' do inspect_source('distinct_count(Issue, :author_id)') expect(cop.offenses).to be_empty end + + it 'does not register an offence with string' do + inspect_source("distinct_count(Issue, 'merge_requests.target_project_id')") + + expect(cop.offenses).to be_empty + end end end |