From af3f6fb5f7b0a3d765541112650161b2f18fd117 Mon Sep 17 00:00:00 2001 From: charlie ablett <cablett@gitlab.com> Date: Wed, 6 Sep 2023 20:24:48 +0000 Subject: [PATCH] Add ClickHouse review in Danger - Mention group - Label support --- danger/database/clickhouse/Dangerfile | 24 +++++++++ spec/tooling/danger/clickhouse_spec.rb | 70 ++++++++++++++++++++++++++ tooling/danger/clickhouse.rb | 11 ++++ tooling/danger/project_helper.rb | 4 ++ 4 files changed, 109 insertions(+) create mode 100644 danger/database/clickhouse/Dangerfile create mode 100644 spec/tooling/danger/clickhouse_spec.rb create mode 100644 tooling/danger/clickhouse.rb diff --git a/danger/database/clickhouse/Dangerfile b/danger/database/clickhouse/Dangerfile new file mode 100644 index 0000000000000..5a82779ce9068 --- /dev/null +++ b/danger/database/clickhouse/Dangerfile @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +CH_MESSAGE = <<~MSG +This merge request requires a ClickHouse review. To make sure these +changes are reviewed, take the following steps: + +1. Ensure the merge request has ~clickhouse and ~"clickhouse::review pending" labels. +1. Assign and mention a ClickHouse reviewer. +MSG + +CH_UNREVIEWED_LABEL = 'clickhouse::review pending' +CH_APPROVED_LABEL = 'clickhouse::approved' + +return if stable_branch.valid_stable_branch? +return if helper.mr_labels.include?(CH_UNREVIEWED_LABEL) + +if helper.mr_labels.include?('clickhouse') || clickhouse.changes.any? + message 'This merge request adds or changes files that require a ' \ + 'review from the [GitLab ClickHouse team](https://gitlab.com/groups/gl-clickhouse/-/group_members).' + + markdown(CH_MESSAGE) + + helper.labels_to_add << CH_UNREVIEWED_LABEL unless helper.has_scoped_label_with_scope?("clickhouse") +end diff --git a/spec/tooling/danger/clickhouse_spec.rb b/spec/tooling/danger/clickhouse_spec.rb new file mode 100644 index 0000000000000..ad2f0b4a827f8 --- /dev/null +++ b/spec/tooling/danger/clickhouse_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'rspec-parameterized' +require 'gitlab-dangerfiles' +require 'danger' +require 'danger/plugins/internal/helper' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../tooling/danger/clickhouse' + +RSpec.describe Tooling::Danger::Clickhouse, feature_category: :tooling do + include_context "with dangerfile" + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:migration_files) do + %w[ + db/click_house/20220901010203_add_widgets_table.rb + db/click_house/20220909010203_add_properties_column.rb + db/click_house/20220910010203_drop_tools_table.rb + db/click_house/20220912010203_add_index_to_widgets_table.rb + ] + end + + subject(:clickhouse) { fake_danger.new(helper: fake_helper) } + + describe '#changes' do + using RSpec::Parameterized::TableSyntax + + where do + { + 'with click_house gem changes' => { + modified_files: %w[gems/click_house-client/lib/click_house/client.rb], + changes_by_category: { + database: [], + clickhouse: %w[gems/click_house-client/lib/click_house/client.rb] + }, + impacted_files: %w[gems/click_house-client/lib/click_house/client.rb] + }, + 'with clickhouse data changes' => { + modified_files: %w[db/clickhouse/20230720114001_add_magic_table_migration.rb], + changes_by_category: { + database: [], + clickhouse: %w[db/clickhouse/20230720114001_add_magic_table_migration.rb] + }, + impacted_files: %w[db/clickhouse/20230720114001_add_magic_table_migration.rb] + }, + 'with clickhouse app changes' => { + modified_files: %w[lib/click_house/query_builder.rb], + changes_by_category: { + database: [], + clickhouse: %w[lib/click_house/query_builder.rb] + }, + impacted_files: %w[lib/click_house/query_builder.rb] + } + } + end + + with_them do + before do + allow(fake_helper).to receive(:modified_files).and_return(modified_files) + allow(fake_helper).to receive(:all_changed_files).and_return(modified_files) + allow(fake_helper).to receive(:changes_by_category).and_return(changes_by_category) + end + + it 'returns only clickhouse changes' do + expect(clickhouse.changes).to match impacted_files + end + end + end +end diff --git a/tooling/danger/clickhouse.rb b/tooling/danger/clickhouse.rb new file mode 100644 index 0000000000000..b36e12219edd3 --- /dev/null +++ b/tooling/danger/clickhouse.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Tooling + module Danger + module Clickhouse + def changes + helper.changes_by_category[:clickhouse] + end + end + end +end diff --git a/tooling/danger/project_helper.rb b/tooling/danger/project_helper.rb index 633c7b570975e..90166438ca046 100644 --- a/tooling/danger/project_helper.rb +++ b/tooling/danger/project_helper.rb @@ -106,6 +106,10 @@ module ProjectHelper %r{\A((ee|jh)/)?app/finders/} => [:database, :backend], %r{\Arubocop/cop/migration(/|\.rb)} => :database, + %r{\A(ee/)?db/click_house/} => :clickhouse, + %r{\Agems/click_house-client/} => :clickhouse, + %r{click(-)?house} => :clickhouse, + %r{\Alib/gitlab/ci/templates} => :ci_template, %r{\A((ee|jh)/)?spec/features/} => :test, -- GitLab