From 95a3801f23baca10f05211b64f17da38f23ddd46 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Wed, 23 Sep 2020 17:55:27 +0200
Subject: [PATCH] Use the gitlab-dangerfiles gem
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The gem provides helpers that can be reused by multiple projects.
Signed-off-by: Rémy Coutable <remy@rymai.me>
---
.gitlab/ci/global.gitlab-ci.yml | 8 +
.gitlab/ci/review.gitlab-ci.yml | 18 +-
Dangerfile | 14 +-
Gemfile | 6 +-
Gemfile.lock | 15 +-
danger/changelog/Dangerfile | 10 +-
danger/ci_templates/Dangerfile | 6 +-
danger/commit_messages/Dangerfile | 20 +-
danger/database/Dangerfile | 12 +-
danger/documentation/Dangerfile | 12 +-
danger/duplicate_yarn_dependencies/Dangerfile | 2 +-
danger/eslint/Dangerfile | 2 +-
danger/karma/Dangerfile | 4 +-
danger/plugins/changelog.rb | 2 +-
.../plugins/{helper.rb => project_helper.rb} | 8 +-
danger/plugins/roulette.rb | 10 -
danger/plugins/sidekiq_queues.rb | 2 +-
danger/prettier/Dangerfile | 2 +-
danger/roulette/Dangerfile | 8 +-
danger/specialization_labels/Dangerfile | 6 +-
lib/tasks/gitlab_danger.rake | 6 +-
spec/tooling/danger/base_linter_spec.rb | 192 -----
spec/tooling/danger/changelog_spec.rb | 35 +-
spec/tooling/danger/commit_linter_spec.rb | 241 ------
spec/tooling/danger/danger_spec_helper.rb | 17 -
spec/tooling/danger/emoji_checker_spec.rb | 37 -
spec/tooling/danger/feature_flag_spec.rb | 23 +-
spec/tooling/danger/helper_spec.rb | 781 ------------------
.../danger/merge_request_linter_spec.rb | 54 --
spec/tooling/danger/project_helper_spec.rb | 260 ++++++
spec/tooling/danger/roulette_spec.rb | 429 ----------
spec/tooling/danger/sidekiq_queues_spec.rb | 11 +-
spec/tooling/danger/teammate_spec.rb | 225 -----
spec/tooling/danger/title_linting_spec.rb | 91 --
.../danger/weightage/maintainers_spec.rb | 34 -
.../danger/weightage/reviewers_spec.rb | 63 --
spec/tooling/gitlab_danger_spec.rb | 76 --
tooling/danger/base_linter.rb | 96 ---
tooling/danger/changelog.rb | 12 +-
tooling/danger/commit_linter.rb | 150 ----
tooling/danger/emoji_checker.rb | 45 -
tooling/danger/helper.rb | 383 ---------
tooling/danger/merge_request_linter.rb | 30 -
tooling/danger/project_helper.rb | 181 ++++
tooling/danger/request_helper.rb | 23 -
tooling/danger/roulette.rb | 169 ----
tooling/danger/teammate.rb | 121 ---
tooling/danger/title_linting.rb | 38 -
tooling/danger/weightage.rb | 10 -
tooling/danger/weightage/maintainers.rb | 33 -
tooling/danger/weightage/reviewers.rb | 65 --
tooling/gitlab_danger.rb | 59 --
52 files changed, 562 insertions(+), 3595 deletions(-)
rename danger/plugins/{helper.rb => project_helper.rb} (54%)
delete mode 100644 danger/plugins/roulette.rb
delete mode 100644 spec/tooling/danger/base_linter_spec.rb
delete mode 100644 spec/tooling/danger/commit_linter_spec.rb
delete mode 100644 spec/tooling/danger/danger_spec_helper.rb
delete mode 100644 spec/tooling/danger/emoji_checker_spec.rb
delete mode 100644 spec/tooling/danger/helper_spec.rb
delete mode 100644 spec/tooling/danger/merge_request_linter_spec.rb
create mode 100644 spec/tooling/danger/project_helper_spec.rb
delete mode 100644 spec/tooling/danger/roulette_spec.rb
delete mode 100644 spec/tooling/danger/teammate_spec.rb
delete mode 100644 spec/tooling/danger/title_linting_spec.rb
delete mode 100644 spec/tooling/danger/weightage/maintainers_spec.rb
delete mode 100644 spec/tooling/danger/weightage/reviewers_spec.rb
delete mode 100644 spec/tooling/gitlab_danger_spec.rb
delete mode 100644 tooling/danger/base_linter.rb
delete mode 100644 tooling/danger/commit_linter.rb
delete mode 100644 tooling/danger/emoji_checker.rb
delete mode 100644 tooling/danger/helper.rb
delete mode 100644 tooling/danger/merge_request_linter.rb
create mode 100644 tooling/danger/project_helper.rb
delete mode 100644 tooling/danger/request_helper.rb
delete mode 100644 tooling/danger/roulette.rb
delete mode 100644 tooling/danger/teammate.rb
delete mode 100644 tooling/danger/title_linting.rb
delete mode 100644 tooling/danger/weightage.rb
delete mode 100644 tooling/danger/weightage/maintainers.rb
delete mode 100644 tooling/danger/weightage/reviewers.rb
delete mode 100644 tooling/gitlab_danger.rb
diff --git a/.gitlab/ci/global.gitlab-ci.yml b/.gitlab/ci/global.gitlab-ci.yml
index 274629f9c609e..de4b609098d42 100644
--- a/.gitlab/ci/global.gitlab-ci.yml
+++ b/.gitlab/ci/global.gitlab-ci.yml
@@ -49,6 +49,14 @@
- vendor/ruby/
policy: pull
+.danger-review-cache:
+ cache:
+ key: "danger-review-v1"
+ paths:
+ - vendor/ruby/
+ - node_modules/
+ policy: pull
+
.qa-cache:
cache:
key: "qa-v2"
diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml
index c18e898dc1245..76191a923bf5d 100644
--- a/.gitlab/ci/review.gitlab-ci.yml
+++ b/.gitlab/ci/review.gitlab-ci.yml
@@ -225,12 +225,22 @@ parallel-spec-reports:
danger-review:
extends:
- .default-retry
- - .yarn-cache
+ - .danger-review-cache
- .review:rules:danger
- image: registry.gitlab.com/gitlab-org/gitlab-build-images:danger
stage: test
needs: []
- script:
+ before_script:
- source ./scripts/utils.sh
+ - run_timed_command "bundle install --jobs=$(nproc) --path=vendor --retry=3 --quiet --with danger"
- run_timed_command "retry yarn install --frozen-lockfile"
- - danger --fail-on-errors=true --verbose
+ script:
+ - run_timed_command "bundle exec danger --fail-on-errors=true --verbose"
+
+update-danger-review-cache:
+ extends:
+ - danger-review
+ - .shared:rules:update-cache
+ stage: prepare
+ script: echo 'Cache is fresh!'
+ cache:
+ policy: push # We want to rebuild the cache from scratch to ensure stale dependencies are cleaned up.
diff --git a/Dangerfile b/Dangerfile
index 34e0efa027a3d..699be613f2d85 100644
--- a/Dangerfile
+++ b/Dangerfile
@@ -1,20 +1,18 @@
# frozen_string_literal: true
-require_relative 'tooling/gitlab_danger'
-require_relative 'tooling/danger/request_helper'
+require 'gitlab-dangerfiles'
-Dir["danger/plugins/*.rb"].sort.each { |f| danger.import_plugin(f) }
+Gitlab::Dangerfiles.import_plugins(danger)
+danger.import_plugin('danger/plugins/*.rb')
return if helper.release_automation?
-gitlab_danger = GitlabDanger.new(helper.gitlab_helper)
-
-gitlab_danger.rule_names.each do |file|
- danger.import_dangerfile(path: File.join('danger', file))
+project_helper.rule_names.each do |rule|
+ danger.import_dangerfile(path: File.join('danger', rule))
end
anything_to_post = status_report.values.any? { |data| data.any? }
-if gitlab_danger.ci? && anything_to_post
+if helper.ci? && anything_to_post
markdown("**If needed, you can retry the [`danger-review` job](#{ENV['CI_JOB_URL']}) that generated this comment.**")
end
diff --git a/Gemfile b/Gemfile
index dd1054080ea9e..3af41fd1dc51f 100644
--- a/Gemfile
+++ b/Gemfile
@@ -344,7 +344,6 @@ end
group :development do
gem 'brakeman', '~> 4.2', require: false
- gem 'danger', '~> 8.0.6', require: false
gem 'lefthook', '~> 0.7', require: false
gem 'letter_opener_web', '~> 1.3.4'
@@ -399,6 +398,11 @@ group :development, :test do
gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false
end
+group :development, :test, :danger do
+ gem 'danger-gitlab', '~> 8.0', require: false
+ gem 'gitlab-dangerfiles', '~> 0.8.0', require: false
+end
+
group :development, :test, :coverage do
gem 'simplecov', '~> 0.18.5', require: false
gem 'simplecov-cobertura', '~> 1.3.1', require: false
diff --git a/Gemfile.lock b/Gemfile.lock
index 9a5c0bc10d4e7..5b041f7db6723 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -216,7 +216,7 @@ GEM
css_parser (1.7.0)
addressable
daemons (1.3.1)
- danger (8.0.6)
+ danger (8.2.3)
claide (~> 1.0)
claide-plugins (>= 0.9.2)
colored2 (~> 3.1)
@@ -228,7 +228,10 @@ GEM
kramdown-parser-gfm (~> 1.0)
no_proxy_fix
octokit (~> 4.7)
- terminal-table (~> 1)
+ terminal-table (>= 1, < 4)
+ danger-gitlab (8.0.0)
+ danger
+ gitlab (~> 4.2, >= 4.2.0)
database_cleaner (1.7.0)
debugger-ruby_core_source (1.3.8)
deckar01-task_list (2.3.1)
@@ -428,8 +431,13 @@ GEM
gitaly (13.9.0.pre.rc1)
grpc (~> 1.0)
github-markup (1.7.0)
+ gitlab (4.16.1)
+ httparty (~> 0.14, >= 0.14.0)
+ terminal-table (~> 1.5, >= 1.5.1)
gitlab-chronic (0.10.5)
numerizer (~> 0.2)
+ gitlab-dangerfiles (0.8.0)
+ danger
gitlab-experiment (0.5.0)
activesupport (>= 3.0)
scientist (~> 1.5, >= 1.5.0)
@@ -1367,7 +1375,7 @@ DEPENDENCIES
countries (~> 3.0)
creole (~> 0.5.0)
crystalball (~> 0.7.0)
- danger (~> 8.0.6)
+ danger-gitlab (~> 8.0)
database_cleaner (~> 1.7.0)
deckar01-task_list (= 2.3.1)
default_value_for (~> 3.4.0)
@@ -1413,6 +1421,7 @@ DEPENDENCIES
gitaly (~> 13.9.0.pre.rc1)
github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5)
+ gitlab-dangerfiles (~> 0.8.0)
gitlab-experiment (~> 0.5.0)
gitlab-fog-azure-rm (~> 1.0.1)
gitlab-fog-google (~> 1.13)
diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile
index 377f3592d5622..444303d4b9ec1 100644
--- a/danger/changelog/Dangerfile
+++ b/danger/changelog/Dangerfile
@@ -17,8 +17,8 @@ def check_changelog_yaml(path)
raw_file = File.read(path)
yaml = YAML.safe_load(raw_file)
- fail "`title` should be set, in #{gitlab.html_link(path)}! #{SEE_DOC}" if yaml["title"].nil?
- fail "`type` should be set, in #{gitlab.html_link(path)}! #{SEE_DOC}" if yaml["type"].nil?
+ fail "`title` should be set, in #{helper.html_link(path)}! #{SEE_DOC}" if yaml["title"].nil?
+ fail "`type` should be set, in #{helper.html_link(path)}! #{SEE_DOC}" if yaml["type"].nil?
return if helper.security_mr?
@@ -30,20 +30,20 @@ def check_changelog_yaml(path)
if mr_line
markdown(format(SUGGEST_MR_COMMENT, mr_iid: gitlab.mr_json["iid"]), file: path, line: mr_line.succ)
else
- message "Consider setting `merge_request` to #{gitlab.mr_json["iid"]} in #{gitlab.html_link(path)}. #{SEE_DOC}"
+ message "Consider setting `merge_request` to #{gitlab.mr_json["iid"]} in #{helper.html_link(path)}. #{SEE_DOC}"
end
elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !cherry_pick_against_stable_branch
fail "Merge request ID was not set to #{gitlab.mr_json["iid"]}! #{SEE_DOC}"
end
rescue Psych::Exception
# YAML could not be parsed, fail the build.
- fail "#{gitlab.html_link(path)} isn't valid YAML! #{SEE_DOC}"
+ fail "#{helper.html_link(path)} isn't valid YAML! #{SEE_DOC}"
rescue StandardError => e
warn "There was a problem trying to check the Changelog. Exception: #{e.class.name} - #{e.message}"
end
def check_changelog_path(path)
- ee_changes = helper.all_ee_changes.dup
+ ee_changes = project_helper.all_ee_changes.dup
ee_changes.delete(path)
if ee_changes.any? && !changelog.ee_changelog? && !changelog.required?
diff --git a/danger/ci_templates/Dangerfile b/danger/ci_templates/Dangerfile
index 34b4bbff7a5e1..fcd9080e7d1c6 100644
--- a/danger/ci_templates/Dangerfile
+++ b/danger/ci_templates/Dangerfile
@@ -1,7 +1,5 @@
# frozen_string_literal: true
-gitlab_danger = GitlabDanger.new(helper.gitlab_helper)
-
TEMPLATE_MESSAGE = <<~MSG
This merge request requires a CI/CD Template review. To make sure these
changes are reviewed, take the following steps:
@@ -17,9 +15,9 @@ TEMPLATE_FILES_MESSAGE = <<~MSG
The following files require a review from the CI/CD Templates maintainers:
MSG
-return unless gitlab_danger.ci?
+return unless helper.ci?
-template_paths_to_review = helper.changes_by_category[:ci_template]
+template_paths_to_review = project_helper.changes_by_category[:ci_template]
if gitlab.mr_labels.include?('ci::templates') || template_paths_to_review.any?
message 'This merge request adds or changes files that require a ' \
diff --git a/danger/commit_messages/Dangerfile b/danger/commit_messages/Dangerfile
index 96a0c08c1840d..b4a0b6ad8cb32 100644
--- a/danger/commit_messages/Dangerfile
+++ b/danger/commit_messages/Dangerfile
@@ -1,7 +1,7 @@
# frozen_string_literal: true
-require_relative File.expand_path('../../tooling/danger/commit_linter', __dir__)
-require_relative File.expand_path('../../tooling/danger/merge_request_linter', __dir__)
+require 'gitlab/dangerfiles/commit_linter'
+require 'gitlab/dangerfiles/merge_request_linter'
COMMIT_MESSAGE_GUIDELINES = "https://docs.gitlab.com/ee/development/contributing/merge_request_workflow.html#commit-messages-guidelines"
MORE_INFO = "For more information, take a look at our [Commit message guidelines](#{COMMIT_MESSAGE_GUIDELINES})."
@@ -18,10 +18,6 @@ This merge request includes more than %<max_commits_count>d commits. Each commit
If this merge request contains commits that do not meet this criteria and/or contains intermediate work, please rebase these commits into a smaller number of commits or split this merge request into multiple smaller merge requests.
MSG
-def gitlab_danger
- @gitlab_danger ||= GitlabDanger.new(helper.gitlab_helper)
-end
-
def fail_commit(commit, message, more_info: true)
self.fail(build_message(commit, message, more_info: more_info))
end
@@ -39,22 +35,22 @@ end
def squash_mr?
# Locally, we assume the MR is set to be squashed so that the user only sees warnings instead of errors.
- gitlab_danger.ci? ? gitlab.mr_json['squash'] : true
+ helper.ci? ? gitlab.mr_json['squash'] : true
end
def wip_mr?
- gitlab_danger.ci? ? gitlab.mr_json['work_in_progress'] : false
+ helper.ci? ? gitlab.mr_json['work_in_progress'] : false
end
def danger_job_link
- gitlab_danger.ci? ? "[#{THE_DANGER_JOB_TEXT}](#{ENV['CI_JOB_URL']})" : THE_DANGER_JOB_TEXT
+ helper.ci? ? "[#{THE_DANGER_JOB_TEXT}](#{ENV['CI_JOB_URL']})" : THE_DANGER_JOB_TEXT
end
# Perform various checks against commits. We're not using
# https://github.com/jonallured/danger-commit_lint because its output is not
# very helpful, and it doesn't offer the means of ignoring merge commits.
def lint_commit(commit)
- linter = Tooling::Danger::CommitLinter.new(commit)
+ linter = Gitlab::Dangerfiles::CommitLinter.new(commit)
# For now we'll ignore merge commits, as getting rid of those is a problem
# separate from enforcing good commit messages.
@@ -93,7 +89,7 @@ end
def lint_mr_title(mr_title)
commit = Struct.new(:message, :sha).new(mr_title)
- Tooling::Danger::MergeRequestLinter.new(commit).lint
+ Gitlab::Dangerfiles::MergeRequestLinter.new(commit).lint
end
def count_non_fixup_commits(commit_linters)
@@ -109,7 +105,7 @@ def lint_commits(commits)
if multi_line_commit_linter && multi_line_commit_linter.failed?
warn_or_fail_commits(multi_line_commit_linter)
commit_linters.delete(multi_line_commit_linter) # Don't show an error (here) and a warning (below)
- elsif gitlab_danger.ci? # We don't have access to the MR title locally
+ elsif helper.ci? # We don't have access to the MR title locally
title_linter = lint_mr_title(gitlab.mr_json['title'])
if title_linter.failed?
warn_or_fail_commits(title_linter)
diff --git a/danger/database/Dangerfile b/danger/database/Dangerfile
index 67a9b53fe3afa..af4d6ed513dc0 100644
--- a/danger/database/Dangerfile
+++ b/danger/database/Dangerfile
@@ -1,7 +1,5 @@
# frozen_string_literal: true
-gitlab_danger = GitlabDanger.new(helper.gitlab_helper)
-
SCHEMA_NOT_UPDATED_MESSAGE_SHORT = "New %<migrations>s added but %<schema>s wasn't updated"
SCHEMA_NOT_UPDATED_MESSAGE_FULL = <<~MSG
@@ -35,20 +33,20 @@ geo_db_schema_updated = !git.modified_files.grep(%r{\Aee/db/geo/schema\.rb}).emp
non_geo_migration_created = !git.added_files.grep(%r{\A(db/(post_)?migrate)/}).empty?
geo_migration_created = !git.added_files.grep(%r{\Aee/db/geo/(post_)?migrate/}).empty?
-format_str = gitlab_danger.ci? ? SCHEMA_NOT_UPDATED_MESSAGE_FULL : SCHEMA_NOT_UPDATED_MESSAGE_SHORT
+format_str = helper.ci? ? SCHEMA_NOT_UPDATED_MESSAGE_FULL : SCHEMA_NOT_UPDATED_MESSAGE_SHORT
if non_geo_migration_created && !non_geo_db_schema_updated
- warn format(format_str, migrations: 'migrations', schema: gitlab_danger.html_link("db/structure.sql"))
+ warn format(format_str, migrations: 'migrations', schema: helper.html_link("db/structure.sql"))
end
if geo_migration_created && !geo_db_schema_updated
- warn format(format_str, migrations: 'Geo migrations', schema: gitlab_danger.html_link("ee/db/geo/schema.rb"))
+ warn format(format_str, migrations: 'Geo migrations', schema: helper.html_link("ee/db/geo/schema.rb"))
end
-return unless gitlab_danger.ci?
+return unless helper.ci?
return if gitlab.mr_labels.include?(DATABASE_APPROVED_LABEL)
-db_paths_to_review = helper.changes_by_category[:database]
+db_paths_to_review = project_helper.changes_by_category[:database]
if gitlab.mr_labels.include?('database') || db_paths_to_review.any?
message 'This merge request adds or changes files that require a ' \
diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile
index df7d0337e94f5..e6fe9fe1c0b7e 100644
--- a/danger/documentation/Dangerfile
+++ b/danger/documentation/Dangerfile
@@ -1,13 +1,7 @@
# frozen_string_literal: true
-def gitlab_danger
- @gitlab_danger ||= GitlabDanger.new(helper.gitlab_helper)
-end
-
def feature_mr?
- return false unless helper.gitlab_helper&.mr_labels
-
- (helper.gitlab_helper.mr_labels & %w[feature::addition feature::enhancement]).any?
+ (helper.mr_labels & %w[feature::addition feature::enhancement]).any?
end
DOCUMENTATION_UPDATE_MISSING = <<~MSG
@@ -19,7 +13,7 @@ For more information, see:
- The [definition of done](https://docs.gitlab.com/ee/development/contributing/merge_request_workflow.html#definition-of-done) documentation.
MSG
-docs_paths_to_review = helper.changes_by_category[:docs]
+docs_paths_to_review = project_helper.changes_by_category[:docs]
# Documentation should be updated for feature::addition and feature::enhancement
if docs_paths_to_review.empty?
@@ -30,7 +24,7 @@ end
message 'This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is **recommended**. Reviews can happen after you merge.'
-return unless gitlab_danger.ci?
+return unless helper.ci?
markdown(<<~MARKDOWN)
## Documentation review
diff --git a/danger/duplicate_yarn_dependencies/Dangerfile b/danger/duplicate_yarn_dependencies/Dangerfile
index 492b888d00e55..7d4294ce95987 100644
--- a/danger/duplicate_yarn_dependencies/Dangerfile
+++ b/danger/duplicate_yarn_dependencies/Dangerfile
@@ -11,7 +11,7 @@ return if duplicate.empty?
warn 'This merge request has introduced duplicated yarn dependencies.'
-if GitlabDanger.new(helper.gitlab_helper).ci?
+if helper.ci?
markdown(<<~MARKDOWN)
## Duplicate yarn dependencies
diff --git a/danger/eslint/Dangerfile b/danger/eslint/Dangerfile
index 92830bd770698..6564c83188ed6 100644
--- a/danger/eslint/Dangerfile
+++ b/danger/eslint/Dangerfile
@@ -13,7 +13,7 @@ return if eslint_candidates.empty?
warn 'This merge request changed files with disabled eslint rules. Please consider fixing them.'
-if GitlabDanger.new(helper.gitlab_helper).ci?
+if helper.ci?
markdown(<<~MARKDOWN)
## Disabled eslint rules
diff --git a/danger/karma/Dangerfile b/danger/karma/Dangerfile
index cededff5f159a..e05bd86313fc5 100644
--- a/danger/karma/Dangerfile
+++ b/danger/karma/Dangerfile
@@ -13,7 +13,7 @@ new_karma_files = get_karma_files(git.added_files.to_a)
unless new_karma_files.empty?
- if GitlabDanger.new(helper.gitlab_helper).ci?
+ if helper.ci?
markdown(<<~MARKDOWN)
## New karma spec file
@@ -36,7 +36,7 @@ return if changed_karma_files.empty?
warn 'You have edited karma spec files. Please consider migrating them to jest.'
-if GitlabDanger.new(helper.gitlab_helper).ci?
+if helper.ci?
markdown(<<~MARKDOWN)
## Edited karma files
diff --git a/danger/plugins/changelog.rb b/danger/plugins/changelog.rb
index fd2dad5932a0c..02ff405c410a9 100644
--- a/danger/plugins/changelog.rb
+++ b/danger/plugins/changelog.rb
@@ -3,7 +3,7 @@
require_relative '../../tooling/danger/changelog'
module Danger
- class Changelog < Plugin
+ class Changelog < ::Danger::Plugin
# Put the helper code somewhere it can be tested
include Tooling::Danger::Changelog
end
diff --git a/danger/plugins/helper.rb b/danger/plugins/project_helper.rb
similarity index 54%
rename from danger/plugins/helper.rb
rename to danger/plugins/project_helper.rb
index 8602868d81783..07a9de0894a0c 100644
--- a/danger/plugins/helper.rb
+++ b/danger/plugins/project_helper.rb
@@ -1,12 +1,12 @@
# frozen_string_literal: true
-require_relative '../../tooling/danger/helper'
+require_relative '../../tooling/danger/project_helper'
module Danger
- # Common helper functions for our danger scripts. See Tooling::Danger::Helper
+ # Common helper functions for our danger scripts. See Tooling::Danger::ProjectHelper
# for more details
- class Helper < Plugin
+ class ProjectHelper < ::Danger::Plugin
# Put the helper code somewhere it can be tested
- include Tooling::Danger::Helper
+ include Tooling::Danger::ProjectHelper
end
end
diff --git a/danger/plugins/roulette.rb b/danger/plugins/roulette.rb
deleted file mode 100644
index 2aa0132852e35..0000000000000
--- a/danger/plugins/roulette.rb
+++ /dev/null
@@ -1,10 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../../tooling/danger/roulette'
-
-module Danger
- class Roulette < Plugin
- # Put the helper code somewhere it can be tested
- include Tooling::Danger::Roulette
- end
-end
diff --git a/danger/plugins/sidekiq_queues.rb b/danger/plugins/sidekiq_queues.rb
index dd436e5cb2b53..d8d55f04e9921 100644
--- a/danger/plugins/sidekiq_queues.rb
+++ b/danger/plugins/sidekiq_queues.rb
@@ -3,7 +3,7 @@
require_relative '../../tooling/danger/sidekiq_queues'
module Danger
- class SidekiqQueues < Plugin
+ class SidekiqQueues < ::Danger::Plugin
# Put the helper code somewhere it can be tested
include Tooling::Danger::SidekiqQueues
end
diff --git a/danger/prettier/Dangerfile b/danger/prettier/Dangerfile
index ee27aaf9228eb..29637986f1d90 100644
--- a/danger/prettier/Dangerfile
+++ b/danger/prettier/Dangerfile
@@ -19,7 +19,7 @@ return if unpretty.empty?
warn 'This merge request changed frontend files without pretty printing them.'
-if GitlabDanger.new(helper.gitlab_helper).ci?
+if helper.ci?
markdown(<<~MARKDOWN)
## Pretty print Frontend files
diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile
index 3096ef471dd9a..b46220d7886ee 100644
--- a/danger/roulette/Dangerfile
+++ b/danger/roulette/Dangerfile
@@ -35,7 +35,7 @@ UNKNOWN_FILES_MESSAGE = <<MARKDOWN
These files couldn't be categorised, so Danger was unable to suggest a reviewer.
Please consider creating a merge request to
-[add support](https://gitlab.com/gitlab-org/gitlab/blob/master/tooling/danger/helper.rb)
+[add support](https://gitlab.com/gitlab-org/gitlab/blob/master/tooling/danger/project_helper.rb)
for them.
MARKDOWN
@@ -67,7 +67,7 @@ def markdown_row_for_spins(category, spins_array)
"| #{helper.label_for_category(category)} | #{reviewer_note} | #{maintainer_note} |"
end
-changes = helper.changes_by_category
+changes = project_helper.changes_by_category
# Ignore any files that are known but uncategorized. Prompt for any unknown files
changes.delete(:none)
@@ -76,10 +76,10 @@ changes.delete(:docs)
categories = changes.keys - [:unknown]
# Ensure to spin for database reviewer/maintainer when ~database is applied (e.g. to review SQL queries)
-categories << :database if helper.gitlab_helper&.mr_labels&.include?('database') && !categories.include?(:database)
+categories << :database if helper.mr_labels.include?('database') && !categories.include?(:database)
if changes.any?
- project = helper.project_name
+ project = project_helper.project_name
random_roulette_spins = roulette.spin(project, categories, timezone_experiment: false)
diff --git a/danger/specialization_labels/Dangerfile b/danger/specialization_labels/Dangerfile
index ac93eb4c3e1f6..2261fe23e4eb6 100644
--- a/danger/specialization_labels/Dangerfile
+++ b/danger/specialization_labels/Dangerfile
@@ -1,8 +1,6 @@
# frozen_string_literal: true
-gitlab_danger = GitlabDanger.new(helper.gitlab_helper)
-
-return unless gitlab_danger.ci?
+return unless helper.ci?
SPECIALIZATIONS = {
database: 'database',
@@ -14,7 +12,7 @@ SPECIALIZATIONS = {
ci_template: 'ci::templates'
}.freeze
-labels_to_add = helper.changes_by_category.each_with_object([]) do |(category, _changes), memo|
+labels_to_add = project_helper.changes_by_category.each_with_object([]) do |(category, _changes), memo|
label = SPECIALIZATIONS[category]
memo << label if label && !gitlab.mr_labels.include?(label)
diff --git a/lib/tasks/gitlab_danger.rake b/lib/tasks/gitlab_danger.rake
index 5df4a8ce4f11f..ff9464a588a3c 100644
--- a/lib/tasks/gitlab_danger.rake
+++ b/lib/tasks/gitlab_danger.rake
@@ -2,16 +2,16 @@
desc 'Run local Danger rules'
task :danger_local do
- require_relative '../../tooling/gitlab_danger'
+ require_relative '../../tooling/danger/project_helper'
require 'gitlab/popen'
- puts("#{GitlabDanger.local_warning_message}\n")
+ 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(GitlabDanger.success_message)
+ puts(Tooling::Danger::ProjectHelper.success_message)
else
puts(output)
exit(1)
diff --git a/spec/tooling/danger/base_linter_spec.rb b/spec/tooling/danger/base_linter_spec.rb
deleted file mode 100644
index 54d8f3dc1f729..0000000000000
--- a/spec/tooling/danger/base_linter_spec.rb
+++ /dev/null
@@ -1,192 +0,0 @@
-# frozen_string_literal: true
-
-require 'rspec-parameterized'
-require_relative 'danger_spec_helper'
-
-require_relative '../../../tooling/danger/base_linter'
-
-RSpec.describe Tooling::Danger::BaseLinter do
- let(:commit_class) do
- Struct.new(:message, :sha, :diff_parent)
- end
-
- let(:commit_message) { 'A commit message' }
- let(:commit) { commit_class.new(commit_message, anything, anything) }
-
- subject(:commit_linter) { described_class.new(commit) }
-
- describe '#failed?' do
- context 'with no failures' do
- it { expect(commit_linter).not_to be_failed }
- end
-
- context 'with failures' do
- before do
- commit_linter.add_problem(:subject_too_long, described_class.subject_description)
- end
-
- it { expect(commit_linter).to be_failed }
- end
- end
-
- describe '#add_problem' do
- it 'stores messages in #failures' do
- commit_linter.add_problem(:subject_too_long, '%s')
-
- expect(commit_linter.problems).to eq({ subject_too_long: described_class.problems_mapping[:subject_too_long] })
- end
- end
-
- shared_examples 'a valid commit' do
- it 'does not have any problem' do
- commit_linter.lint_subject
-
- expect(commit_linter.problems).to be_empty
- end
- end
-
- describe '#lint_subject' do
- context 'when subject valid' do
- it_behaves_like 'a valid commit'
- end
-
- context 'when subject is too short' do
- let(:commit_message) { 'A B' }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:subject_too_short, described_class.subject_description)
-
- commit_linter.lint_subject
- end
- end
-
- context 'when subject is too long' do
- let(:commit_message) { 'A B ' + 'C' * described_class::MAX_LINE_LENGTH }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:subject_too_long, described_class.subject_description)
-
- commit_linter.lint_subject
- end
- end
-
- context 'when ignoring length issues for subject having not-ready wording' do
- using RSpec::Parameterized::TableSyntax
-
- let(:final_message) { 'A B C' }
-
- context 'when used as prefix' do
- where(prefix: [
- 'WIP: ',
- 'WIP:',
- 'wIp:',
- '[WIP] ',
- '[WIP]',
- '[draft]',
- '[draft] ',
- '(draft)',
- '(draft) ',
- 'draft - ',
- 'draft: ',
- 'draft:',
- 'DRAFT:'
- ])
-
- with_them do
- it 'does not have any problems' do
- commit_message = prefix + final_message + 'D' * (described_class::MAX_LINE_LENGTH - final_message.size)
- commit = commit_class.new(commit_message, anything, anything)
-
- linter = described_class.new(commit).lint_subject
-
- expect(linter.problems).to be_empty
- end
- end
- end
-
- context 'when used as suffix' do
- where(suffix: %w[WIP draft])
-
- with_them do
- it 'does not have any problems' do
- commit_message = final_message + 'D' * (described_class::MAX_LINE_LENGTH - final_message.size) + suffix
- commit = commit_class.new(commit_message, anything, anything)
-
- linter = described_class.new(commit).lint_subject
-
- expect(linter.problems).to be_empty
- end
- end
- end
- end
-
- context 'when subject does not have enough words and is too long' do
- let(:commit_message) { 'A ' + 'B' * described_class::MAX_LINE_LENGTH }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:subject_too_short, described_class.subject_description)
- expect(commit_linter).to receive(:add_problem).with(:subject_too_long, described_class.subject_description)
-
- commit_linter.lint_subject
- end
- end
-
- context 'when subject starts with lowercase' do
- let(:commit_message) { 'a B C' }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:subject_starts_with_lowercase, described_class.subject_description)
-
- commit_linter.lint_subject
- end
- end
-
- [
- '[ci skip] A commit message',
- '[Ci skip] A commit message',
- '[API] A commit message',
- 'api: A commit message',
- 'API: A commit message',
- 'API: a commit message',
- 'API: a commit message'
- ].each do |message|
- context "when subject is '#{message}'" do
- let(:commit_message) { message }
-
- it 'does not add a problem' do
- expect(commit_linter).not_to receive(:add_problem)
-
- commit_linter.lint_subject
- end
- end
- end
-
- [
- '[ci skip]A commit message',
- '[Ci skip] A commit message',
- '[ci skip] a commit message',
- 'api: a commit message',
- '! A commit message'
- ].each do |message|
- context "when subject is '#{message}'" do
- let(:commit_message) { message }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:subject_starts_with_lowercase, described_class.subject_description)
-
- commit_linter.lint_subject
- end
- end
- end
-
- context 'when subject ends with a period' do
- let(:commit_message) { 'A B C.' }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:subject_ends_with_a_period, described_class.subject_description)
-
- commit_linter.lint_subject
- end
- end
- end
-end
diff --git a/spec/tooling/danger/changelog_spec.rb b/spec/tooling/danger/changelog_spec.rb
index 8d056b8a78e6e..b74039b3cd156 100644
--- a/spec/tooling/danger/changelog_spec.rb
+++ b/spec/tooling/danger/changelog_spec.rb
@@ -1,26 +1,23 @@
# frozen_string_literal: true
-require_relative 'danger_spec_helper'
+require 'gitlab-dangerfiles'
+require 'gitlab/dangerfiles/spec_helper'
-require_relative '../../../tooling/danger/helper'
require_relative '../../../tooling/danger/changelog'
+require_relative '../../../tooling/danger/project_helper'
RSpec.describe Tooling::Danger::Changelog do
- include DangerSpecHelper
+ include_context "with dangerfile"
- let(:change_class) { Tooling::Danger::Helper::Change }
- let(:changes_class) { Tooling::Danger::Helper::Changes }
- let(:changes) { changes_class.new([]) }
-
- let(:mr_labels) { [] }
- let(:sanitize_mr_title) { 'Fake Title' }
-
- let(:fake_helper) { double('fake-helper', changes: changes, mr_iid: 1234, mr_title: sanitize_mr_title, mr_labels: mr_labels) }
-
- let(:fake_danger) { new_fake_danger.include(described_class) }
+ let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
+ let(:fake_project_helper) { double('fake-project-helper', helper: fake_helper).tap { |h| h.class.include(Tooling::Danger::ProjectHelper) } }
subject(:changelog) { fake_danger.new(helper: fake_helper) }
+ before do
+ allow(changelog).to receive(:project_helper).and_return(fake_project_helper)
+ end
+
describe '#required_reasons' do
subject { changelog.required_reasons }
@@ -165,7 +162,7 @@
subject { changelog.modified_text }
context "when title is not changed from sanitization", :aggregate_failures do
- let(:sanitize_mr_title) { 'Fake Title' }
+ let(:mr_title) { 'Fake Title' }
specify do
expect(subject).to include('CHANGELOG.md was edited')
@@ -175,7 +172,7 @@
end
context "when title needs sanitization", :aggregate_failures do
- let(:sanitize_mr_title) { 'DRAFT: Fake Title' }
+ let(:mr_title) { 'DRAFT: Fake Title' }
specify do
expect(subject).to include('CHANGELOG.md was edited')
@@ -186,7 +183,7 @@
end
describe '#required_texts' do
- let(:sanitize_mr_title) { 'Fake Title' }
+ let(:mr_title) { 'Fake Title' }
subject { changelog.required_texts }
@@ -207,7 +204,7 @@
end
context "when title needs sanitization", :aggregate_failures do
- let(:sanitize_mr_title) { 'DRAFT: Fake Title' }
+ let(:mr_title) { 'DRAFT: Fake Title' }
it_behaves_like 'changelog required text', :db_changes
end
@@ -224,7 +221,7 @@
subject { changelog.optional_text }
context "when title is not changed from sanitization", :aggregate_failures do
- let(:sanitize_mr_title) { 'Fake Title' }
+ let(:mr_title) { 'Fake Title' }
specify do
expect(subject).to include('CHANGELOG missing')
@@ -234,7 +231,7 @@
end
context "when title needs sanitization", :aggregate_failures do
- let(:sanitize_mr_title) { 'DRAFT: Fake Title' }
+ let(:mr_title) { 'DRAFT: Fake Title' }
specify do
expect(subject).to include('CHANGELOG missing')
diff --git a/spec/tooling/danger/commit_linter_spec.rb b/spec/tooling/danger/commit_linter_spec.rb
deleted file mode 100644
index 694e524af21c9..0000000000000
--- a/spec/tooling/danger/commit_linter_spec.rb
+++ /dev/null
@@ -1,241 +0,0 @@
-# frozen_string_literal: true
-
-require 'rspec-parameterized'
-require_relative 'danger_spec_helper'
-
-require_relative '../../../tooling/danger/commit_linter'
-
-RSpec.describe Tooling::Danger::CommitLinter do
- using RSpec::Parameterized::TableSyntax
-
- let(:total_files_changed) { 2 }
- let(:total_lines_changed) { 10 }
- let(:stats) { { total: { files: total_files_changed, lines: total_lines_changed } } }
- let(:diff_parent) { Struct.new(:stats).new(stats) }
- let(:commit_class) do
- Struct.new(:message, :sha, :diff_parent)
- end
-
- let(:commit_message) { 'A commit message' }
- let(:commit_sha) { 'abcd1234' }
- let(:commit) { commit_class.new(commit_message, commit_sha, diff_parent) }
-
- subject(:commit_linter) { described_class.new(commit) }
-
- describe '#fixup?' do
- where(:commit_message, :is_fixup) do
- 'A commit message' | false
- 'fixup!' | true
- 'fixup! A commit message' | true
- 'squash!' | true
- 'squash! A commit message' | true
- end
-
- with_them do
- it 'is true when commit message starts with "fixup!" or "squash!"' do
- expect(commit_linter.fixup?).to be(is_fixup)
- end
- end
- end
-
- describe '#suggestion?' do
- where(:commit_message, :is_suggestion) do
- 'A commit message' | false
- 'Apply suggestion to' | true
- 'Apply suggestion to "A commit message"' | true
- end
-
- with_them do
- it 'is true when commit message starts with "Apply suggestion to"' do
- expect(commit_linter.suggestion?).to be(is_suggestion)
- end
- end
- end
-
- describe '#merge?' do
- where(:commit_message, :is_merge) do
- 'A commit message' | false
- 'Merge branch' | true
- 'Merge branch "A commit message"' | true
- end
-
- with_them do
- it 'is true when commit message starts with "Merge branch"' do
- expect(commit_linter.merge?).to be(is_merge)
- end
- end
- end
-
- describe '#revert?' do
- where(:commit_message, :is_revert) do
- 'A commit message' | false
- 'Revert' | false
- 'Revert "' | true
- 'Revert "A commit message"' | true
- end
-
- with_them do
- it 'is true when commit message starts with "Revert \""' do
- expect(commit_linter.revert?).to be(is_revert)
- end
- end
- end
-
- describe '#multi_line?' do
- where(:commit_message, :is_multi_line) do
- "A commit message" | false
- "A commit message\n" | false
- "A commit message\n\n" | false
- "A commit message\n\nSigned-off-by: User Name <user@name.me>" | false
- "A commit message\n\nWith details" | true
- end
-
- with_them do
- it 'is true when commit message contains details' do
- expect(commit_linter.multi_line?).to be(is_multi_line)
- end
- end
- end
-
- shared_examples 'a valid commit' do
- it 'does not have any problem' do
- commit_linter.lint
-
- expect(commit_linter.problems).to be_empty
- end
- end
-
- describe '#lint' do
- describe 'separator' do
- context 'when separator is missing' do
- let(:commit_message) { "A B C\n" }
-
- it_behaves_like 'a valid commit'
- end
-
- context 'when separator is a blank line' do
- let(:commit_message) { "A B C\n\nMore details." }
-
- it_behaves_like 'a valid commit'
- end
-
- context 'when separator is missing' do
- let(:commit_message) { "A B C\nMore details." }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:separator_missing)
-
- commit_linter.lint
- end
- end
- end
-
- describe 'details' do
- context 'when details are valid' do
- let(:commit_message) { "A B C\n\nMore details." }
-
- it_behaves_like 'a valid commit'
- end
-
- context 'when no details are given and many files are changed' do
- let(:total_files_changed) { described_class::MAX_CHANGED_FILES_IN_COMMIT + 1 }
-
- it_behaves_like 'a valid commit'
- end
-
- context 'when no details are given and many lines are changed' do
- let(:total_lines_changed) { described_class::MAX_CHANGED_LINES_IN_COMMIT + 1 }
-
- it_behaves_like 'a valid commit'
- end
-
- context 'when no details are given and many files and lines are changed' do
- let(:total_files_changed) { described_class::MAX_CHANGED_FILES_IN_COMMIT + 1 }
- let(:total_lines_changed) { described_class::MAX_CHANGED_LINES_IN_COMMIT + 1 }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:details_too_many_changes)
-
- commit_linter.lint
- end
- end
-
- context 'when details exceeds the max line length' do
- let(:commit_message) { "A B C\n\n" + 'D' * (described_class::MAX_LINE_LENGTH + 1) }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:details_line_too_long)
-
- commit_linter.lint
- end
- end
-
- context 'when details exceeds the max line length including URLs' do
- let(:commit_message) do
- "A B C\n\nsome message with https://example.com and https://gitlab.com" + 'D' * described_class::MAX_LINE_LENGTH
- end
-
- it_behaves_like 'a valid commit'
- end
- end
-
- describe 'message' do
- context 'when message includes a text emoji' do
- let(:commit_message) { "A commit message :+1:" }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:message_contains_text_emoji)
-
- commit_linter.lint
- end
- end
-
- context 'when message includes a unicode emoji' do
- let(:commit_message) { "A commit message 🚀" }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:message_contains_unicode_emoji)
-
- commit_linter.lint
- end
- end
-
- context 'when message includes a value that is surrounded by backticks' do
- let(:commit_message) { "A commit message `%20`" }
-
- it 'does not add a problem' do
- expect(commit_linter).not_to receive(:add_problem)
-
- commit_linter.lint
- end
- end
-
- context 'when message includes a short reference' do
- [
- 'A commit message to fix #1234',
- 'A commit message to fix !1234',
- 'A commit message to fix &1234',
- 'A commit message to fix %1234',
- 'A commit message to fix gitlab#1234',
- 'A commit message to fix gitlab!1234',
- 'A commit message to fix gitlab&1234',
- 'A commit message to fix gitlab%1234',
- 'A commit message to fix gitlab-org/gitlab#1234',
- 'A commit message to fix gitlab-org/gitlab!1234',
- 'A commit message to fix gitlab-org/gitlab&1234',
- 'A commit message to fix gitlab-org/gitlab%1234',
- 'A commit message to fix "gitlab-org/gitlab%1234"',
- 'A commit message to fix `gitlab-org/gitlab%1234'
- ].each do |message|
- let(:commit_message) { message }
-
- it 'adds a problem' do
- expect(commit_linter).to receive(:add_problem).with(:message_contains_short_reference)
-
- commit_linter.lint
- end
- end
- end
- end
- end
-end
diff --git a/spec/tooling/danger/danger_spec_helper.rb b/spec/tooling/danger/danger_spec_helper.rb
deleted file mode 100644
index b1e84b3c13de8..0000000000000
--- a/spec/tooling/danger/danger_spec_helper.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-# frozen_string_literal: true
-
-module DangerSpecHelper
- def new_fake_danger
- Class.new do
- attr_reader :git, :gitlab, :helper
-
- # rubocop:disable Gitlab/ModuleWithInstanceVariables
- def initialize(git: nil, gitlab: nil, helper: nil)
- @git = git
- @gitlab = gitlab
- @helper = helper
- end
- # rubocop:enable Gitlab/ModuleWithInstanceVariables
- end
- end
-end
diff --git a/spec/tooling/danger/emoji_checker_spec.rb b/spec/tooling/danger/emoji_checker_spec.rb
deleted file mode 100644
index bbd957b3d00d7..0000000000000
--- a/spec/tooling/danger/emoji_checker_spec.rb
+++ /dev/null
@@ -1,37 +0,0 @@
-# frozen_string_literal: true
-
-require 'rspec-parameterized'
-
-require_relative '../../../tooling/danger/emoji_checker'
-
-RSpec.describe Tooling::Danger::EmojiChecker do
- using RSpec::Parameterized::TableSyntax
-
- describe '#includes_text_emoji?' do
- where(:text, :includes_emoji) do
- 'Hello World!' | false
- ':+1:' | true
- 'Hello World! :+1:' | true
- end
-
- with_them do
- it 'is true when text includes a text emoji' do
- expect(subject.includes_text_emoji?(text)).to be(includes_emoji)
- end
- end
- end
-
- describe '#includes_unicode_emoji?' do
- where(:text, :includes_emoji) do
- 'Hello World!' | false
- '🚀' | true
- 'Hello World! 🚀' | true
- end
-
- with_them do
- it 'is true when text includes a text emoji' do
- expect(subject.includes_unicode_emoji?(text)).to be(includes_emoji)
- end
- end
- end
-end
diff --git a/spec/tooling/danger/feature_flag_spec.rb b/spec/tooling/danger/feature_flag_spec.rb
index db63116cc3795..5e495cd43c687 100644
--- a/spec/tooling/danger/feature_flag_spec.rb
+++ b/spec/tooling/danger/feature_flag_spec.rb
@@ -1,29 +1,16 @@
# frozen_string_literal: true
-require_relative 'danger_spec_helper'
+require 'gitlab-dangerfiles'
+require 'gitlab/dangerfiles/spec_helper'
require_relative '../../../tooling/danger/feature_flag'
RSpec.describe Tooling::Danger::FeatureFlag do
- include DangerSpecHelper
+ include_context "with dangerfile"
- let(:added_files) { nil }
- let(:modified_files) { nil }
- let(:deleted_files) { nil }
- let(:fake_git) { double('fake-git', added_files: added_files, modified_files: modified_files, deleted_files: deleted_files) }
+ let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
- let(:mr_labels) { nil }
- let(:mr_json) { nil }
- let(:fake_gitlab) { double('fake-gitlab', mr_labels: mr_labels, mr_json: mr_json) }
-
- let(:changes_by_category) { nil }
- let(:sanitize_mr_title) { nil }
- let(:ee?) { false }
- let(:fake_helper) { double('fake-helper', changes_by_category: changes_by_category, sanitize_mr_title: sanitize_mr_title, ee?: ee?) }
-
- let(:fake_danger) { new_fake_danger.include(described_class) }
-
- subject(:feature_flag) { fake_danger.new(git: fake_git, gitlab: fake_gitlab, helper: fake_helper) }
+ subject(:feature_flag) { fake_danger.new(git: fake_git) }
describe '#feature_flag_files' do
let(:feature_flag_files) do
diff --git a/spec/tooling/danger/helper_spec.rb b/spec/tooling/danger/helper_spec.rb
deleted file mode 100644
index 9783705dca048..0000000000000
--- a/spec/tooling/danger/helper_spec.rb
+++ /dev/null
@@ -1,781 +0,0 @@
-# frozen_string_literal: true
-
-require 'fast_spec_helper'
-require 'rspec-parameterized'
-require_relative 'danger_spec_helper'
-
-require_relative '../../../tooling/danger/helper'
-
-RSpec.describe Tooling::Danger::Helper do
- using RSpec::Parameterized::TableSyntax
- include DangerSpecHelper
-
- let(:mr_author) { nil }
- let(:fake_gitlab) { double('fake-gitlab', mr_author: mr_author) }
-
- let(:fake_danger) { new_fake_danger.include(described_class) }
-
- let(:added_files) { %w[added1] }
- let(:modified_files) { %w[modified1] }
- let(:deleted_files) { %w[deleted1] }
- let(:renamed_before_file) { 'renamed_before' }
- let(:renamed_after_file) { 'renamed_after' }
- let(:renamed_files) { [{ before: renamed_before_file, after: renamed_after_file }] }
-
- let(:fake_git) { double('fake-git') }
-
- before do
- allow(fake_git).to receive(:added_files) { added_files }
- allow(fake_git).to receive(:modified_files) { modified_files }
- allow(fake_git).to receive(:deleted_files) { deleted_files }
- allow(fake_git).to receive(:renamed_files).at_least(:twice) { renamed_files }
- end
-
- subject(:helper) { fake_danger.new(git: fake_git, gitlab: fake_gitlab) }
-
- describe '#gitlab_helper' do
- context 'when gitlab helper is not available' do
- let(:fake_gitlab) { nil }
-
- it 'returns nil' do
- expect(helper.gitlab_helper).to be_nil
- end
- end
-
- context 'when gitlab helper is available' do
- it 'returns the gitlab helper' do
- expect(helper.gitlab_helper).to eq(fake_gitlab)
- end
- end
-
- context 'when danger gitlab plugin is not available' do
- it 'returns nil' do
- invalid_danger = Class.new do
- include Tooling::Danger::Helper
- end.new
-
- expect(invalid_danger.gitlab_helper).to be_nil
- end
- end
- end
-
- describe '#release_automation?' do
- context 'when gitlab helper is not available' do
- it 'returns false' do
- expect(helper.release_automation?).to be_falsey
- end
- end
-
- context 'when gitlab helper is available' do
- context "but the MR author isn't the RELEASE_TOOLS_BOT" do
- let(:mr_author) { 'johnmarston' }
-
- it 'returns false' do
- expect(helper.release_automation?).to be_falsey
- end
- end
-
- context 'and the MR author is the RELEASE_TOOLS_BOT' do
- let(:mr_author) { described_class::RELEASE_TOOLS_BOT }
-
- it 'returns true' do
- expect(helper.release_automation?).to be_truthy
- end
- end
- end
- end
-
- describe '#all_changed_files' do
- subject { helper.all_changed_files }
-
- it 'interprets a list of changes from the danger git plugin' do
- expect(fake_git).to receive(:added_files) { %w[a b c.old] }
- expect(fake_git).to receive(:modified_files) { %w[d e] }
- expect(fake_git)
- .to receive(:renamed_files)
- .at_least(:once)
- .and_return([{ before: 'c.old', after: 'c.new' }])
-
- is_expected.to contain_exactly('a', 'b', 'c.new', 'd', 'e')
- end
- end
-
- describe '#changed_lines' do
- subject { helper.changed_lines('changed_file.rb') }
-
- before do
- allow(fake_git).to receive(:diff_for_file).with('changed_file.rb').and_return(diff)
- end
-
- context 'when file has diff' do
- let(:diff) { double(:diff, patch: "+ # New change here\n+ # New change there") }
-
- it 'returns file changes' do
- is_expected.to eq(['+ # New change here', '+ # New change there'])
- end
- end
-
- context 'when file has no diff (renamed without changes)' do
- let(:diff) { nil }
-
- it 'returns a blank array' do
- is_expected.to eq([])
- end
- end
- end
-
- describe "changed_files" do
- it 'returns list of changed files matching given regex' do
- expect(helper).to receive(:all_changed_files).and_return(%w[migration.rb usage_data.rb])
-
- expect(helper.changed_files(/usage_data/)).to contain_exactly('usage_data.rb')
- end
- end
-
- describe '#all_ee_changes' do
- subject { helper.all_ee_changes }
-
- it 'returns all changed files starting with ee/' do
- expect(helper).to receive(:all_changed_files).and_return(%w[fr/ee/beer.rb ee/wine.rb ee/lib/ido.rb ee.k])
-
- is_expected.to match_array(%w[ee/wine.rb ee/lib/ido.rb])
- end
- end
-
- describe '#ee?' do
- subject { helper.ee? }
-
- it 'returns true if CI_PROJECT_NAME if set to gitlab' do
- stub_env('CI_PROJECT_NAME', 'gitlab')
- expect(Dir).not_to receive(:exist?)
-
- is_expected.to be_truthy
- end
-
- it 'delegates to CHANGELOG-EE.md existence if CI_PROJECT_NAME is set to something else' do
- stub_env('CI_PROJECT_NAME', 'something else')
- expect(Dir).to receive(:exist?).with(File.expand_path('../../../../ee', __dir__)) { true }
-
- is_expected.to be_truthy
- end
-
- it 'returns true if ee exists' do
- stub_env('CI_PROJECT_NAME', nil)
- expect(Dir).to receive(:exist?).with(File.expand_path('../../../../ee', __dir__)) { true }
-
- is_expected.to be_truthy
- end
-
- it "returns false if ee doesn't exist" do
- stub_env('CI_PROJECT_NAME', nil)
- expect(Dir).to receive(:exist?).with(File.expand_path('../../../../ee', __dir__)) { false }
-
- is_expected.to be_falsy
- end
- end
-
- describe '#project_name' do
- subject { helper.project_name }
-
- it 'returns gitlab if ee? returns true' do
- expect(helper).to receive(:ee?) { true }
-
- is_expected.to eq('gitlab')
- end
-
- it 'returns gitlab-ce if ee? returns false' do
- expect(helper).to receive(:ee?) { false }
-
- is_expected.to eq('gitlab-foss')
- end
- end
-
- describe '#markdown_list' do
- it 'creates a markdown list of items' do
- items = %w[a b]
-
- expect(helper.markdown_list(items)).to eq("* `a`\n* `b`")
- end
-
- it 'wraps items in <details> when there are more than 10 items' do
- items = ('a'..'k').to_a
-
- expect(helper.markdown_list(items)).to match(%r{<details>[^<]+</details>})
- end
- end
-
- describe '#changes_by_category' do
- let(:added_files) { %w[foo foo.md foo.rb foo.js] }
- let(:modified_files) { %w[db/migrate/foo lib/gitlab/database/foo.rb] }
- let(:renamed_files) { [{ before: '', after: 'qa/foo' }, { before: '', after: 'ee/changelogs/foo.yml' }] }
-
- it 'categorizes changed files' do
- expect(helper.changes_by_category).to eq(
- backend: %w[foo.rb],
- database: %w[db/migrate/foo lib/gitlab/database/foo.rb],
- frontend: %w[foo.js],
- migration: %w[db/migrate/foo],
- none: %w[ee/changelogs/foo.yml foo.md],
- qa: %w[qa/foo],
- unknown: %w[foo]
- )
- end
- end
-
- describe 'Tooling::Danger::Helper::Changes', :aggregate_failures do
- let(:added_files) { %w[db/migrate/foo ee/changelogs/unreleased/foo.yml] }
-
- describe '#has_category?' do
- it 'returns true when changes include given category, false otherwise' do
- changes = helper.changes
-
- expect(changes.has_category?(:migration)).to eq(true)
- expect(changes.has_category?(:changelog)).to eq(true)
- expect(changes.has_category?(:backend)).to eq(false)
- end
- end
-
- describe '#by_category' do
- it 'returns an array of Change objects' do
- expect(helper.changes.by_category(:migration)).to all(be_an(described_class::Change))
- end
-
- it 'returns an array of Change objects with the given category' do
- changes = helper.changes
-
- expect(changes.by_category(:migration).files).to eq(['db/migrate/foo'])
- expect(changes.by_category(:changelog).files).to eq(['ee/changelogs/unreleased/foo.yml'])
- expect(changes.by_category(:backend)).to be_empty
- end
- end
-
- describe '#categories' do
- it 'returns an array of category symbols' do
- expect(helper.changes.categories).to contain_exactly(:database, :migration, :changelog, :unknown)
- end
- end
-
- describe '#files' do
- it 'returns an array of files' do
- expect(helper.changes.files).to include(*added_files)
- end
- end
- end
-
- describe '#changes' do
- it 'returns an array of Change objects' do
- expect(helper.changes).to all(be_an(described_class::Change))
- end
-
- it 'groups changes by change type' do
- changes = helper.changes
-
- expect(changes.added.files).to eq(added_files)
- expect(changes.modified.files).to eq(modified_files)
- expect(changes.deleted.files).to eq(deleted_files)
- expect(changes.renamed_before.files).to eq([renamed_before_file])
- expect(changes.renamed_after.files).to eq([renamed_after_file])
- end
- end
-
- describe '#categories_for_file' do
- before do
- allow(fake_git).to receive(:diff_for_file).with('usage_data.rb') { double(:diff, patch: "+ count(User.active)") }
- end
-
- where(:path, :expected_categories) do
- 'usage_data.rb' | [:database, :backend]
- 'doc/foo.md' | [:docs]
- 'CONTRIBUTING.md' | [:docs]
- 'LICENSE' | [:docs]
- 'MAINTENANCE.md' | [:docs]
- 'PHILOSOPHY.md' | [:docs]
- 'PROCESS.md' | [:docs]
- 'README.md' | [:docs]
-
- 'ee/doc/foo' | [:unknown]
- 'ee/README' | [:unknown]
-
- 'app/assets/foo' | [:frontend]
- 'app/views/foo' | [:frontend]
- 'public/foo' | [:frontend]
- 'scripts/frontend/foo' | [:frontend]
- 'spec/javascripts/foo' | [:frontend]
- 'spec/frontend/bar' | [:frontend]
- 'vendor/assets/foo' | [:frontend]
- 'babel.config.js' | [:frontend]
- 'jest.config.js' | [:frontend]
- 'package.json' | [:frontend]
- 'yarn.lock' | [:frontend]
- 'config/foo.js' | [:frontend]
- 'config/deep/foo.js' | [:frontend]
-
- 'ee/app/assets/foo' | [:frontend]
- 'ee/app/views/foo' | [:frontend]
- 'ee/spec/javascripts/foo' | [:frontend]
- 'ee/spec/frontend/bar' | [:frontend]
-
- '.gitlab/ci/frontend.gitlab-ci.yml' | %i[frontend engineering_productivity]
-
- 'app/models/foo' | [:backend]
- 'bin/foo' | [:backend]
- 'config/foo' | [:backend]
- 'lib/foo' | [:backend]
- 'rubocop/foo' | [:backend]
- '.rubocop.yml' | [:backend]
- '.rubocop_todo.yml' | [:backend]
- '.rubocop_manual_todo.yml' | [:backend]
- 'spec/foo' | [:backend]
- 'spec/foo/bar' | [:backend]
-
- 'ee/app/foo' | [:backend]
- 'ee/bin/foo' | [:backend]
- 'ee/spec/foo' | [:backend]
- 'ee/spec/foo/bar' | [:backend]
-
- 'spec/features/foo' | [:test]
- 'ee/spec/features/foo' | [:test]
- 'spec/support/shared_examples/features/foo' | [:test]
- 'ee/spec/support/shared_examples/features/foo' | [:test]
- 'spec/support/shared_contexts/features/foo' | [:test]
- 'ee/spec/support/shared_contexts/features/foo' | [:test]
- 'spec/support/helpers/features/foo' | [:test]
- 'ee/spec/support/helpers/features/foo' | [:test]
-
- 'generator_templates/foo' | [:backend]
- 'vendor/languages.yml' | [:backend]
- 'file_hooks/examples/' | [:backend]
-
- 'Gemfile' | [:backend]
- 'Gemfile.lock' | [:backend]
- 'Rakefile' | [:backend]
- 'FOO_VERSION' | [:backend]
-
- 'Dangerfile' | [:engineering_productivity]
- 'danger/commit_messages/Dangerfile' | [:engineering_productivity]
- 'ee/danger/commit_messages/Dangerfile' | [:engineering_productivity]
- 'danger/commit_messages/' | [:engineering_productivity]
- 'ee/danger/commit_messages/' | [:engineering_productivity]
- '.gitlab-ci.yml' | [:engineering_productivity]
- '.gitlab/ci/cng.gitlab-ci.yml' | [:engineering_productivity]
- '.gitlab/ci/ee-specific-checks.gitlab-ci.yml' | [:engineering_productivity]
- 'scripts/foo' | [:engineering_productivity]
- 'tooling/danger/foo' | [:engineering_productivity]
- 'ee/tooling/danger/foo' | [:engineering_productivity]
- 'lefthook.yml' | [:engineering_productivity]
- '.editorconfig' | [:engineering_productivity]
- 'tooling/bin/find_foss_tests' | [:engineering_productivity]
- '.codeclimate.yml' | [:engineering_productivity]
- '.gitlab/CODEOWNERS' | [:engineering_productivity]
-
- 'lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml' | [:ci_template]
- 'lib/gitlab/ci/templates/dotNET-Core.yml' | [:ci_template]
-
- 'ee/FOO_VERSION' | [:unknown]
-
- 'db/schema.rb' | [:database]
- 'db/structure.sql' | [:database]
- 'db/migrate/foo' | [:database, :migration]
- 'db/post_migrate/foo' | [:database, :migration]
- 'ee/db/geo/migrate/foo' | [:database, :migration]
- 'ee/db/geo/post_migrate/foo' | [:database, :migration]
- 'app/models/project_authorization.rb' | [:database]
- 'app/services/users/refresh_authorized_projects_service.rb' | [:database]
- 'lib/gitlab/background_migration.rb' | [:database]
- 'lib/gitlab/background_migration/foo' | [:database]
- 'ee/lib/gitlab/background_migration/foo' | [:database]
- 'lib/gitlab/database.rb' | [:database]
- 'lib/gitlab/database/foo' | [:database]
- 'ee/lib/gitlab/database/foo' | [:database]
- 'lib/gitlab/github_import.rb' | [:database]
- 'lib/gitlab/github_import/foo' | [:database]
- 'lib/gitlab/sql/foo' | [:database]
- 'rubocop/cop/migration/foo' | [:database]
-
- 'db/fixtures/foo.rb' | [:backend]
- 'ee/db/fixtures/foo.rb' | [:backend]
-
- 'qa/foo' | [:qa]
- 'ee/qa/foo' | [:qa]
-
- 'changelogs/foo' | [:none]
- 'ee/changelogs/foo' | [:none]
- 'locale/gitlab.pot' | [:none]
-
- 'FOO' | [:unknown]
- 'foo' | [:unknown]
-
- 'foo/bar.rb' | [:backend]
- 'foo/bar.js' | [:frontend]
- 'foo/bar.txt' | [:none]
- 'foo/bar.md' | [:none]
- end
-
- with_them do
- subject { helper.categories_for_file(path) }
-
- it { is_expected.to eq(expected_categories) }
- end
-
- context 'having specific changes' do
- where(:expected_categories, :patch, :changed_files) do
- [:database, :backend] | '+ count(User.active)' | ['usage_data.rb', 'lib/gitlab/usage_data.rb', 'ee/lib/ee/gitlab/usage_data.rb']
- [:database, :backend] | '+ estimate_batch_distinct_count(User.active)' | ['usage_data.rb']
- [:backend] | '+ alt_usage_data(User.active)' | ['usage_data.rb']
- [:backend] | '+ count(User.active)' | ['user.rb']
- [:backend] | '+ count(User.active)' | ['usage_data/topology.rb']
- [:backend] | '+ foo_count(User.active)' | ['usage_data.rb']
- end
-
- with_them do
- it 'has the correct categories' do
- changed_files.each do |file|
- allow(fake_git).to receive(:diff_for_file).with(file) { double(:diff, patch: patch) }
-
- expect(helper.categories_for_file(file)).to eq(expected_categories)
- end
- end
- end
- end
- end
-
- describe '#label_for_category' do
- where(:category, :expected_label) do
- :backend | '~backend'
- :database | '~database'
- :docs | '~documentation'
- :foo | '~foo'
- :frontend | '~frontend'
- :none | ''
- :qa | '~QA'
- :engineering_productivity | '~"Engineering Productivity" for CI, Danger'
- :ci_template | '~"ci::templates"'
- end
-
- with_them do
- subject { helper.label_for_category(category) }
-
- it { is_expected.to eq(expected_label) }
- end
- end
-
- describe '#new_teammates' do
- it 'returns an array of Teammate' do
- usernames = %w[filipa iamphil]
-
- teammates = helper.new_teammates(usernames)
-
- expect(teammates.map(&:username)).to eq(usernames)
- end
- end
-
- describe '#mr_iid' do
- it 'returns "" when `gitlab_helper` is unavailable' do
- expect(helper).to receive(:gitlab_helper).and_return(nil)
-
- expect(helper.mr_iid).to eq('')
- end
-
- it 'returns the MR IID when `gitlab_helper` is available' do
- mr_iid = '1234'
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('iid' => mr_iid)
-
- expect(helper.mr_iid).to eq(mr_iid)
- end
- end
-
- describe '#mr_title' do
- it 'returns "" when `gitlab_helper` is unavailable' do
- expect(helper).to receive(:gitlab_helper).and_return(nil)
-
- expect(helper.mr_title).to eq('')
- end
-
- it 'returns the MR title when `gitlab_helper` is available' do
- mr_title = 'My MR title'
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('title' => mr_title)
-
- expect(helper.mr_title).to eq(mr_title)
- end
- end
-
- describe '#mr_web_url' do
- it 'returns "" when `gitlab_helper` is unavailable' do
- expect(helper).to receive(:gitlab_helper).and_return(nil)
-
- expect(helper.mr_web_url).to eq('')
- end
-
- it 'returns the MR web_url when `gitlab_helper` is available' do
- mr_web_url = 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/1'
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('web_url' => mr_web_url)
-
- expect(helper.mr_web_url).to eq(mr_web_url)
- end
- end
-
- describe '#mr_labels' do
- it 'returns "" when `gitlab_helper` is unavailable' do
- expect(helper).to receive(:gitlab_helper).and_return(nil)
-
- expect(helper.mr_labels).to eq([])
- end
-
- it 'returns the MR labels when `gitlab_helper` is available' do
- mr_labels = %w[foo bar baz]
- expect(fake_gitlab).to receive(:mr_labels)
- .and_return(mr_labels)
-
- expect(helper.mr_labels).to eq(mr_labels)
- end
- end
-
- describe '#mr_target_branch' do
- it 'returns "" when `gitlab_helper` is unavailable' do
- expect(helper).to receive(:gitlab_helper).and_return(nil)
-
- expect(helper.mr_target_branch).to eq('')
- end
-
- it 'returns the MR web_url when `gitlab_helper` is available' do
- mr_target_branch = 'main'
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('target_branch' => mr_target_branch)
-
- expect(helper.mr_target_branch).to eq(mr_target_branch)
- end
- end
-
- describe '#security_mr?' do
- it 'returns false when on a normal merge request' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('web_url' => 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/1')
-
- expect(helper).not_to be_security_mr
- end
-
- it 'returns true when on a security merge request' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('web_url' => 'https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1')
-
- expect(helper).to be_security_mr
- end
- end
-
- describe '#draft_mr?' do
- it 'returns true for a draft MR' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('title' => 'Draft: My MR title')
-
- expect(helper).to be_draft_mr
- end
-
- it 'returns false for non draft MR' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('title' => 'My MR title')
-
- expect(helper).not_to be_draft_mr
- end
- end
-
- describe '#cherry_pick_mr?' do
- context 'when MR title does not mention a cherry-pick' do
- it 'returns false' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('title' => 'Add feature xyz')
-
- expect(helper).not_to be_cherry_pick_mr
- end
- end
-
- context 'when MR title mentions a cherry-pick' do
- [
- 'Cherry Pick !1234',
- 'cherry-pick !1234',
- 'CherryPick !1234'
- ].each do |mr_title|
- it 'returns true' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('title' => mr_title)
-
- expect(helper).to be_cherry_pick_mr
- end
- end
- end
- end
-
- describe '#run_all_rspec_mr?' do
- context 'when MR title does not mention RUN ALL RSPEC' do
- it 'returns false' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('title' => 'Add feature xyz')
-
- expect(helper).not_to be_run_all_rspec_mr
- end
- end
-
- context 'when MR title mentions RUN ALL RSPEC' do
- it 'returns true' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('title' => 'Add feature xyz RUN ALL RSPEC')
-
- expect(helper).to be_run_all_rspec_mr
- end
- end
- end
-
- describe '#run_as_if_foss_mr?' do
- context 'when MR title does not mention RUN AS-IF-FOSS' do
- it 'returns false' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('title' => 'Add feature xyz')
-
- expect(helper).not_to be_run_as_if_foss_mr
- end
- end
-
- context 'when MR title mentions RUN AS-IF-FOSS' do
- it 'returns true' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('title' => 'Add feature xyz RUN AS-IF-FOSS')
-
- expect(helper).to be_run_as_if_foss_mr
- end
- end
- end
-
- describe '#stable_branch?' do
- it 'returns false when `gitlab_helper` is unavailable' do
- expect(helper).to receive(:gitlab_helper).and_return(nil)
-
- expect(helper).not_to be_stable_branch
- end
-
- context 'when MR target branch is not a stable branch' do
- it 'returns false' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('target_branch' => 'my-feature-branch')
-
- expect(helper).not_to be_stable_branch
- end
- end
-
- context 'when MR target branch is a stable branch' do
- %w[
- 13-1-stable-ee
- 13-1-stable-ee-patch-1
- ].each do |target_branch|
- it 'returns true' do
- expect(fake_gitlab).to receive(:mr_json)
- .and_return('target_branch' => target_branch)
-
- expect(helper).to be_stable_branch
- end
- end
- end
- end
-
- describe '#mr_has_label?' do
- it 'returns false when `gitlab_helper` is unavailable' do
- expect(helper).to receive(:gitlab_helper).and_return(nil)
-
- expect(helper.mr_has_labels?('telemetry')).to be_falsey
- end
-
- context 'when mr has labels' do
- before do
- mr_labels = ['telemetry', 'telemetry::reviewed']
- expect(fake_gitlab).to receive(:mr_labels).and_return(mr_labels)
- end
-
- it 'returns true with a matched label' do
- expect(helper.mr_has_labels?('telemetry')).to be_truthy
- end
-
- it 'returns false with unmatched label' do
- expect(helper.mr_has_labels?('database')).to be_falsey
- end
-
- it 'returns true with an array of labels' do
- expect(helper.mr_has_labels?(['telemetry', 'telemetry::reviewed'])).to be_truthy
- end
-
- it 'returns true with multi arguments with matched labels' do
- expect(helper.mr_has_labels?('telemetry', 'telemetry::reviewed')).to be_truthy
- end
-
- it 'returns false with multi arguments with unmatched labels' do
- expect(helper.mr_has_labels?('telemetry', 'telemetry::non existing')).to be_falsey
- end
- end
- end
-
- describe '#labels_list' do
- let(:labels) { ['telemetry', 'telemetry::reviewed'] }
-
- it 'composes the labels string' do
- expect(helper.labels_list(labels)).to eq('~"telemetry", ~"telemetry::reviewed"')
- end
-
- context 'when passing a separator' do
- it 'composes the labels string with the given separator' do
- expect(helper.labels_list(labels, sep: ' ')).to eq('~"telemetry" ~"telemetry::reviewed"')
- end
- end
-
- it 'returns empty string for empty array' do
- expect(helper.labels_list([])).to eq('')
- end
- end
-
- describe '#prepare_labels_for_mr' do
- it 'composes the labels string' do
- mr_labels = ['telemetry', 'telemetry::reviewed']
-
- expect(helper.prepare_labels_for_mr(mr_labels)).to eq('/label ~"telemetry" ~"telemetry::reviewed"')
- end
-
- it 'returns empty string for empty array' do
- expect(helper.prepare_labels_for_mr([])).to eq('')
- end
- end
-
- describe '#has_ci_changes?' do
- context 'when .gitlab/ci is changed' do
- it 'returns true' do
- expect(helper).to receive(:all_changed_files).and_return(%w[migration.rb .gitlab/ci/test.yml])
-
- expect(helper.has_ci_changes?).to be_truthy
- end
- end
-
- context 'when .gitlab-ci.yml is changed' do
- it 'returns true' do
- expect(helper).to receive(:all_changed_files).and_return(%w[migration.rb .gitlab-ci.yml])
-
- expect(helper.has_ci_changes?).to be_truthy
- end
- end
-
- context 'when neither .gitlab/ci/ or .gitlab-ci.yml is changed' do
- it 'returns false' do
- expect(helper).to receive(:all_changed_files).and_return(%w[migration.rb nested/.gitlab-ci.yml])
-
- expect(helper.has_ci_changes?).to be_falsey
- end
- end
- end
-
- describe '#group_label' do
- it 'returns nil when no group label is present' do
- expect(helper.group_label(%w[foo bar])).to be_nil
- end
-
- it 'returns the group label when a group label is present' do
- expect(helper.group_label(['foo', 'group::source code', 'bar'])).to eq('group::source code')
- end
- end
-end
diff --git a/spec/tooling/danger/merge_request_linter_spec.rb b/spec/tooling/danger/merge_request_linter_spec.rb
deleted file mode 100644
index 3273b6b3d070c..0000000000000
--- a/spec/tooling/danger/merge_request_linter_spec.rb
+++ /dev/null
@@ -1,54 +0,0 @@
-# frozen_string_literal: true
-
-require 'rspec-parameterized'
-require_relative 'danger_spec_helper'
-
-require_relative '../../../tooling/danger/merge_request_linter'
-
-RSpec.describe Tooling::Danger::MergeRequestLinter do
- using RSpec::Parameterized::TableSyntax
-
- let(:mr_class) do
- Struct.new(:message, :sha, :diff_parent)
- end
-
- let(:mr_title) { 'A B ' + 'C' }
- let(:merge_request) { mr_class.new(mr_title, anything, anything) }
-
- describe '#lint_subject' do
- subject(:mr_linter) { described_class.new(merge_request) }
-
- shared_examples 'a valid mr title' do
- it 'does not have any problem' do
- mr_linter.lint
-
- expect(mr_linter.problems).to be_empty
- end
- end
-
- context 'when subject valid' do
- it_behaves_like 'a valid mr title'
- end
-
- context 'when it is too long' do
- let(:mr_title) { 'A B ' + 'C' * described_class::MAX_LINE_LENGTH }
-
- it 'adds a problem' do
- expect(mr_linter).to receive(:add_problem).with(:subject_too_long, described_class.subject_description)
-
- mr_linter.lint
- end
- end
-
- describe 'using magic mr run options' do
- where(run_option: described_class.mr_run_options_regex.split('|') +
- described_class.mr_run_options_regex.split('|').map! { |x| "[#{x}]" })
-
- with_them do
- let(:mr_title) { run_option + ' A B ' + 'C' * (described_class::MAX_LINE_LENGTH - 5) }
-
- it_behaves_like 'a valid mr title'
- end
- end
- end
-end
diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb
new file mode 100644
index 0000000000000..ff4afcf686366
--- /dev/null
+++ b/spec/tooling/danger/project_helper_spec.rb
@@ -0,0 +1,260 @@
+# frozen_string_literal: true
+
+require 'rspec-parameterized'
+require 'gitlab-dangerfiles'
+require 'danger/helper'
+require 'gitlab/dangerfiles/spec_helper'
+
+require_relative '../../../danger/plugins/project_helper'
+
+RSpec.describe Tooling::Danger::ProjectHelper do
+ include_context "with dangerfile"
+
+ let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
+ let(:fake_helper) { Danger::Helper.new(project_helper) }
+
+ subject(:project_helper) { fake_danger.new(git: fake_git) }
+
+ before do
+ allow(project_helper).to receive(:helper).and_return(fake_helper)
+ end
+
+ describe '#changes' do
+ it 'returns an array of Change objects' do
+ expect(project_helper.changes).to all(be_an(Gitlab::Dangerfiles::Change))
+ end
+
+ it 'groups changes by change type' do
+ changes = project_helper.changes
+
+ expect(changes.added.files).to eq(added_files)
+ expect(changes.modified.files).to eq(modified_files)
+ expect(changes.deleted.files).to eq(deleted_files)
+ expect(changes.renamed_before.files).to eq([renamed_before_file])
+ expect(changes.renamed_after.files).to eq([renamed_after_file])
+ end
+ end
+
+ describe '#categories_for_file' do
+ using RSpec::Parameterized::TableSyntax
+
+ before do
+ allow(fake_git).to receive(:diff_for_file).with('usage_data.rb') { double(:diff, patch: "+ count(User.active)") }
+ end
+
+ where(:path, :expected_categories) do
+ 'usage_data.rb' | [:database, :backend]
+ 'doc/foo.md' | [:docs]
+ 'CONTRIBUTING.md' | [:docs]
+ 'LICENSE' | [:docs]
+ 'MAINTENANCE.md' | [:docs]
+ 'PHILOSOPHY.md' | [:docs]
+ 'PROCESS.md' | [:docs]
+ 'README.md' | [:docs]
+
+ 'ee/doc/foo' | [:unknown]
+ 'ee/README' | [:unknown]
+
+ 'app/assets/foo' | [:frontend]
+ 'app/views/foo' | [:frontend]
+ 'public/foo' | [:frontend]
+ 'scripts/frontend/foo' | [:frontend]
+ 'spec/javascripts/foo' | [:frontend]
+ 'spec/frontend/bar' | [:frontend]
+ 'vendor/assets/foo' | [:frontend]
+ 'babel.config.js' | [:frontend]
+ 'jest.config.js' | [:frontend]
+ 'package.json' | [:frontend]
+ 'yarn.lock' | [:frontend]
+ 'config/foo.js' | [:frontend]
+ 'config/deep/foo.js' | [:frontend]
+
+ 'ee/app/assets/foo' | [:frontend]
+ 'ee/app/views/foo' | [:frontend]
+ 'ee/spec/javascripts/foo' | [:frontend]
+ 'ee/spec/frontend/bar' | [:frontend]
+
+ '.gitlab/ci/frontend.gitlab-ci.yml' | %i[frontend engineering_productivity]
+
+ 'app/models/foo' | [:backend]
+ 'bin/foo' | [:backend]
+ 'config/foo' | [:backend]
+ 'lib/foo' | [:backend]
+ 'rubocop/foo' | [:backend]
+ '.rubocop.yml' | [:backend]
+ '.rubocop_todo.yml' | [:backend]
+ '.rubocop_manual_todo.yml' | [:backend]
+ 'spec/foo' | [:backend]
+ 'spec/foo/bar' | [:backend]
+
+ 'ee/app/foo' | [:backend]
+ 'ee/bin/foo' | [:backend]
+ 'ee/spec/foo' | [:backend]
+ 'ee/spec/foo/bar' | [:backend]
+
+ 'spec/features/foo' | [:test]
+ 'ee/spec/features/foo' | [:test]
+ 'spec/support/shared_examples/features/foo' | [:test]
+ 'ee/spec/support/shared_examples/features/foo' | [:test]
+ 'spec/support/shared_contexts/features/foo' | [:test]
+ 'ee/spec/support/shared_contexts/features/foo' | [:test]
+ 'spec/support/helpers/features/foo' | [:test]
+ 'ee/spec/support/helpers/features/foo' | [:test]
+
+ 'generator_templates/foo' | [:backend]
+ 'vendor/languages.yml' | [:backend]
+ 'file_hooks/examples/' | [:backend]
+
+ 'Gemfile' | [:backend]
+ 'Gemfile.lock' | [:backend]
+ 'Rakefile' | [:backend]
+ 'FOO_VERSION' | [:backend]
+
+ 'Dangerfile' | [:engineering_productivity]
+ 'danger/commit_messages/Dangerfile' | [:engineering_productivity]
+ 'ee/danger/commit_messages/Dangerfile' | [:engineering_productivity]
+ 'danger/commit_messages/' | [:engineering_productivity]
+ 'ee/danger/commit_messages/' | [:engineering_productivity]
+ '.gitlab-ci.yml' | [:engineering_productivity]
+ '.gitlab/ci/cng.gitlab-ci.yml' | [:engineering_productivity]
+ '.gitlab/ci/ee-specific-checks.gitlab-ci.yml' | [:engineering_productivity]
+ 'scripts/foo' | [:engineering_productivity]
+ 'tooling/danger/foo' | [:engineering_productivity]
+ 'ee/tooling/danger/foo' | [:engineering_productivity]
+ 'lefthook.yml' | [:engineering_productivity]
+ '.editorconfig' | [:engineering_productivity]
+ 'tooling/bin/find_foss_tests' | [:engineering_productivity]
+ '.codeclimate.yml' | [:engineering_productivity]
+ '.gitlab/CODEOWNERS' | [:engineering_productivity]
+
+ 'lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml' | [:ci_template]
+ 'lib/gitlab/ci/templates/dotNET-Core.yml' | [:ci_template]
+
+ 'ee/FOO_VERSION' | [:unknown]
+
+ 'db/schema.rb' | [:database]
+ 'db/structure.sql' | [:database]
+ 'db/migrate/foo' | [:database, :migration]
+ 'db/post_migrate/foo' | [:database, :migration]
+ 'ee/db/geo/migrate/foo' | [:database, :migration]
+ 'ee/db/geo/post_migrate/foo' | [:database, :migration]
+ 'app/models/project_authorization.rb' | [:database]
+ 'app/services/users/refresh_authorized_projects_service.rb' | [:database]
+ 'lib/gitlab/background_migration.rb' | [:database]
+ 'lib/gitlab/background_migration/foo' | [:database]
+ 'ee/lib/gitlab/background_migration/foo' | [:database]
+ 'lib/gitlab/database.rb' | [:database]
+ 'lib/gitlab/database/foo' | [:database]
+ 'ee/lib/gitlab/database/foo' | [:database]
+ 'lib/gitlab/github_import.rb' | [:database]
+ 'lib/gitlab/github_import/foo' | [:database]
+ 'lib/gitlab/sql/foo' | [:database]
+ 'rubocop/cop/migration/foo' | [:database]
+
+ 'db/fixtures/foo.rb' | [:backend]
+ 'ee/db/fixtures/foo.rb' | [:backend]
+
+ 'qa/foo' | [:qa]
+ 'ee/qa/foo' | [:qa]
+
+ 'changelogs/foo' | [:none]
+ 'ee/changelogs/foo' | [:none]
+ 'locale/gitlab.pot' | [:none]
+
+ 'FOO' | [:unknown]
+ 'foo' | [:unknown]
+
+ 'foo/bar.rb' | [:backend]
+ 'foo/bar.js' | [:frontend]
+ 'foo/bar.txt' | [:none]
+ 'foo/bar.md' | [:none]
+ end
+
+ with_them do
+ subject { project_helper.categories_for_file(path) }
+
+ it { is_expected.to eq(expected_categories) }
+ end
+
+ context 'having specific changes' do
+ where(:expected_categories, :patch, :changed_files) do
+ [:database, :backend] | '+ count(User.active)' | ['usage_data.rb', 'lib/gitlab/usage_data.rb', 'ee/lib/ee/gitlab/usage_data.rb']
+ [:database, :backend] | '+ estimate_batch_distinct_count(User.active)' | ['usage_data.rb']
+ [:backend] | '+ alt_usage_data(User.active)' | ['usage_data.rb']
+ [:backend] | '+ count(User.active)' | ['user.rb']
+ [:backend] | '+ count(User.active)' | ['usage_data/topology.rb']
+ [:backend] | '+ foo_count(User.active)' | ['usage_data.rb']
+ end
+
+ with_them do
+ it 'has the correct categories' do
+ changed_files.each do |file|
+ allow(fake_git).to receive(:diff_for_file).with(file) { double(:diff, patch: patch) }
+
+ expect(project_helper.categories_for_file(file)).to eq(expected_categories)
+ end
+ end
+ end
+ 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: changes_size, commit_messages, database, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css')
+ 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 '#all_ee_changes' do
+ subject { project_helper.all_ee_changes }
+
+ it 'returns all changed files starting with ee/' do
+ expect(project_helper).to receive(:all_changed_files).and_return(%w[fr/ee/beer.rb ee/wine.rb ee/lib/ido.rb ee.k])
+
+ is_expected.to match_array(%w[ee/wine.rb ee/lib/ido.rb])
+ end
+ end
+
+ describe '#project_name' do
+ subject { project_helper.project_name }
+
+ it 'returns gitlab if ee? returns true' do
+ expect(project_helper).to receive(:ee?) { true }
+
+ is_expected.to eq('gitlab')
+ end
+
+ it 'returns gitlab-ce if ee? returns false' do
+ expect(project_helper).to receive(:ee?) { false }
+
+ is_expected.to eq('gitlab-foss')
+ end
+ end
+end
diff --git a/spec/tooling/danger/roulette_spec.rb b/spec/tooling/danger/roulette_spec.rb
deleted file mode 100644
index 1e500a1ed08d2..0000000000000
--- a/spec/tooling/danger/roulette_spec.rb
+++ /dev/null
@@ -1,429 +0,0 @@
-# frozen_string_literal: true
-
-require 'webmock/rspec'
-require 'timecop'
-
-require_relative '../../../tooling/danger/roulette'
-require 'active_support/testing/time_helpers'
-
-RSpec.describe Tooling::Danger::Roulette do
- include ActiveSupport::Testing::TimeHelpers
-
- around do |example|
- travel_to(Time.utc(2020, 06, 22, 10)) { example.run }
- end
-
- let(:backend_available) { true }
- let(:backend_tz_offset_hours) { 2.0 }
- let(:backend_maintainer) do
- Tooling::Danger::Teammate.new(
- 'username' => 'backend-maintainer',
- 'name' => 'Backend maintainer',
- 'role' => 'Backend engineer',
- 'projects' => { 'gitlab' => 'maintainer backend' },
- 'available' => backend_available,
- 'tz_offset_hours' => backend_tz_offset_hours
- )
- end
-
- let(:frontend_reviewer) do
- Tooling::Danger::Teammate.new(
- 'username' => 'frontend-reviewer',
- 'name' => 'Frontend reviewer',
- 'role' => 'Frontend engineer',
- 'projects' => { 'gitlab' => 'reviewer frontend' },
- 'available' => true,
- 'tz_offset_hours' => 2.0
- )
- end
-
- let(:frontend_maintainer) do
- Tooling::Danger::Teammate.new(
- 'username' => 'frontend-maintainer',
- 'name' => 'Frontend maintainer',
- 'role' => 'Frontend engineer',
- 'projects' => { 'gitlab' => "maintainer frontend" },
- 'available' => true,
- 'tz_offset_hours' => 2.0
- )
- end
-
- let(:software_engineer_in_test) do
- Tooling::Danger::Teammate.new(
- 'username' => 'software-engineer-in-test',
- 'name' => 'Software Engineer in Test',
- 'role' => 'Software Engineer in Test, Create:Source Code',
- 'projects' => { 'gitlab' => 'maintainer qa', 'gitlab-qa' => 'maintainer' },
- 'available' => true,
- 'tz_offset_hours' => 2.0
- )
- end
-
- let(:engineering_productivity_reviewer) do
- Tooling::Danger::Teammate.new(
- 'username' => 'eng-prod-reviewer',
- 'name' => 'EP engineer',
- 'role' => 'Engineering Productivity',
- 'projects' => { 'gitlab' => 'reviewer backend' },
- 'available' => true,
- 'tz_offset_hours' => 2.0
- )
- end
-
- let(:ci_template_reviewer) do
- Tooling::Danger::Teammate.new(
- 'username' => 'ci-template-maintainer',
- 'name' => 'CI Template engineer',
- 'role' => '~"ci::templates"',
- 'projects' => { 'gitlab' => 'reviewer ci_template' },
- 'available' => true,
- 'tz_offset_hours' => 2.0
- )
- end
-
- let(:teammates) do
- [
- backend_maintainer.to_h,
- frontend_maintainer.to_h,
- frontend_reviewer.to_h,
- software_engineer_in_test.to_h,
- engineering_productivity_reviewer.to_h,
- ci_template_reviewer.to_h
- ]
- end
-
- let(:teammate_json) do
- teammates.to_json
- end
-
- subject(:roulette) { Object.new.extend(described_class) }
-
- describe 'Spin#==' do
- it 'compares Spin attributes' do
- spin1 = described_class::Spin.new(:backend, frontend_reviewer, frontend_maintainer, false, false)
- spin2 = described_class::Spin.new(:backend, frontend_reviewer, frontend_maintainer, false, false)
- spin3 = described_class::Spin.new(:backend, frontend_reviewer, frontend_maintainer, false, true)
- spin4 = described_class::Spin.new(:backend, frontend_reviewer, frontend_maintainer, true, false)
- spin5 = described_class::Spin.new(:backend, frontend_reviewer, backend_maintainer, false, false)
- spin6 = described_class::Spin.new(:backend, backend_maintainer, frontend_maintainer, false, false)
- spin7 = described_class::Spin.new(:frontend, frontend_reviewer, frontend_maintainer, false, false)
-
- expect(spin1).to eq(spin2)
- expect(spin1).not_to eq(spin3)
- expect(spin1).not_to eq(spin4)
- expect(spin1).not_to eq(spin5)
- expect(spin1).not_to eq(spin6)
- expect(spin1).not_to eq(spin7)
- end
- end
-
- describe '#spin' do
- let!(:project) { 'gitlab' }
- let!(:mr_source_branch) { 'a-branch' }
- let!(:mr_labels) { ['backend', 'devops::create'] }
- let!(:author) { Tooling::Danger::Teammate.new('username' => 'johndoe') }
- let(:timezone_experiment) { false }
- let(:spins) do
- # Stub the request at the latest time so that we can modify the raw data, e.g. available fields.
- WebMock
- .stub_request(:get, described_class::ROULETTE_DATA_URL)
- .to_return(body: teammate_json)
-
- subject.spin(project, categories, timezone_experiment: timezone_experiment)
- end
-
- before do
- allow(subject).to receive(:mr_author_username).and_return(author.username)
- allow(subject).to receive(:mr_labels).and_return(mr_labels)
- allow(subject).to receive(:mr_source_branch).and_return(mr_source_branch)
- end
-
- context 'when timezone_experiment == false' do
- context 'when change contains backend category' do
- let(:categories) { [:backend] }
-
- it 'assigns backend reviewer and maintainer' do
- expect(spins[0].reviewer).to eq(engineering_productivity_reviewer)
- expect(spins[0].maintainer).to eq(backend_maintainer)
- expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false)])
- end
-
- context 'when teammate is not available' do
- let(:backend_available) { false }
-
- it 'assigns backend reviewer and no maintainer' do
- expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, nil, false, false)])
- end
- end
- end
-
- context 'when change contains frontend category' do
- let(:categories) { [:frontend] }
-
- it 'assigns frontend reviewer and maintainer' do
- expect(spins).to eq([described_class::Spin.new(:frontend, frontend_reviewer, frontend_maintainer, false, false)])
- end
- end
-
- context 'when change contains many categories' do
- let(:categories) { [:frontend, :test, :qa, :engineering_productivity, :ci_template, :backend] }
-
- it 'has a deterministic sorting order' do
- expect(spins.map(&:category)).to eq categories.sort
- end
- end
-
- context 'when change contains QA category' do
- let(:categories) { [:qa] }
-
- it 'assigns QA maintainer' do
- expect(spins).to eq([described_class::Spin.new(:qa, nil, software_engineer_in_test, false, false)])
- end
- end
-
- context 'when change contains QA category and another category' do
- let(:categories) { [:backend, :qa] }
-
- it 'assigns QA maintainer' do
- expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false), described_class::Spin.new(:qa, nil, software_engineer_in_test, :maintainer, false)])
- end
-
- context 'and author is an SET' do
- let!(:author) { Tooling::Danger::Teammate.new('username' => software_engineer_in_test.username) }
-
- it 'assigns QA reviewer' do
- expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false), described_class::Spin.new(:qa, nil, nil, false, false)])
- end
- end
- end
-
- context 'when change contains Engineering Productivity category' do
- let(:categories) { [:engineering_productivity] }
-
- it 'assigns Engineering Productivity reviewer and fallback to backend maintainer' do
- expect(spins).to eq([described_class::Spin.new(:engineering_productivity, engineering_productivity_reviewer, backend_maintainer, false, false)])
- end
- end
-
- context 'when change contains CI/CD Template category' do
- let(:categories) { [:ci_template] }
-
- it 'assigns CI/CD Template reviewer and fallback to backend maintainer' do
- expect(spins).to eq([described_class::Spin.new(:ci_template, ci_template_reviewer, backend_maintainer, false, false)])
- end
- end
-
- context 'when change contains test category' do
- let(:categories) { [:test] }
-
- it 'assigns corresponding SET' do
- expect(spins).to eq([described_class::Spin.new(:test, software_engineer_in_test, nil, :maintainer, false)])
- end
- end
- end
-
- context 'when timezone_experiment == true' do
- let(:timezone_experiment) { true }
-
- context 'when change contains backend category' do
- let(:categories) { [:backend] }
-
- it 'assigns backend reviewer and maintainer' do
- expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, true)])
- end
-
- context 'when teammate is not in a good timezone' do
- let(:backend_tz_offset_hours) { 5.0 }
-
- it 'assigns backend reviewer and no maintainer' do
- expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, nil, false, true)])
- end
- end
- end
-
- context 'when change includes a category with timezone disabled' do
- let(:categories) { [:backend] }
-
- before do
- stub_const("#{described_class}::INCLUDE_TIMEZONE_FOR_CATEGORY", backend: false)
- end
-
- it 'assigns backend reviewer and maintainer' do
- expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false)])
- end
-
- context 'when teammate is not in a good timezone' do
- let(:backend_tz_offset_hours) { 5.0 }
-
- it 'assigns backend reviewer and maintainer' do
- expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false)])
- end
- end
- end
- end
- end
-
- RSpec::Matchers.define :match_teammates do |expected|
- match do |actual|
- expected.each do |expected_person|
- actual_person_found = actual.find { |actual_person| actual_person.name == expected_person.username }
-
- actual_person_found &&
- actual_person_found.name == expected_person.name &&
- actual_person_found.role == expected_person.role &&
- actual_person_found.projects == expected_person.projects
- end
- end
- end
-
- describe '#team' do
- subject(:team) { roulette.team }
-
- context 'HTTP failure' do
- before do
- WebMock
- .stub_request(:get, described_class::ROULETTE_DATA_URL)
- .to_return(status: 404)
- end
-
- it 'raises a pretty error' do
- expect { team }.to raise_error(/Failed to read/)
- end
- end
-
- context 'JSON failure' do
- before do
- WebMock
- .stub_request(:get, described_class::ROULETTE_DATA_URL)
- .to_return(body: 'INVALID JSON')
- end
-
- it 'raises a pretty error' do
- expect { team }.to raise_error(/Failed to parse/)
- end
- end
-
- context 'success' do
- before do
- WebMock
- .stub_request(:get, described_class::ROULETTE_DATA_URL)
- .to_return(body: teammate_json)
- end
-
- it 'returns an array of teammates' do
- is_expected.to match_teammates([
- backend_maintainer,
- frontend_reviewer,
- frontend_maintainer,
- software_engineer_in_test,
- engineering_productivity_reviewer,
- ci_template_reviewer
- ])
- end
-
- it 'memoizes the result' do
- expect(team.object_id).to eq(roulette.team.object_id)
- end
- end
- end
-
- describe '#project_team' do
- subject { roulette.project_team('gitlab-qa') }
-
- before do
- WebMock
- .stub_request(:get, described_class::ROULETTE_DATA_URL)
- .to_return(body: teammate_json)
- end
-
- it 'filters team by project_name' do
- is_expected.to match_teammates([
- software_engineer_in_test
- ])
- end
- end
-
- describe '#spin_for_person' do
- let(:person_tz_offset_hours) { 0.0 }
- let(:person1) do
- Tooling::Danger::Teammate.new(
- 'username' => 'user1',
- 'available' => true,
- 'tz_offset_hours' => person_tz_offset_hours
- )
- end
-
- let(:person2) do
- Tooling::Danger::Teammate.new(
- 'username' => 'user2',
- 'available' => true,
- 'tz_offset_hours' => person_tz_offset_hours)
- end
-
- let(:author) do
- Tooling::Danger::Teammate.new(
- 'username' => 'johndoe',
- 'available' => true,
- 'tz_offset_hours' => 0.0)
- end
-
- let(:unavailable) do
- Tooling::Danger::Teammate.new(
- 'username' => 'janedoe',
- 'available' => false,
- 'tz_offset_hours' => 0.0)
- end
-
- before do
- allow(subject).to receive(:mr_author_username).and_return(author.username)
- end
-
- (-4..4).each do |utc_offset|
- context "when local hour for person is #{10 + utc_offset} (offset: #{utc_offset})" do
- let(:person_tz_offset_hours) { utc_offset }
-
- [false, true].each do |timezone_experiment|
- context "with timezone_experiment == #{timezone_experiment}" do
- it 'returns a random person' do
- persons = [person1, person2]
-
- selected = subject.spin_for_person(persons, random: Random.new, timezone_experiment: timezone_experiment)
-
- expect(persons.map(&:username)).to include(selected.username)
- end
- end
- end
- end
- end
-
- ((-12..-5).to_a + (5..12).to_a).each do |utc_offset|
- context "when local hour for person is #{10 + utc_offset} (offset: #{utc_offset})" do
- let(:person_tz_offset_hours) { utc_offset }
-
- [false, true].each do |timezone_experiment|
- context "with timezone_experiment == #{timezone_experiment}" do
- it 'returns a random person or nil' do
- persons = [person1, person2]
-
- selected = subject.spin_for_person(persons, random: Random.new, timezone_experiment: timezone_experiment)
-
- if timezone_experiment
- expect(selected).to be_nil
- else
- expect(persons.map(&:username)).to include(selected.username)
- end
- end
- end
- end
- end
- end
-
- it 'excludes unavailable persons' do
- expect(subject.spin_for_person([unavailable], random: Random.new)).to be_nil
- end
-
- it 'excludes mr.author' do
- expect(subject.spin_for_person([author], random: Random.new)).to be_nil
- end
- end
-end
diff --git a/spec/tooling/danger/sidekiq_queues_spec.rb b/spec/tooling/danger/sidekiq_queues_spec.rb
index c5fc85926216e..9bffc7ee93d8a 100644
--- a/spec/tooling/danger/sidekiq_queues_spec.rb
+++ b/spec/tooling/danger/sidekiq_queues_spec.rb
@@ -1,20 +1,21 @@
# frozen_string_literal: true
require 'rspec-parameterized'
-require_relative 'danger_spec_helper'
+require 'gitlab-dangerfiles'
+require 'gitlab/dangerfiles/spec_helper'
require_relative '../../../tooling/danger/sidekiq_queues'
RSpec.describe Tooling::Danger::SidekiqQueues do
- using RSpec::Parameterized::TableSyntax
- include DangerSpecHelper
+ include_context "with dangerfile"
- let(:fake_git) { double('fake-git') }
- let(:fake_danger) { new_fake_danger.include(described_class) }
+ let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
subject(:sidekiq_queues) { fake_danger.new(git: fake_git) }
describe '#changed_queue_files' 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)
diff --git a/spec/tooling/danger/teammate_spec.rb b/spec/tooling/danger/teammate_spec.rb
deleted file mode 100644
index f3afdc6e9120e..0000000000000
--- a/spec/tooling/danger/teammate_spec.rb
+++ /dev/null
@@ -1,225 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../../../tooling/danger/teammate'
-require 'active_support/testing/time_helpers'
-require 'rspec-parameterized'
-
-RSpec.describe Tooling::Danger::Teammate do
- using RSpec::Parameterized::TableSyntax
-
- subject { described_class.new(options) }
-
- let(:tz_offset_hours) { 2.0 }
- let(:options) do
- {
- 'username' => 'luigi',
- 'projects' => projects,
- 'role' => role,
- 'markdown_name' => '[Luigi](https://gitlab.com/luigi) (`@luigi`)',
- 'tz_offset_hours' => tz_offset_hours
- }
- end
-
- let(:capabilities) { ['reviewer backend'] }
- let(:projects) { { project => capabilities } }
- let(:role) { 'Engineer, Manage' }
- let(:labels) { [] }
- let(:project) { double }
-
- describe '#==' do
- it 'compares Teammate username' do
- joe1 = described_class.new('username' => 'joe', 'projects' => projects)
- joe2 = described_class.new('username' => 'joe', 'projects' => [])
- jane1 = described_class.new('username' => 'jane', 'projects' => projects)
- jane2 = described_class.new('username' => 'jane', 'projects' => [])
-
- expect(joe1).to eq(joe2)
- expect(jane1).to eq(jane2)
- expect(jane1).not_to eq(nil)
- expect(described_class.new('username' => nil)).not_to eq(nil)
- end
- end
-
- describe '#to_h' do
- it 'returns the given options' do
- expect(subject.to_h).to eq(options)
- end
- end
-
- context 'when having multiple capabilities' do
- let(:capabilities) { ['reviewer backend', 'maintainer frontend', 'trainee_maintainer qa'] }
-
- it '#any_capability? returns true if the person has any capability for the category in the given project' do
- expect(subject.any_capability?(project, :backend)).to be_truthy
- expect(subject.any_capability?(project, :frontend)).to be_truthy
- expect(subject.any_capability?(project, :qa)).to be_truthy
- expect(subject.any_capability?(project, :engineering_productivity)).to be_falsey
- end
-
- it '#reviewer? supports multiple roles per project' do
- expect(subject.reviewer?(project, :backend, labels)).to be_truthy
- end
-
- it '#traintainer? supports multiple roles per project' do
- expect(subject.traintainer?(project, :qa, labels)).to be_truthy
- end
-
- it '#maintainer? supports multiple roles per project' do
- expect(subject.maintainer?(project, :frontend, labels)).to be_truthy
- end
-
- context 'when labels contain devops::create and the category is test' do
- let(:labels) { ['devops::create'] }
-
- context 'when role is Software Engineer in Test, Create' do
- let(:role) { 'Software Engineer in Test, Create' }
-
- it '#reviewer? returns true' do
- expect(subject.reviewer?(project, :test, labels)).to be_truthy
- end
-
- it '#maintainer? returns false' do
- expect(subject.maintainer?(project, :test, labels)).to be_falsey
- end
-
- context 'when hyperlink is mangled in the role' do
- let(:role) { '<a href="#">Software Engineer in Test</a>, Create' }
-
- it '#reviewer? returns true' do
- expect(subject.reviewer?(project, :test, labels)).to be_truthy
- end
- end
- end
-
- context 'when role is Software Engineer in Test' do
- let(:role) { 'Software Engineer in Test' }
-
- it '#reviewer? returns false' do
- expect(subject.reviewer?(project, :test, labels)).to be_falsey
- end
- end
-
- context 'when role is Software Engineer in Test, Manage' do
- let(:role) { 'Software Engineer in Test, Manage' }
-
- it '#reviewer? returns false' do
- expect(subject.reviewer?(project, :test, labels)).to be_falsey
- end
- end
-
- context 'when role is Backend Engineer, Engineering Productivity' do
- let(:role) { 'Backend Engineer, Engineering Productivity' }
-
- it '#reviewer? returns true' do
- expect(subject.reviewer?(project, :engineering_productivity, labels)).to be_truthy
- end
-
- it '#maintainer? returns false' do
- expect(subject.maintainer?(project, :engineering_productivity, labels)).to be_falsey
- end
-
- context 'when capabilities include maintainer backend' do
- let(:capabilities) { ['maintainer backend'] }
-
- it '#maintainer? returns true' do
- expect(subject.maintainer?(project, :engineering_productivity, labels)).to be_truthy
- end
- end
-
- context 'when capabilities include maintainer engineering productivity' do
- let(:capabilities) { ['maintainer engineering_productivity'] }
-
- it '#maintainer? returns true' do
- expect(subject.maintainer?(project, :engineering_productivity, labels)).to be_truthy
- end
- end
-
- context 'when capabilities include trainee_maintainer backend' do
- let(:capabilities) { ['trainee_maintainer backend'] }
-
- it '#traintainer? returns true' do
- expect(subject.traintainer?(project, :engineering_productivity, labels)).to be_truthy
- end
- end
- end
- end
- end
-
- context 'when having single capability' do
- let(:capabilities) { 'reviewer backend' }
-
- it '#reviewer? supports one role per project' do
- expect(subject.reviewer?(project, :backend, labels)).to be_truthy
- end
-
- it '#traintainer? supports one role per project' do
- expect(subject.traintainer?(project, :database, labels)).to be_falsey
- end
-
- it '#maintainer? supports one role per project' do
- expect(subject.maintainer?(project, :frontend, labels)).to be_falsey
- end
- end
-
- describe '#local_hour' do
- include ActiveSupport::Testing::TimeHelpers
-
- around do |example|
- travel_to(Time.utc(2020, 6, 23, 10)) { example.run }
- end
-
- context 'when author is given' do
- where(:tz_offset_hours, :expected_local_hour) do
- -12 | 22
- -10 | 0
- 2 | 12
- 4 | 14
- 12 | 22
- end
-
- with_them do
- it 'returns the correct local_hour' do
- expect(subject.local_hour).to eq(expected_local_hour)
- end
- end
- end
- end
-
- describe '#markdown_name' do
- it 'returns markdown name with timezone info' do
- expect(subject.markdown_name).to eq("#{options['markdown_name']} (UTC+2)")
- end
-
- context 'when offset is 1.5' do
- let(:tz_offset_hours) { 1.5 }
-
- it 'returns markdown name with timezone info, not truncated' do
- expect(subject.markdown_name).to eq("#{options['markdown_name']} (UTC+1.5)")
- end
- end
-
- context 'when author is given' do
- where(:tz_offset_hours, :author_offset, :diff_text) do
- -12 | -10 | "2 hours behind `@mario`"
- -10 | -12 | "2 hours ahead of `@mario`"
- -10 | 2 | "12 hours behind `@mario`"
- 2 | 4 | "2 hours behind `@mario`"
- 4 | 2 | "2 hours ahead of `@mario`"
- 2 | 3 | "1 hour behind `@mario`"
- 3 | 2 | "1 hour ahead of `@mario`"
- 2 | 2 | "same timezone as `@mario`"
- end
-
- with_them do
- it 'returns markdown name with timezone info' do
- author = described_class.new(options.merge('username' => 'mario', 'tz_offset_hours' => author_offset))
-
- floored_offset_hours = subject.__send__(:floored_offset_hours)
- utc_offset = floored_offset_hours >= 0 ? "+#{floored_offset_hours}" : floored_offset_hours
-
- expect(subject.markdown_name(author: author)).to eq("#{options['markdown_name']} (UTC#{utc_offset}, #{diff_text})")
- end
- end
- end
- end
-end
diff --git a/spec/tooling/danger/title_linting_spec.rb b/spec/tooling/danger/title_linting_spec.rb
deleted file mode 100644
index 7bc1684cd87d5..0000000000000
--- a/spec/tooling/danger/title_linting_spec.rb
+++ /dev/null
@@ -1,91 +0,0 @@
-# frozen_string_literal: true
-
-require 'rspec-parameterized'
-
-require_relative '../../../tooling/danger/title_linting'
-
-RSpec.describe Tooling::Danger::TitleLinting do
- using RSpec::Parameterized::TableSyntax
-
- describe '#sanitize_mr_title' do
- where(:mr_title, :expected_mr_title) do
- '`My MR title`' | "\\`My MR title\\`"
- 'WIP: My MR title' | 'My MR title'
- 'Draft: My MR title' | 'My MR title'
- '(Draft) My MR title' | 'My MR title'
- '[Draft] My MR title' | 'My MR title'
- '[DRAFT] My MR title' | 'My MR title'
- 'DRAFT: My MR title' | 'My MR title'
- 'DRAFT: `My MR title`' | "\\`My MR title\\`"
- end
-
- with_them do
- subject { described_class.sanitize_mr_title(mr_title) }
-
- it { is_expected.to eq(expected_mr_title) }
- end
- end
-
- describe '#remove_draft_flag' do
- where(:mr_title, :expected_mr_title) do
- 'WIP: My MR title' | 'My MR title'
- 'Draft: My MR title' | 'My MR title'
- '(Draft) My MR title' | 'My MR title'
- '[Draft] My MR title' | 'My MR title'
- '[DRAFT] My MR title' | 'My MR title'
- 'DRAFT: My MR title' | 'My MR title'
- end
-
- with_them do
- subject { described_class.remove_draft_flag(mr_title) }
-
- it { is_expected.to eq(expected_mr_title) }
- end
- end
-
- describe '#has_draft_flag?' do
- it 'returns true for a draft title' do
- expect(described_class.has_draft_flag?('Draft: My MR title')).to be true
- end
-
- it 'returns false for non draft title' do
- expect(described_class.has_draft_flag?('My MR title')).to be false
- end
- end
-
- describe '#has_cherry_pick_flag?' do
- [
- 'Cherry Pick !1234',
- 'cherry-pick !1234',
- 'CherryPick !1234'
- ].each do |mr_title|
- it 'returns true for cherry-pick title' do
- expect(described_class.has_cherry_pick_flag?(mr_title)).to be true
- end
- end
-
- it 'returns false for non cherry-pick title' do
- expect(described_class.has_cherry_pick_flag?('My MR title')).to be false
- end
- end
-
- describe '#has_run_all_rspec_flag?' do
- it 'returns true for a title that includes RUN ALL RSPEC' do
- expect(described_class.has_run_all_rspec_flag?('My MR title RUN ALL RSPEC')).to be true
- end
-
- it 'returns true for a title that does not include RUN ALL RSPEC' do
- expect(described_class.has_run_all_rspec_flag?('My MR title')).to be false
- end
- end
-
- describe '#has_run_as_if_foss_flag?' do
- it 'returns true for a title that includes RUN AS-IF-FOSS' do
- expect(described_class.has_run_as_if_foss_flag?('My MR title RUN AS-IF-FOSS')).to be true
- end
-
- it 'returns true for a title that does not include RUN AS-IF-FOSS' do
- expect(described_class.has_run_as_if_foss_flag?('My MR title')).to be false
- end
- end
-end
diff --git a/spec/tooling/danger/weightage/maintainers_spec.rb b/spec/tooling/danger/weightage/maintainers_spec.rb
deleted file mode 100644
index b99ffe706a477..0000000000000
--- a/spec/tooling/danger/weightage/maintainers_spec.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../../../../tooling/danger/weightage/maintainers'
-
-RSpec.describe Tooling::Danger::Weightage::Maintainers do
- let(:multiplier) { Tooling::Danger::Weightage::CAPACITY_MULTIPLIER }
- let(:regular_maintainer) { double('Teammate', reduced_capacity: false) }
- let(:reduced_capacity_maintainer) { double('Teammate', reduced_capacity: true) }
- let(:maintainers) do
- [
- regular_maintainer,
- reduced_capacity_maintainer
- ]
- end
-
- let(:maintainer_count) { Tooling::Danger::Weightage::BASE_REVIEWER_WEIGHT * multiplier }
- let(:reduced_capacity_maintainer_count) { Tooling::Danger::Weightage::BASE_REVIEWER_WEIGHT }
-
- subject(:weighted_maintainers) { described_class.new(maintainers).execute }
-
- describe '#execute' do
- it 'weights the maintainers overall' do
- expect(weighted_maintainers.count).to eq maintainer_count + reduced_capacity_maintainer_count
- end
-
- it 'has total count of regular maintainers' do
- expect(weighted_maintainers.count { |r| r.object_id == regular_maintainer.object_id }).to eq maintainer_count
- end
-
- it 'has count of reduced capacity maintainers' do
- expect(weighted_maintainers.count { |r| r.object_id == reduced_capacity_maintainer.object_id }).to eq reduced_capacity_maintainer_count
- end
- end
-end
diff --git a/spec/tooling/danger/weightage/reviewers_spec.rb b/spec/tooling/danger/weightage/reviewers_spec.rb
deleted file mode 100644
index 5693ce7a10cc7..0000000000000
--- a/spec/tooling/danger/weightage/reviewers_spec.rb
+++ /dev/null
@@ -1,63 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../../../../tooling/danger/weightage/reviewers'
-
-RSpec.describe Tooling::Danger::Weightage::Reviewers do
- let(:multiplier) { Tooling::Danger::Weightage::CAPACITY_MULTIPLIER }
- let(:regular_reviewer) { double('Teammate', hungry: false, reduced_capacity: false) }
- let(:hungry_reviewer) { double('Teammate', hungry: true, reduced_capacity: false) }
- let(:reduced_capacity_reviewer) { double('Teammate', hungry: false, reduced_capacity: true) }
- let(:reviewers) do
- [
- hungry_reviewer,
- regular_reviewer,
- reduced_capacity_reviewer
- ]
- end
-
- let(:regular_traintainer) { double('Teammate', hungry: false, reduced_capacity: false) }
- let(:hungry_traintainer) { double('Teammate', hungry: true, reduced_capacity: false) }
- let(:reduced_capacity_traintainer) { double('Teammate', hungry: false, reduced_capacity: true) }
- let(:traintainers) do
- [
- hungry_traintainer,
- regular_traintainer,
- reduced_capacity_traintainer
- ]
- end
-
- let(:hungry_reviewer_count) { Tooling::Danger::Weightage::BASE_REVIEWER_WEIGHT * multiplier + described_class::DEFAULT_REVIEWER_WEIGHT }
- let(:hungry_traintainer_count) { described_class::TRAINTAINER_WEIGHT * multiplier + described_class::DEFAULT_REVIEWER_WEIGHT }
- let(:reviewer_count) { Tooling::Danger::Weightage::BASE_REVIEWER_WEIGHT * multiplier }
- let(:traintainer_count) { Tooling::Danger::Weightage::BASE_REVIEWER_WEIGHT * described_class::TRAINTAINER_WEIGHT * multiplier }
- let(:reduced_capacity_reviewer_count) { Tooling::Danger::Weightage::BASE_REVIEWER_WEIGHT }
- let(:reduced_capacity_traintainer_count) { described_class::TRAINTAINER_WEIGHT }
-
- subject(:weighted_reviewers) { described_class.new(reviewers, traintainers).execute }
-
- describe '#execute', :aggregate_failures do
- it 'weights the reviewers overall' do
- reviewers_count = hungry_reviewer_count + reviewer_count + reduced_capacity_reviewer_count
- traintainers_count = hungry_traintainer_count + traintainer_count + reduced_capacity_traintainer_count
-
- expect(weighted_reviewers.count).to eq reviewers_count + traintainers_count
- end
-
- it 'has total count of hungry reviewers and traintainers' do
- expect(weighted_reviewers.count(&:hungry)).to eq hungry_reviewer_count + hungry_traintainer_count
- expect(weighted_reviewers.count { |r| r.object_id == hungry_reviewer.object_id }).to eq hungry_reviewer_count
- expect(weighted_reviewers.count { |r| r.object_id == hungry_traintainer.object_id }).to eq hungry_traintainer_count
- end
-
- it 'has total count of regular reviewers and traintainers' do
- expect(weighted_reviewers.count { |r| r.object_id == regular_reviewer.object_id }).to eq reviewer_count
- expect(weighted_reviewers.count { |r| r.object_id == regular_traintainer.object_id }).to eq traintainer_count
- end
-
- it 'has count of reduced capacity reviewers' do
- expect(weighted_reviewers.count(&:reduced_capacity)).to eq reduced_capacity_reviewer_count + reduced_capacity_traintainer_count
- expect(weighted_reviewers.count { |r| r.object_id == reduced_capacity_reviewer.object_id }).to eq reduced_capacity_reviewer_count
- expect(weighted_reviewers.count { |r| r.object_id == reduced_capacity_traintainer.object_id }).to eq reduced_capacity_traintainer_count
- end
- end
-end
diff --git a/spec/tooling/gitlab_danger_spec.rb b/spec/tooling/gitlab_danger_spec.rb
deleted file mode 100644
index 20ac40d1d2a89..0000000000000
--- a/spec/tooling/gitlab_danger_spec.rb
+++ /dev/null
@@ -1,76 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../../tooling/gitlab_danger'
-
-RSpec.describe GitlabDanger do
- let(:gitlab_danger_helper) { nil }
-
- subject { described_class.new(gitlab_danger_helper) }
-
- 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: #{described_class::LOCAL_RULES.join(', ')}")
- 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
- it 'returns local only rules' do
- expect(subject.rule_names).to eq(described_class::LOCAL_RULES)
- end
- end
-
- context 'when running under CI' do
- let(:gitlab_danger_helper) { double('danger_gitlab_helper') }
-
- it 'returns all rules' do
- expect(subject.rule_names).to eq(described_class::LOCAL_RULES | described_class::CI_ONLY_RULES)
- end
- end
- end
-
- describe '#html_link' do
- context 'when running locally' do
- it 'returns the same string' do
- str = 'something'
-
- expect(subject.html_link(str)).to eq(str)
- end
- end
-
- context 'when running under CI' do
- let(:gitlab_danger_helper) { double('danger_gitlab_helper') }
-
- it 'returns a HTML link formatted version of the string' do
- str = 'something'
- html_formatted_str = %Q{<a href="#{str}">#{str}</a>}
-
- expect(gitlab_danger_helper).to receive(:html_link).with(str).and_return(html_formatted_str)
-
- expect(subject.html_link(str)).to eq(html_formatted_str)
- end
- end
- end
-
- describe '#ci?' do
- context 'when gitlab_danger_helper is not available' do
- it 'returns false' do
- expect(subject.ci?).to be_falsey
- end
- end
-
- context 'when gitlab_danger_helper is available' do
- let(:gitlab_danger_helper) { double('danger_gitlab_helper') }
-
- it 'returns true' do
- expect(subject.ci?).to be_truthy
- end
- end
- end
-end
diff --git a/tooling/danger/base_linter.rb b/tooling/danger/base_linter.rb
deleted file mode 100644
index c58f2d84dc801..0000000000000
--- a/tooling/danger/base_linter.rb
+++ /dev/null
@@ -1,96 +0,0 @@
-# frozen_string_literal: true
-
-require_relative 'title_linting'
-
-module Tooling
- module Danger
- class BaseLinter
- MIN_SUBJECT_WORDS_COUNT = 3
- MAX_LINE_LENGTH = 72
-
- attr_reader :commit, :problems
-
- def self.problems_mapping
- {
- subject_too_short: "The %s must contain at least #{MIN_SUBJECT_WORDS_COUNT} words",
- subject_too_long: "The %s may not be longer than #{MAX_LINE_LENGTH} characters",
- subject_starts_with_lowercase: "The %s must start with a capital letter",
- subject_ends_with_a_period: "The %s must not end with a period"
- }
- end
-
- def self.subject_description
- 'commit subject'
- end
-
- def initialize(commit)
- @commit = commit
- @problems = {}
- end
-
- def failed?
- problems.any?
- end
-
- def add_problem(problem_key, *args)
- @problems[problem_key] = sprintf(self.class.problems_mapping[problem_key], *args)
- end
-
- def lint_subject
- if subject_too_short?
- add_problem(:subject_too_short, self.class.subject_description)
- end
-
- if subject_too_long?
- add_problem(:subject_too_long, self.class.subject_description)
- end
-
- if subject_starts_with_lowercase?
- add_problem(:subject_starts_with_lowercase, self.class.subject_description)
- end
-
- if subject_ends_with_a_period?
- add_problem(:subject_ends_with_a_period, self.class.subject_description)
- end
-
- self
- end
-
- private
-
- def subject
- TitleLinting.remove_draft_flag(message_parts[0])
- end
-
- def subject_too_short?
- subject.split(' ').length < MIN_SUBJECT_WORDS_COUNT
- end
-
- def subject_too_long?
- line_too_long?(subject)
- end
-
- def line_too_long?(line)
- line.length > MAX_LINE_LENGTH
- end
-
- def subject_starts_with_lowercase?
- return false if ('A'..'Z').cover?(subject[0])
-
- first_char = subject.sub(/\A(\[.+\]|\w+:)\s/, '')[0]
- first_char_downcased = first_char.downcase
- return true unless ('a'..'z').cover?(first_char_downcased)
-
- first_char.downcase == first_char
- end
-
- def subject_ends_with_a_period?
- subject.end_with?('.')
- end
-
- def message_parts
- @message_parts ||= commit.message.split("\n", 3)
- end
- end
- end
-end
diff --git a/tooling/danger/changelog.rb b/tooling/danger/changelog.rb
index 86184b38459bc..672d23d58e411 100644
--- a/tooling/danger/changelog.rb
+++ b/tooling/danger/changelog.rb
@@ -1,6 +1,6 @@
# frozen_string_literal: true
-require_relative 'title_linting'
+require 'gitlab/dangerfiles/title_linting'
module Tooling
module Danger
@@ -44,8 +44,8 @@ module Changelog
def required_reasons
[].tap do |reasons|
- reasons << :db_changes if helper.changes.added.has_category?(:migration)
- reasons << :feature_flag_removed if helper.changes.deleted.has_category?(:feature_flag)
+ reasons << :db_changes if project_helper.changes.added.has_category?(:migration)
+ reasons << :feature_flag_removed if project_helper.changes.deleted.has_category?(:feature_flag)
end
end
@@ -58,7 +58,7 @@ def optional?
end
def found
- @found ||= helper.changes.added.by_category(:changelog).files.first
+ @found ||= project_helper.changes.added.by_category(:changelog).files.first
end
def ee_changelog?
@@ -86,11 +86,11 @@ def optional_text
private
def sanitized_mr_title
- TitleLinting.sanitize_mr_title(helper.mr_title)
+ Gitlab::Dangerfiles::TitleLinting.sanitize_mr_title(helper.mr_title)
end
def categories_need_changelog?
- (helper.changes.categories - NO_CHANGELOG_CATEGORIES).any?
+ (project_helper.changes.categories - NO_CHANGELOG_CATEGORIES).any?
end
def without_no_changelog_label?
diff --git a/tooling/danger/commit_linter.rb b/tooling/danger/commit_linter.rb
deleted file mode 100644
index 905031ec881be..0000000000000
--- a/tooling/danger/commit_linter.rb
+++ /dev/null
@@ -1,150 +0,0 @@
-# frozen_string_literal: true
-
-require_relative 'base_linter'
-require_relative 'emoji_checker'
-
-module Tooling
- module Danger
- class CommitLinter < BaseLinter
- MAX_CHANGED_FILES_IN_COMMIT = 3
- MAX_CHANGED_LINES_IN_COMMIT = 30
- SHORT_REFERENCE_REGEX = %r{([\w\-\/]+)?(?<!`)(#|!|&|%)\d+(?<!`)}.freeze
-
- def self.problems_mapping
- super.merge(
- {
- separator_missing: "The commit subject and body must be separated by a blank line",
- details_too_many_changes: "Commits that change #{MAX_CHANGED_LINES_IN_COMMIT} or more lines across " \
- "at least #{MAX_CHANGED_FILES_IN_COMMIT} files must describe these changes in the commit body",
- details_line_too_long: "The commit body should not contain more than #{MAX_LINE_LENGTH} characters per line",
- message_contains_text_emoji: "Avoid the use of Markdown Emoji such as `:+1:`. These add limited value " \
- "to the commit message, and are displayed as plain text outside of GitLab",
- message_contains_unicode_emoji: "Avoid the use of Unicode Emoji. These add no value to the commit " \
- "message, and may not be displayed properly everywhere",
- message_contains_short_reference: "Use full URLs instead of short references (`gitlab-org/gitlab#123` or " \
- "`!123`), as short references are displayed as plain text outside of GitLab"
- }
- )
- end
-
- def initialize(commit)
- super
-
- @linted = false
- end
-
- def fixup?
- commit.message.start_with?('fixup!', 'squash!')
- end
-
- def suggestion?
- commit.message.start_with?('Apply suggestion to')
- end
-
- def merge?
- commit.message.start_with?('Merge branch')
- end
-
- def revert?
- commit.message.start_with?('Revert "')
- end
-
- def multi_line?
- !details.nil? && !details.empty?
- end
-
- def lint
- return self if @linted
-
- @linted = true
- lint_subject
- lint_separator
- lint_details
- lint_message
-
- self
- end
-
- private
-
- def lint_separator
- return self unless separator && !separator.empty?
-
- add_problem(:separator_missing)
-
- self
- end
-
- def lint_details
- if !multi_line? && many_changes?
- add_problem(:details_too_many_changes)
- end
-
- details&.each_line do |line|
- line_without_urls = line.strip.gsub(%r{https?://\S+}, '')
-
- # If the line includes a URL, we'll allow it to exceed MAX_LINE_LENGTH characters, but
- # only if the line _without_ the URL does not exceed this limit.
- next unless line_too_long?(line_without_urls)
-
- add_problem(:details_line_too_long)
- break
- end
-
- self
- end
-
- def lint_message
- if message_contains_text_emoji?
- add_problem(:message_contains_text_emoji)
- end
-
- if message_contains_unicode_emoji?
- add_problem(:message_contains_unicode_emoji)
- end
-
- if message_contains_short_reference?
- add_problem(:message_contains_short_reference)
- end
-
- self
- end
-
- def files_changed
- commit.diff_parent.stats[:total][:files]
- end
-
- def lines_changed
- commit.diff_parent.stats[:total][:lines]
- end
-
- def many_changes?
- files_changed > MAX_CHANGED_FILES_IN_COMMIT && lines_changed > MAX_CHANGED_LINES_IN_COMMIT
- end
-
- def separator
- message_parts[1]
- end
-
- def details
- message_parts[2]&.gsub(/^Signed-off-by.*$/, '')
- end
-
- def message_contains_text_emoji?
- emoji_checker.includes_text_emoji?(commit.message)
- end
-
- def message_contains_unicode_emoji?
- emoji_checker.includes_unicode_emoji?(commit.message)
- end
-
- def message_contains_short_reference?
- commit.message.match?(SHORT_REFERENCE_REGEX)
- end
-
- def emoji_checker
- @emoji_checker ||= Tooling::Danger::EmojiChecker.new
- end
- end
- end
-end
diff --git a/tooling/danger/emoji_checker.rb b/tooling/danger/emoji_checker.rb
deleted file mode 100644
index 9d8ff93037ccf..0000000000000
--- a/tooling/danger/emoji_checker.rb
+++ /dev/null
@@ -1,45 +0,0 @@
-# frozen_string_literal: true
-
-require 'json'
-
-module Tooling
- module Danger
- class EmojiChecker
- DIGESTS = File.expand_path('../../fixtures/emojis/digests.json', __dir__)
- ALIASES = File.expand_path('../../fixtures/emojis/aliases.json', __dir__)
-
- # A regex that indicates a piece of text _might_ include an Emoji. The regex
- # alone is not enough, as we'd match `:foo:bar:baz`. Instead, we use this
- # regex to save us from having to check for all possible emoji names when we
- # know one definitely is not included.
- LIKELY_EMOJI = /:[\+a-z0-9_\-]+:/.freeze
-
- UNICODE_EMOJI_REGEX = %r{(
- [\u{1F300}-\u{1F5FF}] |
- [\u{1F1E6}-\u{1F1FF}] |
- [\u{2700}-\u{27BF}] |
- [\u{1F900}-\u{1F9FF}] |
- [\u{1F600}-\u{1F64F}] |
- [\u{1F680}-\u{1F6FF}] |
- [\u{2600}-\u{26FF}]
- )}x.freeze
-
- def initialize
- names = JSON.parse(File.read(DIGESTS)).keys +
- JSON.parse(File.read(ALIASES)).keys
-
- @emoji = names.map { |name| ":#{name}:" }
- end
-
- def includes_text_emoji?(text)
- return false unless text.match?(LIKELY_EMOJI)
-
- @emoji.any? { |emoji| text.include?(emoji) }
- end
-
- def includes_unicode_emoji?(text)
- text.match?(UNICODE_EMOJI_REGEX)
- end
- end
- end
-end
diff --git a/tooling/danger/helper.rb b/tooling/danger/helper.rb
deleted file mode 100644
index ef5b2e16bb028..0000000000000
--- a/tooling/danger/helper.rb
+++ /dev/null
@@ -1,383 +0,0 @@
-# frozen_string_literal: true
-
-require 'delegate'
-
-require_relative 'teammate'
-require_relative 'title_linting'
-
-module Tooling
- module Danger
- module Helper
- RELEASE_TOOLS_BOT = 'gitlab-release-tools-bot'
-
- # Returns a list of all files that have been added, modified or renamed.
- # `git.modified_files` might contain paths that already have been renamed,
- # so we need to remove them from the list.
- #
- # Considering these changes:
- #
- # - A new_file.rb
- # - D deleted_file.rb
- # - M modified_file.rb
- # - R renamed_file_before.rb -> renamed_file_after.rb
- #
- # it will return
- # ```
- # [ 'new_file.rb', 'modified_file.rb', 'renamed_file_after.rb' ]
- # ```
- #
- # @return [Array<String>]
- def all_changed_files
- Set.new
- .merge(git.added_files.to_a)
- .merge(git.modified_files.to_a)
- .merge(git.renamed_files.map { |x| x[:after] })
- .subtract(git.renamed_files.map { |x| x[:before] })
- .to_a
- .sort
- end
-
- # Returns a string containing changed lines as git diff
- #
- # Considering changing a line in lib/gitlab/usage_data.rb it will return:
- #
- # [ "--- a/lib/gitlab/usage_data.rb",
- # "+++ b/lib/gitlab/usage_data.rb",
- # "+ # Test change",
- # "- # Old change" ]
- def changed_lines(changed_file)
- diff = git.diff_for_file(changed_file)
- return [] unless diff
-
- diff.patch.split("\n").select { |line| %r{^[+-]}.match?(line) }
- end
-
- def all_ee_changes
- all_changed_files.grep(%r{\Aee/})
- end
-
- def ee?
- # Support former project name for `dev` and support local Danger run
- %w[gitlab gitlab-ee].include?(ENV['CI_PROJECT_NAME']) || Dir.exist?(File.expand_path('../../../ee', __dir__))
- end
-
- def gitlab_helper
- # Unfortunately the following does not work:
- # - respond_to?(:gitlab)
- # - respond_to?(:gitlab, true)
- gitlab
- rescue NameError
- nil
- end
-
- def release_automation?
- gitlab_helper&.mr_author == RELEASE_TOOLS_BOT
- end
-
- def project_name
- ee? ? 'gitlab' : 'gitlab-foss'
- end
-
- def markdown_list(items)
- list = items.map { |item| "* `#{item}`" }.join("\n")
-
- if items.size > 10
- "\n<details>\n\n#{list}\n\n</details>\n"
- else
- list
- end
- end
-
- Change = Struct.new(:file, :change_type, :category)
-
- class Changes < ::SimpleDelegator
- def added
- select_by_change_type(:added)
- end
-
- def modified
- select_by_change_type(:modified)
- end
-
- def deleted
- select_by_change_type(:deleted)
- end
-
- def renamed_before
- select_by_change_type(:renamed_before)
- end
-
- def renamed_after
- select_by_change_type(:renamed_after)
- end
-
- def has_category?(category)
- any? { |change| change.category == category }
- end
-
- def by_category(category)
- Changes.new(select { |change| change.category == category })
- end
-
- def categories
- map(&:category).uniq
- end
-
- def files
- map(&:file)
- end
-
- private
-
- def select_by_change_type(change_type)
- Changes.new(select { |change| change.change_type == change_type })
- end
- end
-
- # @return [Hash<Symbol,Array<String>>]
- def changes_by_category
- all_changed_files.each_with_object(Hash.new { |h, k| h[k] = [] }) do |file, hash|
- categories_for_file(file).each { |category| hash[category] << file }
- end
- end
-
- # @return [Changes]
- def changes
- Changes.new([]).tap do |changes|
- git.added_files.each do |file|
- categories_for_file(file).each { |category| changes << Change.new(file, :added, category) }
- end
-
- git.modified_files.each do |file|
- categories_for_file(file).each { |category| changes << Change.new(file, :modified, category) }
- end
-
- git.deleted_files.each do |file|
- categories_for_file(file).each { |category| changes << Change.new(file, :deleted, category) }
- end
-
- git.renamed_files.map { |x| x[:before] }.each do |file|
- categories_for_file(file).each { |category| changes << Change.new(file, :renamed_before, category) }
- end
-
- git.renamed_files.map { |x| x[:after] }.each do |file|
- categories_for_file(file).each { |category| changes << Change.new(file, :renamed_after, category) }
- end
- end
- end
-
- # Determines the categories a file is in, e.g., `[:frontend]`, `[:backend]`, or `%i[frontend engineering_productivity]`
- # using filename regex and specific change regex if given.
- #
- # @return Array<Symbol>
- def categories_for_file(file)
- _, categories = CATEGORIES.find do |key, _|
- filename_regex, changes_regex = Array(key)
-
- found = filename_regex.match?(file)
- found &&= changed_lines(file).any? { |changed_line| changes_regex.match?(changed_line) } if changes_regex
-
- found
- end
-
- Array(categories || :unknown)
- end
-
- # Returns the GFM for a category label, making its best guess if it's not
- # a category we know about.
- #
- # @return[String]
- def label_for_category(category)
- CATEGORY_LABELS.fetch(category, "~#{category}")
- end
-
- CATEGORY_LABELS = {
- docs: "~documentation", # Docs are reviewed along DevOps stages, so don't need roulette for now.
- none: "",
- qa: "~QA",
- test: "~test ~Quality for `spec/features/*`",
- engineering_productivity: '~"Engineering Productivity" for CI, Danger',
- ci_template: '~"ci::templates"'
- }.freeze
- # 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],
-
- %r{\A(ee/)?config/feature_flags/} => :feature_flag,
-
- %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} => :changelog,
-
- %r{\Adoc/.*(\.(md|png|gif|jpg))\z} => :docs,
- %r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :docs,
- %r{\Adata/whats_new/} => :docs,
-
- %r{\A(ee/)?app/(assets|views)/} => :frontend,
- %r{\A(ee/)?public/} => :frontend,
- %r{\A(ee/)?spec/(javascripts|frontend)/} => :frontend,
- %r{\A(ee/)?vendor/assets/} => :frontend,
- %r{\A(ee/)?scripts/frontend/} => :frontend,
- %r{(\A|/)(
- \.babelrc |
- \.eslintignore |
- \.eslintrc(\.yml)? |
- \.nvmrc |
- \.prettierignore |
- \.prettierrc |
- \.stylelintrc |
- \.haml-lint.yml |
- \.haml-lint_todo.yml |
- babel\.config\.js |
- jest\.config\.js |
- package\.json |
- yarn\.lock |
- config/.+\.js
- )\z}x => :frontend,
-
- %r{(\A|/)(
- \.gitlab/ci/frontend\.gitlab-ci\.yml
- )\z}x => %i[frontend engineering_productivity],
-
- %r{\A(ee/)?db/(geo/)?(migrate|post_migrate)/} => [:database, :migration],
- %r{\A(ee/)?db/(?!fixtures)[^/]+} => :database,
- %r{\A(ee/)?lib/gitlab/(database|background_migration|sql|github_import)(/|\.rb)} => :database,
- %r{\A(app/models/project_authorization|app/services/users/refresh_authorized_projects_service)(/|\.rb)} => :database,
- %r{\A(ee/)?app/finders/} => :database,
- %r{\Arubocop/cop/migration(/|\.rb)} => :database,
-
- %r{\A(\.gitlab-ci\.yml\z|\.gitlab\/ci)} => :engineering_productivity,
- %r{\A\.codeclimate\.yml\z} => :engineering_productivity,
- %r{\Alefthook.yml\z} => :engineering_productivity,
- %r{\A\.editorconfig\z} => :engineering_productivity,
- %r{Dangerfile\z} => :engineering_productivity,
- %r{\A(ee/)?(danger/|tooling/danger/)} => :engineering_productivity,
- %r{\A(ee/)?scripts/} => :engineering_productivity,
- %r{\Atooling/} => :engineering_productivity,
- %r{(CODEOWNERS)} => :engineering_productivity,
- %r{(tests.yml)} => :engineering_productivity,
-
- %r{\Alib/gitlab/ci/templates} => :ci_template,
-
- %r{\A(ee/)?spec/features/} => :test,
- %r{\A(ee/)?spec/support/shared_examples/features/} => :test,
- %r{\A(ee/)?spec/support/shared_contexts/features/} => :test,
- %r{\A(ee/)?spec/support/helpers/features/} => :test,
-
- %r{\A(ee/)?app/(?!assets|views)[^/]+} => :backend,
- %r{\A(ee/)?(bin|config|generator_templates|lib|rubocop)/} => :backend,
- %r{\A(ee/)?spec/} => :backend,
- %r{\A(ee/)?vendor/} => :backend,
- %r{\A(Gemfile|Gemfile.lock|Rakefile)\z} => :backend,
- %r{\A[A-Z_]+_VERSION\z} => :backend,
- %r{\A\.rubocop((_manual)?_todo)?\.yml\z} => :backend,
- %r{\Afile_hooks/} => :backend,
-
- %r{\A(ee/)?qa/} => :qa,
-
- # Files that don't fit into any category are marked with :none
- %r{\A(ee/)?changelogs/} => :none,
- %r{\Alocale/gitlab\.pot\z} => :none,
-
- # GraphQL auto generated doc files and schema
- %r{\Adoc/api/graphql/reference/} => :backend,
-
- # Fallbacks in case the above patterns miss anything
- %r{\.rb\z} => :backend,
- %r{(
- \.(md|txt)\z |
- \.markdownlint\.json
- )}x => :none, # To reinstate roulette for documentation, set to `:docs`.
- %r{\.js\z} => :frontend
- }.freeze
-
- def new_teammates(usernames)
- usernames.map { |u| Tooling::Danger::Teammate.new('username' => u) }
- end
-
- def mr_iid
- return '' unless gitlab_helper
-
- gitlab_helper.mr_json['iid']
- end
-
- def mr_title
- return '' unless gitlab_helper
-
- gitlab_helper.mr_json['title']
- end
-
- def mr_web_url
- return '' unless gitlab_helper
-
- gitlab_helper.mr_json['web_url']
- end
-
- def mr_labels
- return [] unless gitlab_helper
-
- gitlab_helper.mr_labels
- end
-
- def mr_target_branch
- return '' unless gitlab_helper
-
- gitlab_helper.mr_json['target_branch']
- end
-
- def draft_mr?
- TitleLinting.has_draft_flag?(mr_title)
- end
-
- def security_mr?
- mr_web_url.include?('/gitlab-org/security/')
- end
-
- def cherry_pick_mr?
- TitleLinting.has_cherry_pick_flag?(mr_title)
- end
-
- def run_all_rspec_mr?
- TitleLinting.has_run_all_rspec_flag?(mr_title)
- end
-
- def run_as_if_foss_mr?
- TitleLinting.has_run_as_if_foss_flag?(mr_title)
- end
-
- def stable_branch?
- /\A\d+-\d+-stable-ee/i.match?(mr_target_branch)
- end
-
- def mr_has_labels?(*labels)
- labels = labels.flatten.uniq
-
- (labels & mr_labels) == labels
- end
-
- def labels_list(labels, sep: ', ')
- labels.map { |label| %Q{~"#{label}"} }.join(sep)
- end
-
- def prepare_labels_for_mr(labels)
- return '' unless labels.any?
-
- "/label #{labels_list(labels, sep: ' ')}"
- end
-
- def changed_files(regex)
- all_changed_files.grep(regex)
- end
-
- def has_database_scoped_labels?(labels)
- labels.any? { |label| label.start_with?('database::') }
- end
-
- def has_ci_changes?
- changed_files(%r{\A(\.gitlab-ci\.yml|\.gitlab/ci/)}).any?
- end
-
- def group_label(labels)
- labels.find { |label| label.start_with?('group::') }
- end
- end
- end
-end
diff --git a/tooling/danger/merge_request_linter.rb b/tooling/danger/merge_request_linter.rb
deleted file mode 100644
index ddeb9cc2981b9..0000000000000
--- a/tooling/danger/merge_request_linter.rb
+++ /dev/null
@@ -1,30 +0,0 @@
-# frozen_string_literal: true
-
-require_relative 'base_linter'
-
-module Tooling
- module Danger
- class MergeRequestLinter < BaseLinter
- alias_method :lint, :lint_subject
-
- def self.subject_description
- 'merge request title'
- end
-
- def self.mr_run_options_regex
- [
- 'RUN AS-IF-FOSS',
- 'UPDATE CACHE',
- 'RUN ALL RSPEC',
- 'SKIP RSPEC FAIL-FAST'
- ].join('|')
- end
-
- private
-
- def subject
- super.gsub(/\[?(#{self.class.mr_run_options_regex})\]?/, '').strip
- end
- end
- end
-end
diff --git a/tooling/danger/project_helper.rb b/tooling/danger/project_helper.rb
new file mode 100644
index 0000000000000..342bd8422ad5a
--- /dev/null
+++ b/tooling/danger/project_helper.rb
@@ -0,0 +1,181 @@
+# frozen_string_literal: true
+
+module Tooling
+ module Danger
+ module ProjectHelper
+ LOCAL_RULES ||= %w[
+ changes_size
+ commit_messages
+ database
+ documentation
+ duplicate_yarn_dependencies
+ eslint
+ karma
+ pajamas
+ pipeline
+ prettier
+ product_intelligence
+ utility_css
+ ].freeze
+
+ CI_ONLY_RULES ||= %w[
+ ce_ee_vue_templates
+ changelog
+ ci_templates
+ metadata
+ feature_flag
+ roulette
+ sidekiq_queues
+ specialization_labels
+ specs
+ ].freeze
+
+ MESSAGE_PREFIX = '==>'.freeze
+
+ # 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],
+
+ %r{\A(ee/)?config/feature_flags/} => :feature_flag,
+
+ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} => :changelog,
+
+ %r{\Adoc/.*(\.(md|png|gif|jpg))\z} => :docs,
+ %r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :docs,
+ %r{\Adata/whats_new/} => :docs,
+
+ %r{\A(ee/)?app/(assets|views)/} => :frontend,
+ %r{\A(ee/)?public/} => :frontend,
+ %r{\A(ee/)?spec/(javascripts|frontend)/} => :frontend,
+ %r{\A(ee/)?vendor/assets/} => :frontend,
+ %r{\A(ee/)?scripts/frontend/} => :frontend,
+ %r{(\A|/)(
+ \.babelrc |
+ \.eslintignore |
+ \.eslintrc(\.yml)? |
+ \.nvmrc |
+ \.prettierignore |
+ \.prettierrc |
+ \.stylelintrc |
+ \.haml-lint.yml |
+ \.haml-lint_todo.yml |
+ babel\.config\.js |
+ jest\.config\.js |
+ package\.json |
+ yarn\.lock |
+ config/.+\.js
+ )\z}x => :frontend,
+
+ %r{(\A|/)(
+ \.gitlab/ci/frontend\.gitlab-ci\.yml
+ )\z}x => %i[frontend engineering_productivity],
+
+ %r{\A(ee/)?db/(geo/)?(migrate|post_migrate)/} => [:database, :migration],
+ %r{\A(ee/)?db/(?!fixtures)[^/]+} => :database,
+ %r{\A(ee/)?lib/gitlab/(database|background_migration|sql|github_import)(/|\.rb)} => :database,
+ %r{\A(app/models/project_authorization|app/services/users/refresh_authorized_projects_service)(/|\.rb)} => :database,
+ %r{\A(ee/)?app/finders/} => :database,
+ %r{\Arubocop/cop/migration(/|\.rb)} => :database,
+
+ %r{\A(\.gitlab-ci\.yml\z|\.gitlab\/ci)} => :engineering_productivity,
+ %r{\A\.codeclimate\.yml\z} => :engineering_productivity,
+ %r{\Alefthook.yml\z} => :engineering_productivity,
+ %r{\A\.editorconfig\z} => :engineering_productivity,
+ %r{Dangerfile\z} => :engineering_productivity,
+ %r{\A(ee/)?(danger/|tooling/danger/)} => :engineering_productivity,
+ %r{\A(ee/)?scripts/} => :engineering_productivity,
+ %r{\Atooling/} => :engineering_productivity,
+ %r{(CODEOWNERS)} => :engineering_productivity,
+ %r{(tests.yml)} => :engineering_productivity,
+
+ %r{\Alib/gitlab/ci/templates} => :ci_template,
+
+ %r{\A(ee/)?spec/features/} => :test,
+ %r{\A(ee/)?spec/support/shared_examples/features/} => :test,
+ %r{\A(ee/)?spec/support/shared_contexts/features/} => :test,
+ %r{\A(ee/)?spec/support/helpers/features/} => :test,
+
+ %r{\A(ee/)?app/(?!assets|views)[^/]+} => :backend,
+ %r{\A(ee/)?(bin|config|generator_templates|lib|rubocop)/} => :backend,
+ %r{\A(ee/)?spec/} => :backend,
+ %r{\A(ee/)?vendor/} => :backend,
+ %r{\A(Gemfile|Gemfile.lock|Rakefile)\z} => :backend,
+ %r{\A[A-Z_]+_VERSION\z} => :backend,
+ %r{\A\.rubocop((_manual)?_todo)?\.yml\z} => :backend,
+ %r{\Afile_hooks/} => :backend,
+
+ %r{\A(ee/)?qa/} => :qa,
+
+ # Files that don't fit into any category are marked with :none
+ %r{\A(ee/)?changelogs/} => :none,
+ %r{\Alocale/gitlab\.pot\z} => :none,
+
+ # GraphQL auto generated doc files and schema
+ %r{\Adoc/api/graphql/reference/} => :backend,
+
+ # Fallbacks in case the above patterns miss anything
+ %r{\.rb\z} => :backend,
+ %r{(
+ \.(md|txt)\z |
+ \.markdownlint\.json
+ )}x => :none, # To reinstate roulette for documentation, set to `:docs`.
+ %r{\.js\z} => :frontend
+ }.freeze
+
+ def changes_by_category
+ helper.changes_by_category(CATEGORIES)
+ end
+
+ def changes
+ helper.changes(CATEGORIES)
+ end
+
+ def categories_for_file(file)
+ helper.categories_for_file(file, CATEGORIES)
+ end
+
+ 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 all_ee_changes
+ all_changed_files.grep(%r{\Aee/})
+ end
+
+ def project_name
+ ee? ? 'gitlab' : 'gitlab-foss'
+ end
+
+ def missing_database_labels(current_mr_labels)
+ labels = if has_database_scoped_labels?(current_mr_labels)
+ ['database']
+ else
+ ['database', 'database::review pending']
+ end
+
+ labels - current_mr_labels
+ end
+
+ private
+
+ def ee?
+ # Support former project name for `dev` and support local Danger run
+ %w[gitlab gitlab-ee].include?(ENV['CI_PROJECT_NAME']) || Dir.exist?(File.expand_path('../../../ee', __dir__))
+ end
+
+ def has_database_scoped_labels?(current_mr_labels)
+ current_mr_labels.any? { |label| label.start_with?('database::') }
+ end
+ end
+ end
+end
diff --git a/tooling/danger/request_helper.rb b/tooling/danger/request_helper.rb
deleted file mode 100644
index d6b99f562f92e..0000000000000
--- a/tooling/danger/request_helper.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-# frozen_string_literal: true
-
-require 'net/http'
-require 'json'
-
-module Tooling
- module Danger
- module RequestHelper
- HTTPError = Class.new(RuntimeError)
-
- # @param [String] url
- def self.http_get_json(url)
- rsp = Net::HTTP.get_response(URI.parse(url))
-
- unless rsp.is_a?(Net::HTTPOK)
- raise HTTPError, "Failed to read #{url}: #{rsp.code} #{rsp.message}"
- end
-
- JSON.parse(rsp.body)
- end
- end
- end
-end
diff --git a/tooling/danger/roulette.rb b/tooling/danger/roulette.rb
deleted file mode 100644
index c928fb2b6554f..0000000000000
--- a/tooling/danger/roulette.rb
+++ /dev/null
@@ -1,169 +0,0 @@
-# frozen_string_literal: true
-
-require_relative 'teammate'
-require_relative 'request_helper'
-require_relative 'weightage/reviewers'
-require_relative 'weightage/maintainers'
-
-module Tooling
- module Danger
- module Roulette
- ROULETTE_DATA_URL = 'https://gitlab-org.gitlab.io/gitlab-roulette/roulette.json'
- HOURS_WHEN_PERSON_CAN_BE_PICKED = (6..14).freeze
-
- INCLUDE_TIMEZONE_FOR_CATEGORY = {
- database: false
- }.freeze
-
- Spin = Struct.new(:category, :reviewer, :maintainer, :optional_role, :timezone_experiment)
-
- def team_mr_author
- team.find { |person| person.username == mr_author_username }
- end
-
- # Assigns GitLab team members to be reviewer and maintainer
- # for each change category that a Merge Request contains.
- #
- # @return [Array<Spin>]
- def spin(project, categories, timezone_experiment: false)
- spins = categories.sort.map do |category|
- including_timezone = INCLUDE_TIMEZONE_FOR_CATEGORY.fetch(category, timezone_experiment)
-
- spin_for_category(project, category, timezone_experiment: including_timezone)
- end
-
- backend_spin = spins.find { |spin| spin.category == :backend }
-
- spins.each do |spin|
- including_timezone = INCLUDE_TIMEZONE_FOR_CATEGORY.fetch(spin.category, timezone_experiment)
- case spin.category
- when :qa
- # MR includes QA changes, but also other changes, and author isn't an SET
- if categories.size > 1 && !team_mr_author&.any_capability?(project, spin.category)
- spin.optional_role = :maintainer
- end
- when :test
- spin.optional_role = :maintainer
-
- if spin.reviewer.nil?
- # Fetch an already picked backend reviewer, or pick one otherwise
- spin.reviewer = backend_spin&.reviewer || spin_for_category(project, :backend, timezone_experiment: including_timezone).reviewer
- end
- when :engineering_productivity
- if spin.maintainer.nil?
- # Fetch an already picked backend maintainer, or pick one otherwise
- spin.maintainer = backend_spin&.maintainer || spin_for_category(project, :backend, timezone_experiment: including_timezone).maintainer
- end
- when :ci_template
- if spin.maintainer.nil?
- # Fetch an already picked backend maintainer, or pick one otherwise
- spin.maintainer = backend_spin&.maintainer || spin_for_category(project, :backend, timezone_experiment: including_timezone).maintainer
- end
- end
- end
-
- spins
- end
-
- # Looks up the current list of GitLab team members and parses it into a
- # useful form
- #
- # @return [Array<Teammate>]
- def team
- @team ||=
- begin
- data = Tooling::Danger::RequestHelper.http_get_json(ROULETTE_DATA_URL)
- data.map { |hash| ::Tooling::Danger::Teammate.new(hash) }
- rescue JSON::ParserError
- raise "Failed to parse JSON response from #{ROULETTE_DATA_URL}"
- end
- end
-
- # Like +team+, but only returns teammates in the current project, based on
- # project_name.
- #
- # @return [Array<Teammate>]
- def project_team(project_name)
- team.select { |member| member.in_project?(project_name) }
- rescue => err
- warn("Reviewer roulette failed to load team data: #{err.message}")
- []
- end
-
- # Known issue: If someone is rejected due to OOO, and then becomes not OOO, the
- # selection will change on next spin
- # @param [Array<Teammate>] people
- def spin_for_person(people, random:, timezone_experiment: false)
- shuffled_people = people.shuffle(random: random)
-
- if timezone_experiment
- shuffled_people.find(&method(:valid_person_with_timezone?))
- else
- shuffled_people.find(&method(:valid_person?))
- end
- end
-
- private
-
- # @param [Teammate] person
- # @return [Boolean]
- def valid_person?(person)
- !mr_author?(person) && person.available
- end
-
- # @param [Teammate] person
- # @return [Boolean]
- def valid_person_with_timezone?(person)
- valid_person?(person) && HOURS_WHEN_PERSON_CAN_BE_PICKED.cover?(person.local_hour)
- end
-
- # @param [Teammate] person
- # @return [Boolean]
- def mr_author?(person)
- person.username == mr_author_username
- end
-
- def mr_author_username
- helper.gitlab_helper&.mr_author || `whoami`
- end
-
- def mr_source_branch
- return `git rev-parse --abbrev-ref HEAD` unless helper.gitlab_helper&.mr_json
-
- helper.gitlab_helper.mr_json['source_branch']
- end
-
- def mr_labels
- helper.gitlab_helper&.mr_labels || []
- end
-
- def new_random(seed)
- Random.new(Digest::MD5.hexdigest(seed).to_i(16))
- end
-
- def spin_role_for_category(team, role, project, category)
- team.select do |member|
- member.public_send("#{role}?", project, category, mr_labels) # rubocop:disable GitlabSecurity/PublicSend
- end
- end
-
- def spin_for_category(project, category, timezone_experiment: false)
- team = project_team(project)
- reviewers, traintainers, maintainers =
- %i[reviewer traintainer maintainer].map do |role|
- spin_role_for_category(team, role, project, category)
- end
-
- random = new_random(mr_source_branch)
-
- weighted_reviewers = Weightage::Reviewers.new(reviewers, traintainers).execute
- weighted_maintainers = Weightage::Maintainers.new(maintainers).execute
-
- reviewer = spin_for_person(weighted_reviewers, random: random, timezone_experiment: timezone_experiment)
- maintainer = spin_for_person(weighted_maintainers, random: random, timezone_experiment: timezone_experiment)
-
- Spin.new(category, reviewer, maintainer, false, timezone_experiment)
- end
- end
- end
-end
diff --git a/tooling/danger/teammate.rb b/tooling/danger/teammate.rb
deleted file mode 100644
index bcd33bebdc962..0000000000000
--- a/tooling/danger/teammate.rb
+++ /dev/null
@@ -1,121 +0,0 @@
-# frozen_string_literal: true
-
-module Tooling
- module Danger
- class Teammate
- attr_reader :options, :username, :name, :role, :projects, :available, :hungry, :reduced_capacity, :tz_offset_hours
-
- # The options data are produced by https://gitlab.com/gitlab-org/gitlab-roulette/-/blob/master/lib/team_member.rb
- def initialize(options = {})
- @options = options
- @username = options['username']
- @name = options['name']
- @markdown_name = options['markdown_name']
- @role = options['role']
- @projects = options['projects']
- @available = options['available']
- @hungry = options['hungry']
- @reduced_capacity = options['reduced_capacity']
- @tz_offset_hours = options['tz_offset_hours']
- end
-
- def to_h
- options
- end
-
- def ==(other)
- return false unless other.respond_to?(:username)
-
- other.username == username
- end
-
- def in_project?(name)
- projects&.has_key?(name)
- end
-
- def any_capability?(project, category)
- capabilities(project).any? { |capability| capability.end_with?(category.to_s) }
- end
-
- def reviewer?(project, category, labels)
- has_capability?(project, category, :reviewer, labels)
- end
-
- def traintainer?(project, category, labels)
- has_capability?(project, category, :trainee_maintainer, labels)
- end
-
- def maintainer?(project, category, labels)
- has_capability?(project, category, :maintainer, labels)
- end
-
- def markdown_name(author: nil)
- "#{@markdown_name} (#{utc_offset_text(author)})"
- end
-
- def local_hour
- (Time.now.utc + tz_offset_hours * 3600).hour
- end
-
- protected
-
- def floored_offset_hours
- floored_offset = tz_offset_hours.floor(0)
-
- floored_offset == tz_offset_hours ? floored_offset : tz_offset_hours
- end
-
- private
-
- def utc_offset_text(author = nil)
- offset_text =
- if floored_offset_hours >= 0
- "UTC+#{floored_offset_hours}"
- else
- "UTC#{floored_offset_hours}"
- end
-
- return offset_text unless author
-
- "#{offset_text}, #{offset_diff_compared_to_author(author)}"
- end
-
- def offset_diff_compared_to_author(author)
- diff = floored_offset_hours - author.floored_offset_hours
- return "same timezone as `@#{author.username}`" if diff == 0
-
- ahead_or_behind = diff < 0 ? 'behind' : 'ahead of'
- pluralized_hours = pluralize(diff.abs, 'hour', 'hours')
-
- "#{pluralized_hours} #{ahead_or_behind} `@#{author.username}`"
- end
-
- def has_capability?(project, category, kind, labels)
- case category
- when :test
- area = role[/Software Engineer in Test(?:.*?, (\w+))/, 1]
-
- area && labels.any?("devops::#{area.downcase}") if kind == :reviewer
- when :engineering_productivity
- return false unless role[/Engineering Productivity/]
- return true if kind == :reviewer
- return true if capabilities(project).include?("#{kind} engineering_productivity")
-
- capabilities(project).include?("#{kind} backend")
- else
- capabilities(project).include?("#{kind} #{category}")
- end
- end
-
- def capabilities(project)
- Array(projects.fetch(project, []))
- end
-
- def pluralize(count, singular, plural)
- word = count == 1 || count.to_s =~ /^1(\.0+)?$/ ? singular : plural
-
- "#{count || 0} #{word}"
- end
- end
- end
-end
diff --git a/tooling/danger/title_linting.rb b/tooling/danger/title_linting.rb
deleted file mode 100644
index dcd83df7d93a1..0000000000000
--- a/tooling/danger/title_linting.rb
+++ /dev/null
@@ -1,38 +0,0 @@
-# frozen_string_literal: true
-
-module Tooling
- module Danger
- module TitleLinting
- DRAFT_REGEX = /\A*#{Regexp.union(/(?i)(\[WIP\]\s*|WIP:\s*|WIP$)/, /(?i)(\[draft\]|\(draft\)|draft:|draft\s\-\s|draft$)/)}+\s*/i.freeze
- CHERRY_PICK_REGEX = /cherry[\s-]*pick/i.freeze
- RUN_ALL_RSPEC_REGEX = /RUN ALL RSPEC/i.freeze
- RUN_AS_IF_FOSS_REGEX = /RUN AS-IF-FOSS/i.freeze
-
- module_function
-
- def sanitize_mr_title(title)
- remove_draft_flag(title).gsub(/`/, '\\\`')
- end
-
- def remove_draft_flag(title)
- title.gsub(DRAFT_REGEX, '')
- end
-
- def has_draft_flag?(title)
- DRAFT_REGEX.match?(title)
- end
-
- def has_cherry_pick_flag?(title)
- CHERRY_PICK_REGEX.match?(title)
- end
-
- def has_run_all_rspec_flag?(title)
- RUN_ALL_RSPEC_REGEX.match?(title)
- end
-
- def has_run_as_if_foss_flag?(title)
- RUN_AS_IF_FOSS_REGEX.match?(title)
- end
- end
- end
-end
diff --git a/tooling/danger/weightage.rb b/tooling/danger/weightage.rb
deleted file mode 100644
index cf8d17410dca2..0000000000000
--- a/tooling/danger/weightage.rb
+++ /dev/null
@@ -1,10 +0,0 @@
-# frozen_string_literal: true
-
-module Tooling
- module Danger
- module Weightage
- CAPACITY_MULTIPLIER = 2 # change this number to change what it means to be a reduced capacity reviewer 1/this number
- BASE_REVIEWER_WEIGHT = 1
- end
- end
-end
diff --git a/tooling/danger/weightage/maintainers.rb b/tooling/danger/weightage/maintainers.rb
deleted file mode 100644
index 068b24e79138e..0000000000000
--- a/tooling/danger/weightage/maintainers.rb
+++ /dev/null
@@ -1,33 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../weightage'
-
-module Tooling
- module Danger
- module Weightage
- class Maintainers
- def initialize(maintainers)
- @maintainers = maintainers
- end
-
- def execute
- maintainers.each_with_object([]) do |maintainer, weighted_maintainers|
- add_weighted_reviewer(weighted_maintainers, maintainer, BASE_REVIEWER_WEIGHT)
- end
- end
-
- private
-
- attr_reader :maintainers
-
- def add_weighted_reviewer(reviewers, reviewer, weight)
- if reviewer.reduced_capacity
- reviewers.fill(reviewer, reviewers.size, weight)
- else
- reviewers.fill(reviewer, reviewers.size, weight * CAPACITY_MULTIPLIER)
- end
- end
- end
- end
- end
-end
diff --git a/tooling/danger/weightage/reviewers.rb b/tooling/danger/weightage/reviewers.rb
deleted file mode 100644
index e74fce3718721..0000000000000
--- a/tooling/danger/weightage/reviewers.rb
+++ /dev/null
@@ -1,65 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../weightage'
-
-module Tooling
- module Danger
- module Weightage
- # Weights after (current multiplier of 2)
- #
- # +------------------------------+--------------------------------+
- # | reviewer type | weight(times in reviewer pool) |
- # +------------------------------+--------------------------------+
- # | reduced capacity reviewer | 1 |
- # | reviewer | 2 |
- # | hungry reviewer | 4 |
- # | reduced capacity traintainer | 3 |
- # | traintainer | 6 |
- # | hungry traintainer | 8 |
- # +------------------------------+--------------------------------+
- #
- class Reviewers
- DEFAULT_REVIEWER_WEIGHT = CAPACITY_MULTIPLIER * BASE_REVIEWER_WEIGHT
- TRAINTAINER_WEIGHT = 3
-
- def initialize(reviewers, traintainers)
- @reviewers = reviewers
- @traintainers = traintainers
- end
-
- def execute
- # TODO: take CODEOWNERS into account?
- # https://gitlab.com/gitlab-org/gitlab/issues/26723
-
- weighted_reviewers + weighted_traintainers
- end
-
- private
-
- attr_reader :reviewers, :traintainers
-
- def weighted_reviewers
- reviewers.each_with_object([]) do |reviewer, total_reviewers|
- add_weighted_reviewer(total_reviewers, reviewer, BASE_REVIEWER_WEIGHT)
- end
- end
-
- def weighted_traintainers
- traintainers.each_with_object([]) do |reviewer, total_traintainers|
- add_weighted_reviewer(total_traintainers, reviewer, TRAINTAINER_WEIGHT)
- end
- end
-
- def add_weighted_reviewer(reviewers, reviewer, weight)
- if reviewer.reduced_capacity
- reviewers.fill(reviewer, reviewers.size, weight)
- elsif reviewer.hungry
- reviewers.fill(reviewer, reviewers.size, weight * CAPACITY_MULTIPLIER + DEFAULT_REVIEWER_WEIGHT)
- else
- reviewers.fill(reviewer, reviewers.size, weight * CAPACITY_MULTIPLIER)
- end
- end
- end
- end
- end
-end
diff --git a/tooling/gitlab_danger.rb b/tooling/gitlab_danger.rb
deleted file mode 100644
index d20d349964177..0000000000000
--- a/tooling/gitlab_danger.rb
+++ /dev/null
@@ -1,59 +0,0 @@
-# frozen_string_literal: true
-
-# rubocop:todo Gitlab/NamespacedClass
-class GitlabDanger
- LOCAL_RULES ||= %w[
- changes_size
- commit_messages
- database
- documentation
- duplicate_yarn_dependencies
- eslint
- karma
- pajamas
- pipeline
- prettier
- product_intelligence
- utility_css
- ].freeze
-
- CI_ONLY_RULES ||= %w[
- ce_ee_vue_templates
- changelog
- ci_templates
- metadata
- feature_flag
- roulette
- sidekiq_queues
- specialization_labels
- specs
- ].freeze
-
- MESSAGE_PREFIX = '==>'.freeze
-
- attr_reader :gitlab_danger_helper
-
- def initialize(gitlab_danger_helper)
- @gitlab_danger_helper = gitlab_danger_helper
- end
-
- def self.local_warning_message
- "#{MESSAGE_PREFIX} Only the following Danger rules can be run locally: #{LOCAL_RULES.join(', ')}"
- end
-
- def self.success_message
- "#{MESSAGE_PREFIX} No Danger rule violations!"
- end
-
- def rule_names
- ci? ? LOCAL_RULES | CI_ONLY_RULES : LOCAL_RULES
- end
-
- def html_link(str)
- self.ci? ? gitlab_danger_helper.html_link(str) : str
- end
-
- def ci?
- !gitlab_danger_helper.nil?
- end
-end
--
GitLab