diff options
Diffstat (limited to 'spec/tooling')
31 files changed, 518 insertions, 90 deletions
diff --git a/spec/tooling/danger/analytics_instrumentation_spec.rb b/spec/tooling/danger/analytics_instrumentation_spec.rb index 5d12647e02f..79c75b2e89c 100644 --- a/spec/tooling/danger/analytics_instrumentation_spec.rb +++ b/spec/tooling/danger/analytics_instrumentation_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'gitlab-dangerfiles' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/analytics_instrumentation' @@ -231,4 +231,64 @@ RSpec.describe Tooling::Danger::AnalyticsInstrumentation, feature_category: :ser end end end + + describe '#check_deprecated_data_sources!' do + let(:fake_project_helper) { instance_double(Tooling::Danger::ProjectHelper) } + + subject(:check_data_source) { analytics_instrumentation.check_deprecated_data_sources! } + + before do + allow(fake_helper).to receive(:added_files).and_return([added_file]) + allow(fake_helper).to receive(:changed_lines).with(added_file).and_return(changed_lines) + allow(analytics_instrumentation).to receive(:project_helper).and_return(fake_project_helper) + allow(analytics_instrumentation.project_helper).to receive(:file_lines).and_return(changed_lines.map { |line| line.delete_prefix('+') }) + end + + context 'when no metric definitions were modified' do + let(:added_file) { 'app/models/user.rb' } + let(:changed_lines) { ['+ data_source: redis,'] } + + it 'does not trigger warning' do + expect(analytics_instrumentation).not_to receive(:markdown) + + check_data_source + end + end + + context 'when metrics fields were modified' do + let(:added_file) { 'config/metrics/count7_d/example_metric.yml' } + + [:redis, :redis_hll].each do |source| + context "when source is #{source}" do + let(:changed_lines) { ["+ data_source: #{source}"] } + let(:template) do + <<~SUGGEST_COMMENT + ```suggestion + data_source: internal_events + ``` + + %<message>s + SUGGEST_COMMENT + end + + it 'issues a warning' do + expected_comment = format(template, message: Tooling::Danger::AnalyticsInstrumentation::CHANGE_DEPRECATED_DATA_SOURCE_MESSAGE) + expect(analytics_instrumentation).to receive(:markdown).with(expected_comment.strip, file: added_file, line: 1) + + check_data_source + end + end + end + + context 'when neither redis nor redis_hll used as a data_source' do + let(:changed_lines) { ['+ data_source: database,'] } + + it 'does not issue a warning' do + expect(analytics_instrumentation).not_to receive(:markdown) + + check_data_source + end + end + end + end end diff --git a/spec/tooling/danger/bulk_database_actions_spec.rb b/spec/tooling/danger/bulk_database_actions_spec.rb index 620b4ac2b18..eba3eacb212 100644 --- a/spec/tooling/danger/bulk_database_actions_spec.rb +++ b/spec/tooling/danger/bulk_database_actions_spec.rb @@ -1,10 +1,8 @@ # frozen_string_literal: true -require 'gitlab-dangerfiles' -require 'danger' -require 'danger/plugins/internal/helper' -require 'gitlab/dangerfiles/spec_helper' +require 'fast_spec_helper' require 'rspec-parameterized' +require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/bulk_database_actions' require_relative '../../../tooling/danger/project_helper' diff --git a/spec/tooling/danger/change_column_default_spec.rb b/spec/tooling/danger/change_column_default_spec.rb new file mode 100644 index 00000000000..8cfcbfa1dc0 --- /dev/null +++ b/spec/tooling/danger/change_column_default_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'danger' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../tooling/danger/change_column_default' +require_relative '../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::ChangeColumnDefault, feature_category: :tooling do + subject(:change_column_default) { fake_danger.new(helper: fake_helper) } + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:fake_project_helper) { instance_double(Tooling::Danger::ProjectHelper) } + let(:comment) { described_class::COMMENT.chomp } + let(:file_diff) do + File.read(File.expand_path("../fixtures/change_column_default_migration.txt", __dir__)).split("\n") + end + + include_context "with dangerfile" + + describe '#add_comment_for_change_column_default' do + let(:file_lines) { file_diff.map { |line| line.delete_prefix('+').delete_prefix('-') } } + let(:matching_lines) { [7, 9, 11] } + + before do + allow(change_column_default).to receive(:project_helper).and_return(fake_project_helper) + allow(change_column_default.project_helper).to receive(:file_lines).and_return(file_lines) + allow(change_column_default.helper).to receive(:all_changed_files).and_return([filename]) + allow(change_column_default.helper).to receive(:changed_lines).with(filename).and_return(file_diff) + end + + context 'when column default is changed in a regular migration' do + let(:filename) { 'db/migrate/change_column_default_migration.rb' } + + it 'adds comment at the correct line' do + matching_lines.each do |line_number| + expect(change_column_default).to receive(:markdown).with("\n#{comment}", file: filename, line: line_number) + end + + change_column_default.add_comment_for_change_column_default + end + end + + context 'when column default is changed in a post-deployment migration' do + let(:filename) { 'db/post_migrate/change_column_default_migration.rb' } + + it 'adds comment at the correct line' do + matching_lines.each do |line_number| + expect(change_column_default).to receive(:markdown).with("\n#{comment}", file: filename, line: line_number) + end + + change_column_default.add_comment_for_change_column_default + end + end + + context 'when a regular migration does not change column default' do + let(:filename) { 'db/migrate/my_migration.rb' } + let(:file_diff) do + [ + "+ undo_cleanup_concurrent_column_rename(:my_table, :old_column, :new_column)", + "- cleanup_concurrent_column_rename(:my_table, :new_column, :old_column)" + ] + end + + let(:file_lines) do + [ + ' def up', + ' undo_cleanup_concurrent_column_rename(:my_table, :old_column, :new_column)', + ' end' + ] + end + + it 'does not add comment' do + expect(change_column_default).not_to receive(:markdown) + + change_column_default.add_comment_for_change_column_default + end + end + + context 'when a post-deployment migration does not change column default' do + let(:filename) { 'db/post_migrate/my_migration.rb' } + let(:file_diff) do + [ + "+ restore_conversion_of_integer_to_bigint(TABLE, COLUMNS)", + "- cleanup_conversion_of_integer_to_bigint(TABLE, COLUMNS)" + ] + end + + let(:file_lines) do + [ + ' def up', + ' cleanup_conversion_of_integer_to_bigint(TABLE, COLUMNS)', + ' end' + ] + end + + it 'does not add comment' do + expect(change_column_default).not_to receive(:markdown) + + change_column_default.add_comment_for_change_column_default + end + end + end +end diff --git a/spec/tooling/danger/clickhouse_spec.rb b/spec/tooling/danger/clickhouse_spec.rb index ad2f0b4a827..135f8810e61 100644 --- a/spec/tooling/danger/clickhouse_spec.rb +++ b/spec/tooling/danger/clickhouse_spec.rb @@ -1,9 +1,7 @@ # frozen_string_literal: true require 'rspec-parameterized' -require 'gitlab-dangerfiles' -require 'danger' -require 'danger/plugins/internal/helper' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/clickhouse' diff --git a/spec/tooling/danger/config_files_spec.rb b/spec/tooling/danger/config_files_spec.rb index 42fc08ad901..f9b7dc64e6d 100644 --- a/spec/tooling/danger/config_files_spec.rb +++ b/spec/tooling/danger/config_files_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true -require 'gitlab-dangerfiles' -require 'danger' -require 'danger/plugins/internal/helper' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/config_files' diff --git a/spec/tooling/danger/customer_success_spec.rb b/spec/tooling/danger/customer_success_spec.rb index 798905212f1..40ab7c79418 100644 --- a/spec/tooling/danger/customer_success_spec.rb +++ b/spec/tooling/danger/customer_success_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'rspec-parameterized' -require 'gitlab-dangerfiles' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/customer_success' @@ -17,53 +17,53 @@ RSpec.describe Tooling::Danger::CustomerSuccess do where do { 'with data category changes to Ops and no Customer Success::Impact Check label' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb], changed_lines: ['-data_category: cat1', '+data_category: operational'], customer_labeled: false, impacted: true, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] }, 'with data category changes and Customer Success::Impact Check label' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml], changed_lines: ['-data_category: cat1', '+data_category: operational'], customer_labeled: true, impacted: false, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] }, 'with metric file changes and no data category changes' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml], changed_lines: ['-product_stage: growth'], customer_labeled: false, impacted: false, impacted_files: [] }, 'with data category changes from Ops' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb], changed_lines: ['-data_category: operational', '+data_category: cat2'], customer_labeled: false, impacted: true, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] }, 'with data category removed' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb], changed_lines: ['-data_category: operational'], customer_labeled: false, impacted: true, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] }, 'with data category added' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb], changed_lines: ['+data_category: operational'], customer_labeled: false, impacted: true, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] }, 'with data category in uppercase' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb], changed_lines: ['+data_category: Operational'], customer_labeled: false, impacted: true, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] } } end diff --git a/spec/tooling/danger/database_dictionary_spec.rb b/spec/tooling/danger/database_dictionary_spec.rb index 1a771a6cec0..943179cda19 100644 --- a/spec/tooling/danger/database_dictionary_spec.rb +++ b/spec/tooling/danger/database_dictionary_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'gitlab-dangerfiles' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/database_dictionary' diff --git a/spec/tooling/danger/database_spec.rb b/spec/tooling/danger/database_spec.rb index a342014cf6b..bfc92e0a744 100644 --- a/spec/tooling/danger/database_spec.rb +++ b/spec/tooling/danger/database_spec.rb @@ -1,9 +1,7 @@ # frozen_string_literal: true +require 'fast_spec_helper' require 'rspec-parameterized' -require 'gitlab-dangerfiles' -require 'danger' -require 'danger/plugins/internal/helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/database' diff --git a/spec/tooling/danger/datateam_spec.rb b/spec/tooling/danger/datateam_spec.rb index de8a93baa27..9d8aaf08520 100644 --- a/spec/tooling/danger/datateam_spec.rb +++ b/spec/tooling/danger/datateam_spec.rb @@ -1,9 +1,8 @@ # frozen_string_literal: true +require 'fast_spec_helper' require 'rspec-parameterized' -require 'gitlab-dangerfiles' require 'gitlab/dangerfiles/spec_helper' -require 'pry' require_relative '../../../tooling/danger/datateam' RSpec.describe Tooling::Danger::Datateam do @@ -17,89 +16,96 @@ RSpec.describe Tooling::Danger::Datateam do where do { - 'with structure.sql changes and no Data Warehouse::Impact Check label' => { - modified_files: %w(db/structure.sql app/models/user.rb), - changed_lines: ['+group_id bigint NOT NULL'], + 'with structure.sql subtraction changes and no Data Warehouse::Impact Check label' => { + modified_files: %w[db/structure.sql app/models/user.rb], + changed_lines: ['-group_id bigint NOT NULL'], mr_labels: [], impacted: true, - impacted_files: %w(db/structure.sql) + impacted_files: %w[db/structure.sql] }, - 'with structure.sql changes and Data Warehouse::Impact Check label' => { - modified_files: %w(db/structure.sql), - changed_lines: ['+group_id bigint NOT NULL)'], + 'with structure.sql subtraction changes and Data Warehouse::Impact Check label' => { + modified_files: %w[db/structure.sql], + changed_lines: ['-group_id bigint NOT NULL)'], mr_labels: ['Data Warehouse::Impact Check'], impacted: false, - impacted_files: %w(db/structure.sql) + impacted_files: %w[db/structure.sql] + }, + 'with structure.sql addition changes and no Data Warehouse::Impact Check label' => { + modified_files: %w[db/structure.sql app/models/user.rb], + changed_lines: ['+group_id bigint NOT NULL'], + mr_labels: [], + impacted: false, + impacted_files: %w[db/structure.sql] }, 'with user model changes' => { - modified_files: %w(app/models/users.rb), + modified_files: %w[app/models/users.rb], changed_lines: ['+has_one :namespace'], mr_labels: [], impacted: false, impacted_files: [] }, 'with perfomance indicator changes and no Data Warehouse::Impact Check label' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb], changed_lines: ['+-gmau'], mr_labels: [], impacted: true, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] }, 'with perfomance indicator changes and Data Warehouse::Impact Check label' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml], changed_lines: ['+-gmau'], mr_labels: ['Data Warehouse::Impact Check'], impacted: false, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] }, 'with metric file changes and no performance indicator changes' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml], changed_lines: ['-product_stage: growth'], mr_labels: [], impacted: false, impacted_files: [] }, 'with metric file changes and no performance indicator changes and other label' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml], changed_lines: ['-product_stage: growth'], mr_labels: ['type::maintenance'], impacted: false, impacted_files: [] }, 'with performance indicator changes and other label' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb], changed_lines: ['+-gmau'], mr_labels: ['type::maintenance'], impacted: true, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] }, 'with performance indicator changes, Data Warehouse::Impact Check and other label' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb], changed_lines: ['+-gmau'], mr_labels: ['type::maintenance', 'Data Warehouse::Impact Check'], impacted: false, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] }, 'with performance indicator changes and other labels' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb], changed_lines: ['+-gmau'], mr_labels: ['type::maintenance', 'Data Warehouse::Impacted'], impacted: false, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] }, 'with metric status removed' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb], changed_lines: ['+status: removed'], mr_labels: ['type::maintenance'], impacted: true, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] }, 'with metric status active' => { - modified_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb), + modified_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml app/models/user.rb], changed_lines: ['+status: active'], mr_labels: ['type::maintenance'], impacted: false, - impacted_files: %w(config/metrics/20210216182127_user_secret_detection_jobs.yml) + impacted_files: %w[config/metrics/20210216182127_user_secret_detection_jobs.yml] } } end diff --git a/spec/tooling/danger/experiments_spec.rb b/spec/tooling/danger/experiments_spec.rb index 85f8060a3ec..7c4921670a3 100644 --- a/spec/tooling/danger/experiments_spec.rb +++ b/spec/tooling/danger/experiments_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'gitlab-dangerfiles' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/experiments' diff --git a/spec/tooling/danger/feature_flag_spec.rb b/spec/tooling/danger/feature_flag_spec.rb index 4575d8ca981..9298028feb3 100644 --- a/spec/tooling/danger/feature_flag_spec.rb +++ b/spec/tooling/danger/feature_flag_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'gitlab-dangerfiles' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/feature_flag' @@ -134,7 +134,7 @@ RSpec.describe Tooling::Danger::FeatureFlag do context 'when group is not nil' do it 'is true only if MR has the same group label' do expect(found.group_match_mr_label?(group)).to eq true - expect(found.group_match_mr_label?('group::authentication and authorization')).to eq false + expect(found.group_match_mr_label?('group::authentication')).to eq false end end end diff --git a/spec/tooling/danger/gitlab_schema_validation_suggestion_spec.rb b/spec/tooling/danger/gitlab_schema_validation_suggestion_spec.rb new file mode 100644 index 00000000000..c83e0319423 --- /dev/null +++ b/spec/tooling/danger/gitlab_schema_validation_suggestion_spec.rb @@ -0,0 +1,108 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../tooling/danger/gitlab_schema_validation_suggestion' +require_relative '../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::GitlabSchemaValidationSuggestion, feature_category: :cell do + include_context "with dangerfile" + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:fake_project_helper) { instance_double(Tooling::Danger::ProjectHelper) } + let(:filename) { 'db/docs/application_settings.yml' } + let(:file_lines) do + file_diff.map { |line| line.delete_prefix('+') } + end + + let(:file_diff) do + [ + "+---", + "+table_name: application_settings", + "+classes:", + "+- ApplicationSetting", + "+feature_categories:", + "+- continuous_integration", + "+description: GitLab application settings", + "+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/commit/8589b4e137f50293952923bb07e2814257d7784d", + "+milestone: '7.7'", + "+gitlab_schema: #{schema}" + ] + end + + subject(:gitlab_schema_validation) { fake_danger.new(helper: fake_helper) } + + before do + allow(gitlab_schema_validation).to receive(:project_helper).and_return(fake_project_helper) + allow(gitlab_schema_validation.project_helper).to receive(:file_lines).and_return(file_lines) + allow(gitlab_schema_validation.helper).to receive(:changed_lines).with(filename).and_return(file_diff) + allow(gitlab_schema_validation.helper).to receive(:all_changed_files).and_return([filename]) + end + + shared_examples_for 'does not add a comment' do + it do + expect(gitlab_schema_validation).not_to receive(:markdown) + + gitlab_schema_validation.add_suggestions_on_using_clusterwide_schema + end + end + + context 'for discouraging the use of gitlab_main_clusterwide schema' do + let(:schema) { 'gitlab_main_clusterwide' } + + context 'when the file path matches' do + it 'adds the comment' do + expected_comment = "\n#{described_class::SUGGESTION.chomp}" + + expect(gitlab_schema_validation).to receive(:markdown).with(expected_comment, file: filename, line: 10) + + gitlab_schema_validation.add_suggestions_on_using_clusterwide_schema + end + end + + context 'when the file path does not match' do + let(:filename) { 'some_path/application_settings.yml' } + + it_behaves_like 'does not add a comment' + end + + context 'for EE' do + let(:filename) { 'ee/db/docs/application_settings.yml' } + + it_behaves_like 'does not add a comment' + end + + context 'for a deleted table' do + let(:filename) { 'db/docs/deleted_tables/application_settings.yml' } + + it_behaves_like 'does not add a comment' + end + end + + context 'on removing the gitlab_main_clusterwide schema' do + let(:file_diff) do + [ + "+---", + "+table_name: application_settings", + "+classes:", + "+- ApplicationSetting", + "+feature_categories:", + "+- continuous_integration", + "+description: GitLab application settings", + "+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/commit/8589b4e137f50293952923bb07e2814257d7784d", + "+milestone: '7.7'", + "-gitlab_schema: gitlab_main_clusterwide", + "+gitlab_schema: gitlab_main_cell" + ] + end + + it_behaves_like 'does not add a comment' + end + + context 'when a different schema is added' do + let(:schema) { 'gitlab_main' } + + it_behaves_like 'does not add a comment' + end +end diff --git a/spec/tooling/danger/ignored_model_columns_spec.rb b/spec/tooling/danger/ignored_model_columns_spec.rb index 737b6cce077..3d19f80a4ed 100644 --- a/spec/tooling/danger/ignored_model_columns_spec.rb +++ b/spec/tooling/danger/ignored_model_columns_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'danger' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/ignored_model_columns' diff --git a/spec/tooling/danger/model_validations_spec.rb b/spec/tooling/danger/model_validations_spec.rb index 18ff4b83b6e..2dc2bc3e186 100644 --- a/spec/tooling/danger/model_validations_spec.rb +++ b/spec/tooling/danger/model_validations_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true -require 'gitlab-dangerfiles' -require 'danger' -require 'danger/plugins/internal/helper' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/model_validations' diff --git a/spec/tooling/danger/multiversion_spec.rb b/spec/tooling/danger/multiversion_spec.rb index 90edad61d47..649a13013c4 100644 --- a/spec/tooling/danger/multiversion_spec.rb +++ b/spec/tooling/danger/multiversion_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'rspec-parameterized' -require 'gitlab-dangerfiles' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/multiversion' diff --git a/spec/tooling/danger/outdated_todo_spec.rb b/spec/tooling/danger/outdated_todo_spec.rb new file mode 100644 index 00000000000..3a3909c69ac --- /dev/null +++ b/spec/tooling/danger/outdated_todo_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../tooling/danger/outdated_todo' + +RSpec.describe Tooling::Danger::OutdatedTodo, feature_category: :tooling do + let(:fake_danger) { double } + let(:filenames) { ['app/controllers/application_controller.rb'] } + + let(:todos) do + [ + File.join('spec', 'fixtures', 'tooling', 'danger', 'rubocop_todo', '**', '*.yml') + ] + end + + subject(:plugin) { described_class.new(filenames, context: fake_danger, todos: todos) } + + context 'when the filenames are mentioned in single todo' do + let(:filenames) { ['app/controllers/acme_challenges_controller.rb'] } + + it 'warns about mentions' do + expect(fake_danger) + .to receive(:warn) + .with <<~MESSAGE + `app/controllers/acme_challenges_controller.rb` was removed but is mentioned in: + - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:5` + MESSAGE + + plugin.check + end + end + + context 'when the filenames are mentioned in multiple todos' do + let(:filenames) do + [ + 'app/controllers/application_controller.rb', + 'app/controllers/acme_challenges_controller.rb' + ] + end + + it 'warns about mentions' do + expect(fake_danger) + .to receive(:warn) + .with(<<~FIRSTMESSAGE) + `app/controllers/application_controller.rb` was removed but is mentioned in: + - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:4` + - `spec/fixtures/tooling/danger/rubocop_todo/cop2.yml:4` + FIRSTMESSAGE + + expect(fake_danger) + .to receive(:warn) + .with(<<~SECONDMESSAGE) + `app/controllers/acme_challenges_controller.rb` was removed but is mentioned in: + - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:5` + SECONDMESSAGE + + plugin.check + end + end + + context 'when the filenames are not mentioned in todos' do + let(:filenames) { ['any/inexisting/file.rb'] } + + it 'does not warn' do + expect(fake_danger).not_to receive(:warn) + + plugin.check + end + end + + context 'when there is no todos' do + let(:filenames) { ['app/controllers/acme_challenges_controller.rb'] } + let(:todos) { [] } + + it 'does not warn' do + expect(fake_danger).not_to receive(:warn) + + plugin.check + end + end +end diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb index 28b8b2278d0..2da90ddbd67 100644 --- a/spec/tooling/danger/project_helper_spec.rb +++ b/spec/tooling/danger/project_helper_spec.rb @@ -1,11 +1,10 @@ # frozen_string_literal: true +require 'fast_spec_helper' require 'rspec-parameterized' -require 'gitlab-dangerfiles' require 'danger' require 'danger/plugins/internal/helper' require 'gitlab/dangerfiles/spec_helper' -require 'gitlab/rspec/all' require_relative '../../../danger/plugins/project_helper' @@ -244,6 +243,7 @@ RSpec.describe Tooling::Danger::ProjectHelper do [:analytics_instrumentation] | '+data-track-action' | ['components/welcome.vue'] [:analytics_instrumentation] | '+ data: { track_label:' | ['admin/groups/_form.html.haml'] [:analytics_instrumentation] | '+ Gitlab::Tracking.event' | ['dashboard/todos_controller.rb', 'admin/groups/_form.html.haml'] + [:analytics_instrumentation] | '+ Gitlab::Tracking.event("c", "a")' | ['dashboard/todos_controller.rb', 'admin/groups/_form.html.haml'] [:database, :backend, :analytics_instrumentation] | '+ count(User.active)' | ['usage_data.rb', 'lib/gitlab/usage_data.rb', 'ee/lib/ee/gitlab/usage_data.rb'] [:database, :backend, :analytics_instrumentation] | '+ estimate_batch_distinct_count(User.active)' | ['usage_data.rb'] [:backend, :analytics_instrumentation] | '+ alt_usage_data(User.active)' | ['lib/gitlab/usage_data.rb'] diff --git a/spec/tooling/danger/required_stops_spec.rb b/spec/tooling/danger/required_stops_spec.rb index 7a90f19ac09..1b811166d13 100644 --- a/spec/tooling/danger/required_stops_spec.rb +++ b/spec/tooling/danger/required_stops_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true -require 'gitlab-dangerfiles' -require 'danger' -require 'danger/plugins/internal/helper' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/required_stops' diff --git a/spec/tooling/danger/rubocop_inline_disable_suggestion_spec.rb b/spec/tooling/danger/rubocop_inline_disable_suggestion_spec.rb index 94dd5192d74..6b9ff667564 100644 --- a/spec/tooling/danger/rubocop_inline_disable_suggestion_spec.rb +++ b/spec/tooling/danger/rubocop_inline_disable_suggestion_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/rubocop_inline_disable_suggestion' @@ -14,10 +15,15 @@ RSpec.describe Tooling::Danger::RubocopInlineDisableSuggestion, feature_category let(:template) do <<~SUGGESTION_MARKDOWN.chomp + ```suggestion + %<suggested_line>s + ``` Consider removing this inline disabling and adhering to the rubocop rule. - If that isn't possible, please provide context as a reply for reviewers. - See [rubocop best practices](https://docs.gitlab.com/ee/development/rubocop_development_guide.html). + + If that isn't possible, please provide the reason as a code comment in the + same line where the rule is disabled separated by ` -- `. + See [rubocop best practices](https://docs.gitlab.com/ee/development/rubocop_development_guide.html#disabling-rules-inline). ---- @@ -73,6 +79,23 @@ RSpec.describe Tooling::Danger::RubocopInlineDisableSuggestion, feature_category show_out_of_pipeline_minutes_notification?(project, namespace) end + + def show_my_new_dot?(project, namespace) + return false unless ::Gitlab.com? # rubocop: todo Gitlab/AvoidGitlabInstanceChecks -- Reason for disabling + thatsfine = "".dup # rubocop:disable Lint/UselessAssignment,Performance/UnfreezeString -- That's OK + me = "".dup # rubocop:disable Lint/UselessAssignment,Performance/UnfreezeString + test = "".dup # rubocop:disable Lint/UselessAssignment, Performance/UnfreezeString + return false if notification_dot_acknowledged? + + show_out_of_pipeline_minutes_notification?(project, namespace) + end + + def show_my_bad_dot?(project, namespace) + return false unless ::Gitlab.com? # rubocop: todo Gitlab/AvoidGitlabInstanceChecks -- + return false if notification_dot_acknowledged? + + show_out_of_pipeline_minutes_notification?(project, namespace) + end RUBY end @@ -86,6 +109,10 @@ RSpec.describe Tooling::Danger::RubocopInlineDisableSuggestion, feature_category + return false unless ::Gitlab.com? # rubocop: disable Gitlab/AvoidGitlabInstanceChecks + return false unless ::Gitlab.com? # rubocop:todo Gitlab/AvoidGitlabInstanceChecks + return false unless ::Gitlab.com? # rubocop: todo Gitlab/AvoidGitlabInstanceChecks + + return false unless ::Gitlab.com? # rubocop: todo Gitlab/AvoidGitlabInstanceChecks -- Reason for disabling + + me = "".dup # rubocop:disable Lint/UselessAssignment,Performance/UnfreezeString + + test = "".dup # rubocop:disable Lint/UselessAssignment, Performance/UnfreezeString + + return false unless ::Gitlab.com? # rubocop: todo Gitlab/AvoidGitlabInstanceChecks -- DIFF end @@ -102,8 +129,12 @@ RSpec.describe Tooling::Danger::RubocopInlineDisableSuggestion, feature_category end it 'adds comments at the correct lines', :aggregate_failures do - [3, 7, 13, 20, 27, 34, 41].each do |line_number| - expect(rubocop).to receive(:markdown).with(template, file: filename, line: line_number) + [3, 7, 13, 20, 27, 34, 41, 50, 51, 58].each do |line_number| + existing_line = file_lines[line_number - 1].sub(/ --\s*$/, '') + suggested_line = "#{existing_line} -- TODO: Reason why the rule must be disabled" + comment = format(template, suggested_line: suggested_line) + + expect(rubocop).to receive(:markdown).with(comment, file: filename, line: line_number) end rubocop.add_suggestions_for(filename) diff --git a/spec/tooling/danger/saas_feature_spec.rb b/spec/tooling/danger/saas_feature_spec.rb index 7ce9116ea5f..019dbf6944f 100644 --- a/spec/tooling/danger/saas_feature_spec.rb +++ b/spec/tooling/danger/saas_feature_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'gitlab-dangerfiles' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/saas_feature' diff --git a/spec/tooling/danger/sidekiq_args_spec.rb b/spec/tooling/danger/sidekiq_args_spec.rb index 29bf32a9a02..c44486a83b5 100644 --- a/spec/tooling/danger/sidekiq_args_spec.rb +++ b/spec/tooling/danger/sidekiq_args_spec.rb @@ -1,9 +1,7 @@ # frozen_string_literal: true require 'rspec-parameterized' -require 'gitlab-dangerfiles' -require 'danger' -require 'danger/plugins/internal/helper' +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/sidekiq_args' diff --git a/spec/tooling/danger/sidekiq_queues_spec.rb b/spec/tooling/danger/sidekiq_queues_spec.rb index 9bffc7ee93d..143ea9732cd 100644 --- a/spec/tooling/danger/sidekiq_queues_spec.rb +++ b/spec/tooling/danger/sidekiq_queues_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true +require 'fast_spec_helper' require 'rspec-parameterized' -require 'gitlab-dangerfiles' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/sidekiq_queues' @@ -17,12 +17,12 @@ RSpec.describe Tooling::Danger::SidekiqQueues do using RSpec::Parameterized::TableSyntax where(:modified_files, :changed_queue_files) do - %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml foo) | %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml) - %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml) | %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml) - %w(app/workers/all_queues.yml foo) | %w(app/workers/all_queues.yml) - %w(ee/app/workers/all_queues.yml foo) | %w(ee/app/workers/all_queues.yml) - %w(foo) | %w() - %w() | %w() + %w[app/workers/all_queues.yml ee/app/workers/all_queues.yml foo] | %w[app/workers/all_queues.yml ee/app/workers/all_queues.yml] + %w[app/workers/all_queues.yml ee/app/workers/all_queues.yml] | %w[app/workers/all_queues.yml ee/app/workers/all_queues.yml] + %w[app/workers/all_queues.yml foo] | %w[app/workers/all_queues.yml] + %w[ee/app/workers/all_queues.yml foo] | %w[ee/app/workers/all_queues.yml] + %w[foo] | %w[] + %w[] | %w[] end with_them do diff --git a/spec/tooling/danger/specs/feature_category_suggestion_spec.rb b/spec/tooling/danger/specs/feature_category_suggestion_spec.rb index 87eb20e5e50..ea8a00afac1 100644 --- a/spec/tooling/danger/specs/feature_category_suggestion_spec.rb +++ b/spec/tooling/danger/specs/feature_category_suggestion_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../../tooling/danger/specs' diff --git a/spec/tooling/danger/specs/match_with_array_suggestion_spec.rb b/spec/tooling/danger/specs/match_with_array_suggestion_spec.rb index b065772a09b..92c4a134a34 100644 --- a/spec/tooling/danger/specs/match_with_array_suggestion_spec.rb +++ b/spec/tooling/danger/specs/match_with_array_suggestion_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../../tooling/danger/specs' diff --git a/spec/tooling/danger/specs/project_factory_suggestion_spec.rb b/spec/tooling/danger/specs/project_factory_suggestion_spec.rb index b765d5073af..078f415cc44 100644 --- a/spec/tooling/danger/specs/project_factory_suggestion_spec.rb +++ b/spec/tooling/danger/specs/project_factory_suggestion_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../../tooling/danger/specs' diff --git a/spec/tooling/danger/specs_spec.rb b/spec/tooling/danger/specs_spec.rb index b4953858ef7..f601e11a7a5 100644 --- a/spec/tooling/danger/specs_spec.rb +++ b/spec/tooling/danger/specs_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'fast_spec_helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/specs' diff --git a/spec/tooling/danger/stable_branch_spec.rb b/spec/tooling/danger/stable_branch_spec.rb index 69e68f983fd..a33788f54f2 100644 --- a/spec/tooling/danger/stable_branch_spec.rb +++ b/spec/tooling/danger/stable_branch_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require 'gitlab-dangerfiles' -require 'gitlab/dangerfiles/spec_helper' require 'rspec-parameterized' +require 'fast_spec_helper' +require 'gitlab/dangerfiles/spec_helper' require 'httparty' require_relative '../../../tooling/danger/stable_branch' diff --git a/spec/tooling/fixtures/change_column_default_migration.txt b/spec/tooling/fixtures/change_column_default_migration.txt new file mode 100644 index 00000000000..a74c31464ee --- /dev/null +++ b/spec/tooling/fixtures/change_column_default_migration.txt @@ -0,0 +1,13 @@ ++# frozen_string_literal: true ++ ++class TestMigration < Gitlab::Database::Migration[2.1] ++ enable_lock_retries! ++ ++ def change ++ change_column_default('ci_builds', 'partition_id', from: 100, to: 101) ++ ++ change_column_default('ci_builds', 'partition_id', from: 100, to: 101) ++ ++ remove_column_default('ci_builds', 'partition_id') ++ end ++end diff --git a/spec/tooling/lib/tooling/find_changes_spec.rb b/spec/tooling/lib/tooling/find_changes_spec.rb index fef29ad3f2c..85e3eadac6f 100644 --- a/spec/tooling/lib/tooling/find_changes_spec.rb +++ b/spec/tooling/lib/tooling/find_changes_spec.rb @@ -15,7 +15,8 @@ RSpec.describe Tooling::FindChanges, feature_category: :tooling do changed_files_pathname: changed_files_pathname, predictive_tests_pathname: predictive_tests_pathname, frontend_fixtures_mapping_pathname: frontend_fixtures_mapping_pathname, - from: from) + from: from, + file_filter: file_filter) end let(:changed_files_pathname) { changed_files_file.path } @@ -23,6 +24,7 @@ RSpec.describe Tooling::FindChanges, feature_category: :tooling do let(:frontend_fixtures_mapping_pathname) { frontend_fixtures_mapping_file.path } let(:from) { :api } let(:gitlab_client) { double('GitLab') } # rubocop:disable RSpec/VerifiedDoubles + let(:file_filter) { ->(_) { true } } around do |example| self.changed_files_file = Tempfile.new('changed_files_file') @@ -89,6 +91,37 @@ RSpec.describe Tooling::FindChanges, feature_category: :tooling do subject end + + context 'when used with file_filter' do + let(:file_filter) { ->(file) { file['new_path'] =~ %r{doc/.*} } } + + let(:mr_changes_array) do + [ + { + "new_path" => "scripts/test.js", + "old_path" => "scripts/test.js" + }, + { + "new_path" => "doc/index.md", + "old_path" => "doc/index.md" + } + ] + end + + before do + # rubocop:disable RSpec/VerifiedDoubles -- The class from the GitLab gem isn't public, so we cannot use verified doubles for it. + allow(gitlab_client).to receive(:merge_request_changes) + .with('dummy-project', '1234') + .and_return(double(changes: mr_changes_array)) + # rubocop:enable RSpec/VerifiedDoubles + end + + it 'only writes matching files to output' do + subject + + expect(File.read(changed_files_file)).to eq('doc/index.md') + end + end end context 'when fetching changes from changed files' do diff --git a/spec/tooling/lib/tooling/test_map_generator_spec.rb b/spec/tooling/lib/tooling/test_map_generator_spec.rb index 1b369923d8d..eaaf525fc49 100644 --- a/spec/tooling/lib/tooling/test_map_generator_spec.rb +++ b/spec/tooling/lib/tooling/test_map_generator_spec.rb @@ -88,7 +88,7 @@ RSpec.describe Tooling::TestMapGenerator do end it 'displays a warning when report has no examples' do - expect { subject.parse('yaml3.yml') }.to output(%|No examples in yaml3.yml! Metadata: {:type=>"Crystalball::ExecutionMap", :commit=>"74056e8d9cf3773f43faa1cf5416f8779c8284c9", :timestamp=>1602671965, :version=>nil}\n|).to_stdout + expect { subject.parse('yaml3.yml') }.to output(%(No examples in yaml3.yml! Metadata: {:type=>"Crystalball::ExecutionMap", :commit=>"74056e8d9cf3773f43faa1cf5416f8779c8284c9", :timestamp=>1602671965, :version=>nil}\n)).to_stdout end end diff --git a/spec/tooling/quality/test_level_spec.rb b/spec/tooling/quality/test_level_spec.rb index 6ccd2e46f7b..d7d04015b48 100644 --- a/spec/tooling/quality/test_level_spec.rb +++ b/spec/tooling/quality/test_level_spec.rb @@ -46,7 +46,7 @@ RSpec.describe Quality::TestLevel, feature_category: :tooling do context 'when level is unit' do it 'returns a pattern' do expect(subject.pattern(:unit)) - .to eq("spec/{bin,channels,components,config,contracts,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,lib,metrics_server,models,policies,presenters,rack_servers,replicators,routing,rubocop,scripts,serializers,services,sidekiq,sidekiq_cluster,spam,support_specs,tasks,uploaders,validators,views,workers,tooling}{,/**/}*_spec.rb") + .to eq("spec/{bin,channels,click_house,components,config,contracts,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,lib,metrics_server,models,policies,presenters,rack_servers,replicators,routing,rubocop,scripts,serializers,services,sidekiq,sidekiq_cluster,spam,support_specs,tasks,uploaders,validators,views,workers,tooling}{,/**/}*_spec.rb") end end @@ -121,7 +121,7 @@ RSpec.describe Quality::TestLevel, feature_category: :tooling do context 'when level is unit' do it 'returns a regexp' do expect(subject.regexp(:unit)) - .to eq(%r{spec/(bin|channels|components|config|contracts|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|lib|metrics_server|models|policies|presenters|rack_servers|replicators|routing|rubocop|scripts|serializers|services|sidekiq|sidekiq_cluster|spam|support_specs|tasks|uploaders|validators|views|workers|tooling)/}) + .to eq(%r{spec/(bin|channels|click_house|components|config|contracts|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|lib|metrics_server|models|policies|presenters|rack_servers|replicators|routing|rubocop|scripts|serializers|services|sidekiq|sidekiq_cluster|spam|support_specs|tasks|uploaders|validators|views|workers|tooling)/}) end end |