From d831e60df87955bb9af013fa6833e4ac1c5808c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Wed, 2 Mar 2022 17:19:28 +0100
Subject: [PATCH] Update gitlab-dangerfiles which import plugins/rules
automatically
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This will reduce the boilerplate code.
Signed-off-by: Rémy Coutable <remy@rymai.me>
---
.gitlab/ci/frontend.gitlab-ci.yml | 5 ++-
Dangerfile | 14 ++------
Gemfile | 2 +-
Gemfile.lock | 7 ++--
Rakefile | 3 ++
.../Dangerfile => Dangerfile-bundle_size} | 1 +
danger/ci_templates/Dangerfile | 2 +-
danger/database/Dangerfile | 4 +--
danger/feature_flag/Dangerfile | 8 ++---
danger/specialization_labels/Dangerfile | 4 +--
danger/specs/Dangerfile | 2 +-
danger/z_metadata/Dangerfile | 6 ++--
lefthook.yml | 2 +-
lib/tasks/gitlab_danger.rake | 19 -----------
spec/tooling/danger/project_helper_spec.rb | 34 -------------------
tooling/danger/project_helper.rb | 32 -----------------
16 files changed, 30 insertions(+), 115 deletions(-)
rename danger/{bundle_size/Dangerfile => Dangerfile-bundle_size} (88%)
delete mode 100644 lib/tasks/gitlab_danger.rake
diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml
index cb07384676dda..5d4521b3f36a6 100644
--- a/.gitlab/ci/frontend.gitlab-ci.yml
+++ b/.gitlab/ci/frontend.gitlab-ci.yml
@@ -335,10 +335,13 @@ bundle-size-review:
stage: test
needs: ["compile-production-assets"]
script:
+ - source scripts/utils.sh
- mkdir -p bundle-size-review
- cp webpack-report/index.html bundle-size-review/bundle-report.html
- yarn global add https://gitlab.com/gitlab-org/frontend/playground/webpack-memory-metrics.git
- - danger --dangerfile=danger/bundle_size/Dangerfile --fail-on-errors=true --verbose --danger_id=bundle-size-review
+ - |
+ danger_id=$(echo -n ${DANGER_GITLAB_API_TOKEN} | md5sum | awk '{print $1}' | cut -c5-10)
+ run_timed_command "danger --dangerfile=danger/Dangerfile-bundle_size --fail-on-errors=true --verbose --danger_id=bundle-size-review-${danger_id}"
artifacts:
when: always
name: bundle-size-review
diff --git a/Dangerfile b/Dangerfile
index aaa1aae813bd7..280a73d432c83 100644
--- a/Dangerfile
+++ b/Dangerfile
@@ -11,16 +11,8 @@ project_name = ee? ? 'gitlab' : 'gitlab-foss'
Gitlab::Dangerfiles.for_project(self, project_name) do |gitlab_dangerfiles|
gitlab_dangerfiles.import_plugins
+ gitlab_dangerfiles.config.ci_only_rules = ProjectHelper::CI_ONLY_RULES
+ gitlab_dangerfiles.config.files_to_category = ProjectHelper::CATEGORIES
- unless helper.release_automation?
- danger.import_plugin('danger/plugins/*.rb')
- gitlab_dangerfiles.import_dangerfiles(except: %w[simple_roulette])
- gitlab_dangerfiles.config.files_to_category = ProjectHelper::CATEGORIES
- end
-end
-
-return if helper.release_automation?
-
-project_helper.rule_names.each do |rule|
- danger.import_dangerfile(path: File.join('danger', rule))
+ gitlab_dangerfiles.import_dangerfiles(except: %w[simple_roulette])
end
diff --git a/Gemfile b/Gemfile
index ac5b5517f228f..49a5e75d4e914 100644
--- a/Gemfile
+++ b/Gemfile
@@ -403,7 +403,7 @@ group :development, :test do
end
group :development, :test, :danger do
- gem 'gitlab-dangerfiles', '~> 2.11.0', require: false
+ gem 'gitlab-dangerfiles', '~> 3.0', require: false
end
group :development, :test, :coverage do
diff --git a/Gemfile.lock b/Gemfile.lock
index dbffdd3835cc3..6a981d37fe697 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -224,7 +224,7 @@ GEM
css_parser (1.7.0)
addressable
daemons (1.3.1)
- danger (8.4.5)
+ danger (8.5.0)
claide (~> 1.0)
claide-plugins (>= 0.9.2)
colored2 (~> 3.1)
@@ -463,9 +463,10 @@ GEM
terminal-table (~> 1.5, >= 1.5.1)
gitlab-chronic (0.10.5)
numerizer (~> 0.2)
- gitlab-dangerfiles (2.11.0)
+ gitlab-dangerfiles (3.0.0)
danger (>= 8.4.5)
danger-gitlab (>= 8.0.0)
+ rake
gitlab-experiment (0.7.0)
activesupport (>= 3.0)
request_store (>= 1.0)
@@ -1494,7 +1495,7 @@ DEPENDENCIES
gitaly (~> 14.9.0.pre.rc4)
github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5)
- gitlab-dangerfiles (~> 2.11.0)
+ gitlab-dangerfiles (~> 3.0)
gitlab-experiment (~> 0.7.0)
gitlab-fog-azure-rm (~> 1.2.0)
gitlab-labkit (~> 0.22.0)
diff --git a/Rakefile b/Rakefile
index 9545516d2a916..9a651fda7a0d5 100755
--- a/Rakefile
+++ b/Rakefile
@@ -16,3 +16,6 @@ require File.expand_path('config/initializers/01_active_record_database_tasks_co
Gitlab::Application.load_tasks
Knapsack.load_tasks if defined?(Knapsack)
+
+require 'gitlab-dangerfiles'
+Gitlab::Dangerfiles.load_tasks
diff --git a/danger/bundle_size/Dangerfile b/danger/Dangerfile-bundle_size
similarity index 88%
rename from danger/bundle_size/Dangerfile
rename to danger/Dangerfile-bundle_size
index b824edb5dab42..23ab726096e06 100644
--- a/danger/bundle_size/Dangerfile
+++ b/danger/Dangerfile-bundle_size
@@ -1,4 +1,5 @@
# frozen_string_literal: true
+# This file isn't named "Dangerfile" so that it's not imported by default since it's only meant to be run in the `bundle-size-review` job.
analysis_result = "./bundle-size-review/analysis.json"
markdown_result = "./bundle-size-review/comparison.md"
diff --git a/danger/ci_templates/Dangerfile b/danger/ci_templates/Dangerfile
index 3d57436ef947d..ace9905e91d21 100644
--- a/danger/ci_templates/Dangerfile
+++ b/danger/ci_templates/Dangerfile
@@ -19,7 +19,7 @@ return unless helper.ci?
template_paths_to_review = helper.changes_by_category[:ci_template]
-if gitlab.mr_labels.include?('ci::templates') || template_paths_to_review.any?
+if helper.mr_labels.include?('ci::templates') || template_paths_to_review.any?
message 'This merge request adds or changes files that require a ' \
'review from the CI/CD Templates maintainers.'
diff --git a/danger/database/Dangerfile b/danger/database/Dangerfile
index 0128f0fa19513..f94184263adb7 100644
--- a/danger/database/Dangerfile
+++ b/danger/database/Dangerfile
@@ -49,11 +49,11 @@ if geo_migration_created && !geo_db_schema_updated
end
return unless helper.ci?
-return if gitlab.mr_labels.include?(DATABASE_APPROVED_LABEL)
+return if helper.mr_labels.include?(DATABASE_APPROVED_LABEL)
db_paths_to_review = helper.changes_by_category[:database]
-if gitlab.mr_labels.include?('database') || db_paths_to_review.any?
+if helper.mr_labels.include?('database') || db_paths_to_review.any?
message 'This merge request adds or changes files that require a ' \
'review from the [Database team](https://gitlab.com/groups/gl-database/-/group_members).'
diff --git a/danger/feature_flag/Dangerfile b/danger/feature_flag/Dangerfile
index 5fe9d42a7a1c4..d5b907377aa61 100644
--- a/danger/feature_flag/Dangerfile
+++ b/danger/feature_flag/Dangerfile
@@ -22,21 +22,21 @@ def check_feature_flag_yaml(feature_flag)
end
rescue Psych::Exception
# YAML could not be parsed, fail the build.
- fail "#{gitlab.html_link(feature_flag.path)} isn't valid YAML! #{SEE_DOC}"
+ fail "#{helper.html_link(feature_flag.path)} isn't valid YAML! #{SEE_DOC}"
rescue StandardError => e
warn "There was a problem trying to check the Feature Flag file. Exception: #{e.class.name} - #{e.message}"
end
def message_for_feature_flag_missing_group!(feature_flag:, mr_group_label:)
if mr_group_label.nil?
- warn "Consider setting `group` in #{gitlab.html_link(feature_flag.path)}. #{SEE_DOC}"
+ warn "Consider setting `group` in #{helper.html_link(feature_flag.path)}. #{SEE_DOC}"
else
mr_line = feature_flag.raw.lines.find_index("group:\n")
if mr_line
markdown(format(SUGGEST_MR_COMMENT, group: mr_group_label), file: feature_flag.path, line: mr_line.succ)
else
- warn %(Consider setting `group: "#{mr_group_label}"` in #{gitlab.html_link(feature_flag.path)}. #{SEE_DOC})
+ warn %(Consider setting `group: "#{mr_group_label}"` in #{helper.html_link(feature_flag.path)}. #{SEE_DOC})
end
end
end
@@ -60,7 +60,7 @@ def message_for_feature_flag_with_group!(feature_flag:, mr_group_label:)
if mr_group_label.nil?
helper.labels_to_add << feature_flag.group
else
- fail %(`group` is set to ~"#{feature_flag.group}" in #{gitlab.html_link(feature_flag.path)}, which does not match ~"#{mr_group_label}" set on the MR!)
+ fail %(`group` is set to ~"#{feature_flag.group}" in #{helper.html_link(feature_flag.path)}, which does not match ~"#{mr_group_label}" set on the MR!)
end
end
diff --git a/danger/specialization_labels/Dangerfile b/danger/specialization_labels/Dangerfile
index f161c470f36ad..615ceb8625d03 100644
--- a/danger/specialization_labels/Dangerfile
+++ b/danger/specialization_labels/Dangerfile
@@ -16,11 +16,11 @@ SPECIALIZATIONS = {
labels_to_add = helper.changes_by_category.each_with_object([]) do |(category, _changes), memo|
label = SPECIALIZATIONS[category]
next unless label
- next if gitlab.mr_labels.include?(label)
+ next if helper.mr_labels.include?(label)
# Don't override already-set scoped labels.
label_scope = label.split('::')[0...-1].join('::')
- next if !label_scope.empty? && gitlab.mr_labels.any? { |mr_label| mr_label.start_with?(label_scope) }
+ next if !label_scope.empty? && helper.has_scoped_label_with_scope?(label_scope)
memo << label
end
diff --git a/danger/specs/Dangerfile b/danger/specs/Dangerfile
index 8ef046f7bc16a..dc9809b20b553 100644
--- a/danger/specs/Dangerfile
+++ b/danger/specs/Dangerfile
@@ -37,7 +37,7 @@ has_ee_app_changes = all_changed_files.grep(%r{\Aee/(app|lib|db/(geo/)?(post_)?m
spec_changes = specs.changed_specs_files(ee: :exclude)
has_spec_changes = spec_changes.any?
has_ee_spec_changes = specs.changed_specs_files(ee: :only).any?
-new_specs_needed = (gitlab.mr_labels & NO_SPECS_LABELS).empty?
+new_specs_needed = (helper.mr_labels & NO_SPECS_LABELS).empty?
if (has_app_changes || has_ee_app_changes) && !(has_spec_changes || has_ee_spec_changes) && new_specs_needed
warn format(NO_NEW_SPEC_MESSAGE, labels: helper.labels_list(NO_SPECS_LABELS)), sticky: false
diff --git a/danger/z_metadata/Dangerfile b/danger/z_metadata/Dangerfile
index 546fdc8de5f87..8aeb0c33a9ca1 100644
--- a/danger/z_metadata/Dangerfile
+++ b/danger/z_metadata/Dangerfile
@@ -14,12 +14,12 @@ end
has_milestone = !gitlab.mr_json["milestone"].nil?
-unless has_milestone || (helper.security_mr? && gitlab.branch_for_base == DEFAULT_BRANCH)
+unless has_milestone || (helper.security_mr? && helper.mr_target_branch == DEFAULT_BRANCH)
warn "This merge request does not refer to an existing milestone.", sticky: false
end
-has_pick_into_stable_label = gitlab.mr_labels.find { |label| label.start_with?('Pick into') }
+has_pick_into_stable_label = helper.mr_labels.find { |label| label.start_with?('Pick into') }
-if gitlab.branch_for_base != DEFAULT_BRANCH && !has_pick_into_stable_label && !helper.security_mr?
+if helper.mr_target_branch != DEFAULT_BRANCH && !has_pick_into_stable_label && !helper.security_mr?
warn "Most of the time, merge requests should target `#{DEFAULT_BRANCH}`. Otherwise, please set the relevant `Pick into X.Y` label."
end
diff --git a/lefthook.yml b/lefthook.yml
index b21db70a38545..086e70fcbb9e2 100644
--- a/lefthook.yml
+++ b/lefthook.yml
@@ -2,7 +2,7 @@ pre-push:
parallel: true
commands:
danger:
- run: CI_PROJECT_DIR=. bundle exec danger dry_run
+ run: bundle exec rake danger_local
eslint:
tags: frontend style
files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD
diff --git a/lib/tasks/gitlab_danger.rake b/lib/tasks/gitlab_danger.rake
deleted file mode 100644
index ff9464a588a3c..0000000000000
--- a/lib/tasks/gitlab_danger.rake
+++ /dev/null
@@ -1,19 +0,0 @@
-# frozen_string_literal: true
-
-desc 'Run local Danger rules'
-task :danger_local do
- require_relative '../../tooling/danger/project_helper'
- require 'gitlab/popen'
-
- puts("#{Tooling::Danger::ProjectHelper.local_warning_message}\n")
-
- # _status will _always_ be 0, regardless of failure or success :(
- output, _status = Gitlab::Popen.popen(%w{danger dry_run})
-
- if output.empty?
- puts(Tooling::Danger::ProjectHelper.success_message)
- else
- puts(output)
- exit(1)
- end
-end
diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb
index 902e01e2cbde9..b3fb592c2e358 100644
--- a/spec/tooling/danger/project_helper_spec.rb
+++ b/spec/tooling/danger/project_helper_spec.rb
@@ -276,40 +276,6 @@
end
end
- describe '.local_warning_message' do
- it 'returns an informational message with rules that can run' do
- expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: ci_config, database, documentation, duplicate_yarn_dependencies, eslint, gitaly, pajamas, pipeline, prettier, product_intelligence, utility_css, vue_shared_documentation, datateam')
- end
- end
-
- describe '.success_message' do
- it 'returns an informational success message' do
- expect(described_class.success_message).to eq('==> No Danger rule violations!')
- end
- end
-
- describe '#rule_names' do
- context 'when running locally' do
- before do
- expect(fake_helper).to receive(:ci?).and_return(false)
- end
-
- it 'returns local only rules' do
- expect(project_helper.rule_names).to match_array(described_class::LOCAL_RULES)
- end
- end
-
- context 'when running under CI' do
- before do
- expect(fake_helper).to receive(:ci?).and_return(true)
- end
-
- it 'returns all rules' do
- expect(project_helper.rule_names).to eq(described_class::LOCAL_RULES | described_class::CI_ONLY_RULES)
- end
- end
- end
-
describe '#file_lines' do
let(:filename) { 'spec/foo_spec.rb' }
let(:file_spy) { spy }
diff --git a/tooling/danger/project_helper.rb b/tooling/danger/project_helper.rb
index 02002e1d1b2c1..fc87498f5d024 100644
--- a/tooling/danger/project_helper.rb
+++ b/tooling/danger/project_helper.rb
@@ -3,22 +3,6 @@
module Tooling
module Danger
module ProjectHelper
- LOCAL_RULES ||= %w[
- ci_config
- database
- documentation
- duplicate_yarn_dependencies
- eslint
- gitaly
- pajamas
- pipeline
- prettier
- product_intelligence
- utility_css
- vue_shared_documentation
- datateam
- ].freeze
-
CI_ONLY_RULES ||= %w[
ce_ee_vue_templates
ci_templates
@@ -31,8 +15,6 @@ module ProjectHelper
z_metadata
].freeze
- MESSAGE_PREFIX = '==>'
-
# First-match win, so be sure to put more specific regex at the top...
CATEGORIES = {
[%r{usage_data\.rb}, %r{^(\+|-).*\s+(count|distinct_count|estimate_batch_distinct_count)\(.*\)(.*)$}] => [:database, :backend, :product_intelligence],
@@ -181,20 +163,6 @@ module ProjectHelper
%r{\.js\z} => :frontend
}.freeze
- def local_warning_message
- "#{MESSAGE_PREFIX} Only the following Danger rules can be run locally: #{LOCAL_RULES.join(', ')}"
- end
- module_function :local_warning_message # rubocop:disable Style/AccessModifierDeclarations
-
- def success_message
- "#{MESSAGE_PREFIX} No Danger rule violations!"
- end
- module_function :success_message # rubocop:disable Style/AccessModifierDeclarations
-
- def rule_names
- helper.ci? ? LOCAL_RULES | CI_ONLY_RULES : LOCAL_RULES
- end
-
def file_lines(filename)
read_file(filename).lines(chomp: true)
end
--
GitLab