Commit 95a3801f authored by Rémy Coutable's avatar Rémy Coutable Committed by Lin Jen-Shin

Use the gitlab-dangerfiles gem

The gem provides helpers that can be reused by multiple projects.
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 587851e5
...@@ -49,6 +49,14 @@ ...@@ -49,6 +49,14 @@
- vendor/ruby/ - vendor/ruby/
policy: pull policy: pull
.danger-review-cache:
cache:
key: "danger-review-v1"
paths:
- vendor/ruby/
- node_modules/
policy: pull
.qa-cache: .qa-cache:
cache: cache:
key: "qa-v2" key: "qa-v2"
......
...@@ -225,12 +225,22 @@ parallel-spec-reports: ...@@ -225,12 +225,22 @@ parallel-spec-reports:
danger-review: danger-review:
extends: extends:
- .default-retry - .default-retry
- .yarn-cache - .danger-review-cache
- .review:rules:danger - .review:rules:danger
image: registry.gitlab.com/gitlab-org/gitlab-build-images:danger
stage: test stage: test
needs: [] needs: []
script: before_script:
- source ./scripts/utils.sh - 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" - 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.
# frozen_string_literal: true # frozen_string_literal: true
require_relative 'tooling/gitlab_danger' require 'gitlab-dangerfiles'
require_relative 'tooling/danger/request_helper'
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? return if helper.release_automation?
gitlab_danger = GitlabDanger.new(helper.gitlab_helper) project_helper.rule_names.each do |rule|
danger.import_dangerfile(path: File.join('danger', rule))
gitlab_danger.rule_names.each do |file|
danger.import_dangerfile(path: File.join('danger', file))
end end
anything_to_post = status_report.values.any? { |data| data.any? } 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.**") markdown("**If needed, you can retry the [`danger-review` job](#{ENV['CI_JOB_URL']}) that generated this comment.**")
end end
...@@ -344,7 +344,6 @@ end ...@@ -344,7 +344,6 @@ end
group :development do group :development do
gem 'brakeman', '~> 4.2', require: false gem 'brakeman', '~> 4.2', require: false
gem 'danger', '~> 8.0.6', require: false
gem 'lefthook', '~> 0.7', require: false gem 'lefthook', '~> 0.7', require: false
gem 'letter_opener_web', '~> 1.3.4' gem 'letter_opener_web', '~> 1.3.4'
...@@ -399,6 +398,11 @@ group :development, :test do ...@@ -399,6 +398,11 @@ group :development, :test do
gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false
end 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 group :development, :test, :coverage do
gem 'simplecov', '~> 0.18.5', require: false gem 'simplecov', '~> 0.18.5', require: false
gem 'simplecov-cobertura', '~> 1.3.1', require: false gem 'simplecov-cobertura', '~> 1.3.1', require: false
......
...@@ -216,7 +216,7 @@ GEM ...@@ -216,7 +216,7 @@ GEM
css_parser (1.7.0) css_parser (1.7.0)
addressable addressable
daemons (1.3.1) daemons (1.3.1)
danger (8.0.6) danger (8.2.3)
claide (~> 1.0) claide (~> 1.0)
claide-plugins (>= 0.9.2) claide-plugins (>= 0.9.2)
colored2 (~> 3.1) colored2 (~> 3.1)
...@@ -228,7 +228,10 @@ GEM ...@@ -228,7 +228,10 @@ GEM
kramdown-parser-gfm (~> 1.0) kramdown-parser-gfm (~> 1.0)
no_proxy_fix no_proxy_fix
octokit (~> 4.7) 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) database_cleaner (1.7.0)
debugger-ruby_core_source (1.3.8) debugger-ruby_core_source (1.3.8)
deckar01-task_list (2.3.1) deckar01-task_list (2.3.1)
...@@ -428,8 +431,13 @@ GEM ...@@ -428,8 +431,13 @@ GEM
gitaly (13.9.0.pre.rc1) gitaly (13.9.0.pre.rc1)
grpc (~> 1.0) grpc (~> 1.0)
github-markup (1.7.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) gitlab-chronic (0.10.5)
numerizer (~> 0.2) numerizer (~> 0.2)
gitlab-dangerfiles (0.8.0)
danger
gitlab-experiment (0.5.0) gitlab-experiment (0.5.0)
activesupport (>= 3.0) activesupport (>= 3.0)
scientist (~> 1.5, >= 1.5.0) scientist (~> 1.5, >= 1.5.0)
...@@ -1367,7 +1375,7 @@ DEPENDENCIES ...@@ -1367,7 +1375,7 @@ DEPENDENCIES
countries (~> 3.0) countries (~> 3.0)
creole (~> 0.5.0) creole (~> 0.5.0)
crystalball (~> 0.7.0) crystalball (~> 0.7.0)
danger (~> 8.0.6) danger-gitlab (~> 8.0)
database_cleaner (~> 1.7.0) database_cleaner (~> 1.7.0)
deckar01-task_list (= 2.3.1) deckar01-task_list (= 2.3.1)
default_value_for (~> 3.4.0) default_value_for (~> 3.4.0)
...@@ -1413,6 +1421,7 @@ DEPENDENCIES ...@@ -1413,6 +1421,7 @@ DEPENDENCIES
gitaly (~> 13.9.0.pre.rc1) gitaly (~> 13.9.0.pre.rc1)
github-markup (~> 1.7.0) github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5) gitlab-chronic (~> 0.10.5)
gitlab-dangerfiles (~> 0.8.0)
gitlab-experiment (~> 0.5.0) gitlab-experiment (~> 0.5.0)
gitlab-fog-azure-rm (~> 1.0.1) gitlab-fog-azure-rm (~> 1.0.1)
gitlab-fog-google (~> 1.13) gitlab-fog-google (~> 1.13)
......
...@@ -17,8 +17,8 @@ def check_changelog_yaml(path) ...@@ -17,8 +17,8 @@ def check_changelog_yaml(path)
raw_file = File.read(path) raw_file = File.read(path)
yaml = YAML.safe_load(raw_file) yaml = YAML.safe_load(raw_file)
fail "`title` should be set, in #{gitlab.html_link(path)}! #{SEE_DOC}" if yaml["title"].nil? fail "`title` should be set, in #{helper.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 "`type` should be set, in #{helper.html_link(path)}! #{SEE_DOC}" if yaml["type"].nil?
return if helper.security_mr? return if helper.security_mr?
...@@ -30,20 +30,20 @@ def check_changelog_yaml(path) ...@@ -30,20 +30,20 @@ def check_changelog_yaml(path)
if mr_line if mr_line
markdown(format(SUGGEST_MR_COMMENT, mr_iid: gitlab.mr_json["iid"]), file: path, line: mr_line.succ) markdown(format(SUGGEST_MR_COMMENT, mr_iid: gitlab.mr_json["iid"]), file: path, line: mr_line.succ)
else 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 end
elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !cherry_pick_against_stable_branch 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}" fail "Merge request ID was not set to #{gitlab.mr_json["iid"]}! #{SEE_DOC}"
end end
rescue Psych::Exception rescue Psych::Exception
# YAML could not be parsed, fail the build. # 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 rescue StandardError => e
warn "There was a problem trying to check the Changelog. Exception: #{e.class.name} - #{e.message}" warn "There was a problem trying to check the Changelog. Exception: #{e.class.name} - #{e.message}"
end end
def check_changelog_path(path) def check_changelog_path(path)
ee_changes = helper.all_ee_changes.dup ee_changes = project_helper.all_ee_changes.dup
ee_changes.delete(path) ee_changes.delete(path)
if ee_changes.any? && !changelog.ee_changelog? && !changelog.required? if ee_changes.any? && !changelog.ee_changelog? && !changelog.required?
......
# frozen_string_literal: true # frozen_string_literal: true
gitlab_danger = GitlabDanger.new(helper.gitlab_helper)
TEMPLATE_MESSAGE = <<~MSG TEMPLATE_MESSAGE = <<~MSG
This merge request requires a CI/CD Template review. To make sure these This merge request requires a CI/CD Template review. To make sure these
changes are reviewed, take the following steps: changes are reviewed, take the following steps:
...@@ -17,9 +15,9 @@ TEMPLATE_FILES_MESSAGE = <<~MSG ...@@ -17,9 +15,9 @@ TEMPLATE_FILES_MESSAGE = <<~MSG
The following files require a review from the CI/CD Templates maintainers: The following files require a review from the CI/CD Templates maintainers:
MSG 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? if gitlab.mr_labels.include?('ci::templates') || template_paths_to_review.any?
message 'This merge request adds or changes files that require a ' \ message 'This merge request adds or changes files that require a ' \
......
# frozen_string_literal: true # frozen_string_literal: true
require_relative File.expand_path('../../tooling/danger/commit_linter', __dir__) require 'gitlab/dangerfiles/commit_linter'
require_relative File.expand_path('../../tooling/danger/merge_request_linter', __dir__) require 'gitlab/dangerfiles/merge_request_linter'
COMMIT_MESSAGE_GUIDELINES = "https://docs.gitlab.com/ee/development/contributing/merge_request_workflow.html#commit-messages-guidelines" 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})." 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 ...@@ -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. 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 MSG
def gitlab_danger
@gitlab_danger ||= GitlabDanger.new(helper.gitlab_helper)
end
def fail_commit(commit, message, more_info: true) def fail_commit(commit, message, more_info: true)
self.fail(build_message(commit, message, more_info: more_info)) self.fail(build_message(commit, message, more_info: more_info))
end end
...@@ -39,22 +35,22 @@ end ...@@ -39,22 +35,22 @@ end
def squash_mr? def squash_mr?
# Locally, we assume the MR is set to be squashed so that the user only sees warnings instead of errors. # 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 end
def wip_mr? def wip_mr?
gitlab_danger.ci? ? gitlab.mr_json['work_in_progress'] : false helper.ci? ? gitlab.mr_json['work_in_progress'] : false
end end
def danger_job_link 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 end
# Perform various checks against commits. We're not using # Perform various checks against commits. We're not using
# https://github.com/jonallured/danger-commit_lint because its output is not # 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. # very helpful, and it doesn't offer the means of ignoring merge commits.
def lint_commit(commit) 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 # For now we'll ignore merge commits, as getting rid of those is a problem
# separate from enforcing good commit messages. # separate from enforcing good commit messages.
...@@ -93,7 +89,7 @@ end ...@@ -93,7 +89,7 @@ end
def lint_mr_title(mr_title) def lint_mr_title(mr_title)
commit = Struct.new(:message, :sha).new(mr_title) commit = Struct.new(:message, :sha).new(mr_title)
Tooling::Danger::MergeRequestLinter.new(commit).lint Gitlab::Dangerfiles::MergeRequestLinter.new(commit).lint
end end
def count_non_fixup_commits(commit_linters) def count_non_fixup_commits(commit_linters)
...@@ -109,7 +105,7 @@ def lint_commits(commits) ...@@ -109,7 +105,7 @@ def lint_commits(commits)
if multi_line_commit_linter && multi_line_commit_linter.failed? if multi_line_commit_linter && multi_line_commit_linter.failed?
warn_or_fail_commits(multi_line_commit_linter) 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) 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']) title_linter = lint_mr_title(gitlab.mr_json['title'])
if title_linter.failed? if title_linter.failed?
warn_or_fail_commits(title_linter) warn_or_fail_commits(title_linter)
......
# frozen_string_literal: true # 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_SHORT = "New %<migrations>s added but %<schema>s wasn't updated"
SCHEMA_NOT_UPDATED_MESSAGE_FULL = <<~MSG 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 ...@@ -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? 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? 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 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 end
if geo_migration_created && !geo_db_schema_updated 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 end
return unless gitlab_danger.ci? return unless helper.ci?
return if gitlab.mr_labels.include?(DATABASE_APPROVED_LABEL) 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? if gitlab.mr_labels.include?('database') || db_paths_to_review.any?
message 'This merge request adds or changes files that require a ' \ message 'This merge request adds or changes files that require a ' \
......
# frozen_string_literal: true # frozen_string_literal: true
def gitlab_danger
@gitlab_danger ||= GitlabDanger.new(helper.gitlab_helper)
end
def feature_mr? def feature_mr?
return false unless helper.gitlab_helper&.mr_labels (helper.mr_labels & %w[feature::addition feature::enhancement]).any?
(helper.gitlab_helper.mr_labels & %w[feature::addition feature::enhancement]).any?
end end
DOCUMENTATION_UPDATE_MISSING = <<~MSG DOCUMENTATION_UPDATE_MISSING = <<~MSG
...@@ -19,7 +13,7 @@ For more information, see: ...@@ -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. - The [definition of done](https://docs.gitlab.com/ee/development/contributing/merge_request_workflow.html#definition-of-done) documentation.
MSG 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 # Documentation should be updated for feature::addition and feature::enhancement
if docs_paths_to_review.empty? if docs_paths_to_review.empty?
...@@ -30,7 +24,7 @@ end ...@@ -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.' 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) markdown(<<~MARKDOWN)
## Documentation review ## Documentation review
......
...@@ -11,7 +11,7 @@ return if duplicate.empty? ...@@ -11,7 +11,7 @@ return if duplicate.empty?
warn 'This merge request has introduced duplicated yarn dependencies.' warn 'This merge request has introduced duplicated yarn dependencies.'
if GitlabDanger.new(helper.gitlab_helper).ci? if helper.ci?
markdown(<<~MARKDOWN) markdown(<<~MARKDOWN)
## Duplicate yarn dependencies ## Duplicate yarn dependencies
......
...@@ -13,7 +13,7 @@ return if eslint_candidates.empty? ...@@ -13,7 +13,7 @@ return if eslint_candidates.empty?
warn 'This merge request changed files with disabled eslint rules. Please consider fixing them.' 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) markdown(<<~MARKDOWN)
## Disabled eslint rules ## Disabled eslint rules
......
...@@ -13,7 +13,7 @@ new_karma_files = get_karma_files(git.added_files.to_a) ...@@ -13,7 +13,7 @@ new_karma_files = get_karma_files(git.added_files.to_a)
unless new_karma_files.empty? unless new_karma_files.empty?
if GitlabDanger.new(helper.gitlab_helper).ci? if helper.ci?
markdown(<<~MARKDOWN) markdown(<<~MARKDOWN)
## New karma spec file ## New karma spec file
...@@ -36,7 +36,7 @@ return if changed_karma_files.empty? ...@@ -36,7 +36,7 @@ return if changed_karma_files.empty?
warn 'You have edited karma spec files. Please consider migrating them to jest.' 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) markdown(<<~MARKDOWN)
## Edited karma files ## Edited karma files
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require_relative '../../tooling/danger/changelog' require_relative '../../tooling/danger/changelog'
module Danger module Danger
class Changelog < Plugin class Changelog < ::Danger::Plugin
# Put the helper code somewhere it can be tested # Put the helper code somewhere it can be tested
include Tooling::Danger::Changelog include Tooling::Danger::Changelog
end end
......
# frozen_string_literal: true # frozen_string_literal: true
require_relative '../../tooling/danger/helper' require_relative '../../tooling/danger/project_helper'
module Danger 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 # for more details
class Helper < Plugin class ProjectHelper < ::Danger::Plugin
# Put the helper code somewhere it can be tested # Put the helper code somewhere it can be tested
include Tooling::Danger::Helper include Tooling::Danger::ProjectHelper
end end
end end
# 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
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require_relative '../../tooling/danger/sidekiq_queues' require_relative '../../tooling/danger/sidekiq_queues'
module Danger module Danger
class SidekiqQueues < Plugin class SidekiqQueues < ::Danger::Plugin
# Put the helper code somewhere it can be tested # Put the helper code somewhere it can be tested
include Tooling::Danger::SidekiqQueues include Tooling::Danger::SidekiqQueues
end end
......
...@@ -19,7 +19,7 @@ return if unpretty.empty? ...@@ -19,7 +19,7 @@ return if unpretty.empty?
warn 'This merge request changed frontend files without pretty printing them.' warn 'This merge request changed frontend files without pretty printing them.'
if GitlabDanger.new(helper.gitlab_helper).ci? if helper.ci?
markdown(<<~MARKDOWN) markdown(<<~MARKDOWN)
## Pretty print Frontend files ## Pretty print Frontend files
......
...@@ -35,7 +35,7 @@ UNKNOWN_FILES_MESSAGE = <<MARKDOWN ...@@ -35,7 +35,7 @@ UNKNOWN_FILES_MESSAGE = <<MARKDOWN
These files couldn't be categorised, so Danger was unable to suggest a reviewer. These files couldn't be categorised, so Danger was unable to suggest a reviewer.
Please consider creating a merge request to 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. for them.
MARKDOWN MARKDOWN
...@@ -67,7 +67,7 @@ def markdown_row_for_spins(category, spins_array) ...@@ -67,7 +67,7 @@ def markdown_row_for_spins(category, spins_array)
"| #{helper.label_for_category(category)} | #{reviewer_note} | #{maintainer_note} |" "| #{helper.label_for_category(category)} | #{reviewer_note} | #{maintainer_note} |"
end 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 # Ignore any files that are known but uncategorized. Prompt for any unknown files
changes.delete(:none) changes.delete(:none)
...@@ -76,10 +76,10 @@ changes.delete(:docs) ...@@ -76,10 +76,10 @@ changes.delete(:docs)
categories = changes.keys - [:unknown] categories = changes.keys - [:unknown]
# Ensure to spin for database reviewer/maintainer when ~database is applied (e.g. to review SQL queries) # 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? if changes.any?
project = helper.project_name project = project_helper.project_name
random_roulette_spins = roulette.spin(project, categories, timezone_experiment: false) random_roulette_spins = roulette.spin(project, categories, timezone_experiment: false)
......
# frozen_string_literal: true # frozen_string_literal: true
gitlab_danger = GitlabDanger.new(helper.gitlab_helper) return unless helper.ci?
return unless gitlab_danger.ci?
SPECIALIZATIONS = { SPECIALIZATIONS = {
database: 'database', database: 'database',
...@@ -14,7 +12,7 @@ SPECIALIZATIONS = { ...@@ -14,7 +12,7 @@ SPECIALIZATIONS = {
ci_template: 'ci::templates' ci_template: 'ci::templates'
}.freeze }.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] label = SPECIALIZATIONS[category]
memo << label if label && !gitlab.mr_labels.include?(label) memo << label if label && !gitlab.mr_labels.include?(label)
......
...@@ -2,16 +2,16 @@ ...@@ -2,16 +2,16 @@
desc 'Run local Danger rules' desc 'Run local Danger rules'
task :danger_local do task :danger_local do
require_relative '../../tooling/gitlab_danger' require_relative '../../tooling/danger/project_helper'
require 'gitlab/popen' 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 :( # _status will _always_ be 0, regardless of failure or success :(
output, _status = Gitlab::Popen.popen(%w{danger dry_run}) output, _status = Gitlab::Popen.popen(%w{danger dry_run})
if output.empty? if output.empty?
puts(GitlabDanger.success_message) puts(Tooling::Danger::ProjectHelper.success_message)
else else
puts(output) puts(output)
exit(1) exit(1)
......
# 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
# frozen_string_literal: true # 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/changelog'
require_relative '../../../tooling/danger/project_helper'
RSpec.describe Tooling::Danger::Changelog do RSpec.describe Tooling::Danger::Changelog do
include DangerSpecHelper include_context "with dangerfile"
let(:change_class) { Tooling::Danger::Helper::Change } let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
let(:changes_class) { Tooling::Danger::Helper::Changes } let(:fake_project_helper) { double('fake-project-helper', helper: fake_helper).tap { |h| h.class.include(Tooling::Danger::ProjectHelper) } }
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) }
subject(:changelog) { fake_danger.new(helper: fake_helper) } 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 describe '#required_reasons' do
subject { changelog.required_reasons } subject { changelog.required_reasons }
...@@ -165,7 +162,7 @@ RSpec.describe Tooling::Danger::Changelog do ...@@ -165,7 +162,7 @@ RSpec.describe Tooling::Danger::Changelog do
subject { changelog.modified_text } subject { changelog.modified_text }
context "when title is not changed from sanitization", :aggregate_failures do context "when title is not changed from sanitization", :aggregate_failures do
let(:sanitize_mr_title) { 'Fake Title' } let(:mr_title) { 'Fake Title' }
specify do specify do
expect(subject).to include('CHANGELOG.md was edited') expect(subject).to include('CHANGELOG.md was edited')
...@@ -175,7 +172,7 @@ RSpec.describe Tooling::Danger::Changelog do ...@@ -175,7 +172,7 @@ RSpec.describe Tooling::Danger::Changelog do
end end
context "when title needs sanitization", :aggregate_failures do context "when title needs sanitization", :aggregate_failures do
let(:sanitize_mr_title) { 'DRAFT: Fake Title' } let(:mr_title) { 'DRAFT: Fake Title' }
specify do specify do
expect(subject).to include('CHANGELOG.md was edited') expect(subject).to include('CHANGELOG.md was edited')
...@@ -186,7 +183,7 @@ RSpec.describe Tooling::Danger::Changelog do ...@@ -186,7 +183,7 @@ RSpec.describe Tooling::Danger::Changelog do
end end
describe '#required_texts' do describe '#required_texts' do
let(:sanitize_mr_title) { 'Fake Title' } let(:mr_title) { 'Fake Title' }
subject { changelog.required_texts } subject { changelog.required_texts }
...@@ -207,7 +204,7 @@ RSpec.describe Tooling::Danger::Changelog do ...@@ -207,7 +204,7 @@ RSpec.describe Tooling::Danger::Changelog do
end end
context "when title needs sanitization", :aggregate_failures do 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 it_behaves_like 'changelog required text', :db_changes
end end
...@@ -224,7 +221,7 @@ RSpec.describe Tooling::Danger::Changelog do ...@@ -224,7 +221,7 @@ RSpec.describe Tooling::Danger::Changelog do
subject { changelog.optional_text } subject { changelog.optional_text }
context "when title is not changed from sanitization", :aggregate_failures do context "when title is not changed from sanitization", :aggregate_failures do
let(:sanitize_mr_title) { 'Fake Title' } let(:mr_title) { 'Fake Title' }
specify do specify do
expect(subject).to include('CHANGELOG missing') expect(subject).to include('CHANGELOG missing')
...@@ -234,7 +231,7 @@ RSpec.describe Tooling::Danger::Changelog do ...@@ -234,7 +231,7 @@ RSpec.describe Tooling::Danger::Changelog do
end end
context "when title needs sanitization", :aggregate_failures do context "when title needs sanitization", :aggregate_failures do
let(:sanitize_mr_title) { 'DRAFT: Fake Title' } let(:mr_title) { 'DRAFT: Fake Title' }
specify do specify do
expect(subject).to include('CHANGELOG missing') expect(subject).to include('CHANGELOG missing')
......
# 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
# 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
# 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
# frozen_string_literal: true # frozen_string_literal: true
require_relative 'danger_spec_helper' require 'gitlab-dangerfiles'
require 'gitlab/dangerfiles/spec_helper'
require_relative '../../../tooling/danger/feature_flag' require_relative '../../../tooling/danger/feature_flag'
RSpec.describe Tooling::Danger::FeatureFlag do RSpec.describe Tooling::Danger::FeatureFlag do
include DangerSpecHelper include_context "with dangerfile"
let(:added_files) { nil } let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
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(:mr_labels) { nil } subject(:feature_flag) { fake_danger.new(git: fake_git) }
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) }
describe '#feature_flag_files' do describe '#feature_flag_files' do
let(:feature_flag_files) do let(:feature_flag_files) do
......
# 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
This diff is collapsed.
# frozen_string_literal: true # frozen_string_literal: true
require 'rspec-parameterized' require 'rspec-parameterized'
require_relative 'danger_spec_helper' require 'gitlab-dangerfiles'
require 'gitlab/dangerfiles/spec_helper'
require_relative '../../../tooling/danger/sidekiq_queues' require_relative '../../../tooling/danger/sidekiq_queues'
RSpec.describe Tooling::Danger::SidekiqQueues do RSpec.describe Tooling::Danger::SidekiqQueues do
using RSpec::Parameterized::TableSyntax include_context "with dangerfile"
include DangerSpecHelper
let(:fake_git) { double('fake-git') } let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
let(:fake_danger) { new_fake_danger.include(described_class) }
subject(:sidekiq_queues) { fake_danger.new(git: fake_git) } subject(:sidekiq_queues) { fake_danger.new(git: fake_git) }
describe '#changed_queue_files' do describe '#changed_queue_files' do
using RSpec::Parameterized::TableSyntax
where(:modified_files, :changed_queue_files) do 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 foo) | %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml)
%w(app/workers/all_queues.yml ee/app/workers/all_queues.yml) | %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml) %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml) | %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml)
......
# 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
# 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
# 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
# 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
# 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
# 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
# frozen_string_literal: true # frozen_string_literal: true
require_relative 'title_linting' require 'gitlab/dangerfiles/title_linting'
module Tooling module Tooling
module Danger module Danger
...@@ -44,8 +44,8 @@ module Tooling ...@@ -44,8 +44,8 @@ module Tooling
def required_reasons def required_reasons
[].tap do |reasons| [].tap do |reasons|
reasons << :db_changes if helper.changes.added.has_category?(:migration) reasons << :db_changes if project_helper.changes.added.has_category?(:migration)
reasons << :feature_flag_removed if helper.changes.deleted.has_category?(:feature_flag) reasons << :feature_flag_removed if project_helper.changes.deleted.has_category?(:feature_flag)
end end
end end
...@@ -58,7 +58,7 @@ module Tooling ...@@ -58,7 +58,7 @@ module Tooling
end end
def found def found
@found ||= helper.changes.added.by_category(:changelog).files.first @found ||= project_helper.changes.added.by_category(:changelog).files.first
end end
def ee_changelog? def ee_changelog?
...@@ -86,11 +86,11 @@ module Tooling ...@@ -86,11 +86,11 @@ module Tooling
private private
def sanitized_mr_title def sanitized_mr_title
TitleLinting.sanitize_mr_title(helper.mr_title) Gitlab::Dangerfiles::TitleLinting.sanitize_mr_title(helper.mr_title)
end end
def categories_need_changelog? def categories_need_changelog?
(helper.changes.categories - NO_CHANGELOG_CATEGORIES).any? (project_helper.changes.categories - NO_CHANGELOG_CATEGORIES).any?
end end
def without_no_changelog_label? def without_no_changelog_label?
......
# 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
# 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
# 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
# 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
# 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
# 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
# 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
# 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
# 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
# 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
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment