kotovalexarian-likes-gitlab
/
gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add new GitlabDanger class 

This class encapsulates our use of the Danger gem.
This commit is contained in:
Ash McKenzie 2019-09-06 11:21:53 +00:00 committed by Lin Jen-Shin
parent 3441092b38
commit 9fc9ab2ba2
15 changed files with 334 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
Dangerfile
View File

@ -1,23 +1,12 @@
# frozen_string_literal: true
require_relative 'lib/gitlab_danger'
danger.import_plugin('danger/plugins/helper.rb')
danger.import_plugin('danger/plugins/roulette.rb')
unless helper.release_automation?
danger.import_dangerfile(path: 'danger/metadata')
danger.import_dangerfile(path: 'danger/changes_size')
danger.import_dangerfile(path: 'danger/changelog')
danger.import_dangerfile(path: 'danger/specs')
danger.import_dangerfile(path: 'danger/gemfile')
danger.import_dangerfile(path: 'danger/database')
danger.import_dangerfile(path: 'danger/documentation')
danger.import_dangerfile(path: 'danger/frozen_string')
danger.import_dangerfile(path: 'danger/commit_messages')
danger.import_dangerfile(path: 'danger/duplicate_yarn_dependencies')
danger.import_dangerfile(path: 'danger/prettier')
danger.import_dangerfile(path: 'danger/eslint')
danger.import_dangerfile(path: 'danger/roulette')
danger.import_dangerfile(path: 'danger/single_codebase')
danger.import_dangerfile(path: 'danger/gitlab_ui_wg')
danger.import_dangerfile(path: 'danger/ce_ee_vue_templates')
danger.import_dangerfile(path: 'danger/only_documentation')
GitlabDanger.new(helper.gitlab_helper).rule_names.each do |file|
danger.import_dangerfile(path: File.join('danger', file))
end
end

1
Gemfile
View File

@ -318,6 +318,7 @@ end
group :development do
gem 'foreman', '~> 0.84.0'
gem 'brakeman', '~> 4.2', require: false
gem 'danger', '~> 6.0', require: false
gem 'letter_opener_web', '~> 1.3.4'
gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false

33
Gemfile.lock
View File

@ -140,9 +140,15 @@ GEM
numerizer (~> 0.1.1)
chunky_png (1.3.5)
citrus (3.0.2)
claide (1.0.3)
claide-plugins (0.9.2)
cork
nap
open4 (~> 1.3)
coderay (1.1.2)
coercible (1.0.0)
descendants_tracker (~> 0.0.1)
colored2 (3.1.2)
commonmarker (0.17.13)
ruby-enum (~> 0.5)
concord (0.1.5)
@ -151,6 +157,8 @@ GEM
concurrent-ruby (1.1.5)
connection_pool (2.2.2)
contracts (0.11.0)
cork (0.3.0)
colored2 (~> 3.1)
crack (0.4.3)
safe_yaml (~> 1.0.0)
crass (1.0.4)
@ -158,6 +166,19 @@ GEM
css_parser (1.5.0)
addressable
daemons (1.2.6)
danger (6.0.9)
claide (~> 1.0)
claide-plugins (>= 0.9.2)
colored2 (~> 3.1)
cork (~> 0.1)
faraday (~> 0.9)
faraday-http-cache (~> 2.0)
git (~> 1.5)
kramdown (~> 2.0)
kramdown-parser-gfm (~> 1.0)
no_proxy_fix
octokit (~> 4.7)
terminal-table (~> 1)
database_cleaner (1.7.0)
debug_inspector (0.0.3)
debugger-ruby_core_source (1.3.8)
@ -227,6 +248,8 @@ GEM
railties (>= 3.0.0)
faraday (0.12.2)
multipart-post (>= 1.2, < 3)
faraday-http-cache (2.0.0)
faraday (~> 0.8)
faraday_middleware (0.12.2)
faraday (>= 0.7.4, < 1.0)
faraday_middleware-multi_json (0.0.6)
@ -308,6 +331,7 @@ GEM
gettext_i18n_rails (>= 0.7.1)
po_to_json (>= 1.0.0)
rails (>= 3.2.0)
git (1.5.0)
gitaly (1.58.0)
grpc (~> 1.0)
github-markup (1.7.0)
@ -478,6 +502,9 @@ GEM
kgio (2.11.2)
knapsack (1.17.0)
rake
kramdown (2.1.0)
kramdown-parser-gfm (1.1.0)
kramdown (~> 2.0)
kubeclient (4.2.2)
http (~> 3.0)
recursive-open-struct (~> 1.0, >= 1.0.4)
@ -535,10 +562,12 @@ GEM
mustermann-grape (1.0.0)
mustermann (~> 1.0.0)
nakayoshi_fork (0.0.4)
nap (1.1.0)
net-ldap (0.16.0)
net-ssh (5.2.0)
netrc (0.11.0)
nio4r (2.3.1)
no_proxy_fix (0.1.2)
nokogiri (1.10.4)
mini_portile2 (~> 2.4.0)
nokogumbo (1.5.0)
@ -615,6 +644,7 @@ GEM
addressable (~> 2.5)
omniauth (~> 1.3)
openid_connect (~> 1.1)
open4 (1.3.4)
openid_connect (1.1.6)
activemodel
attr_required (>= 1.0.0)
@ -926,6 +956,8 @@ GEM
ffi
sysexits (1.2.0)
temple (0.8.0)
terminal-table (1.8.0)
unicode-display_width (~> 1.1, >= 1.1.1)
test-prof (0.2.5)
text (1.3.1)
thin (1.7.2)
@ -1055,6 +1087,7 @@ DEPENDENCIES
concurrent-ruby (~> 1.1)
connection_pool (~> 2.0)
creole (~> 0.5.0)
danger (~> 6.0)
database_cleaner (~> 1.7.0)
deckar01-task_list (= 2.2.0)
default_value_for (~> 3.2.0)

18
danger/database/Dangerfile
View File

@ -1,7 +1,13 @@
# frozen_string_literal: true
SCHEMA_NOT_UPDATED_MESSAGE = <<~MSG
**New %<migrations>s added but %<schema>s wasn't updated.**
gitlab_danger = GitlabDanger.new(helper.gitlab_helper)
SCHEMA_NOT_UPDATED_MESSAGE_SHORT = <<~MSG
New %<migrations>s added but %<schema>s wasn't updated.
MSG
SCHEMA_NOT_UPDATED_MESSAGE_FULL = <<~MSG
**#{SCHEMA_NOT_UPDATED_MESSAGE_SHORT}**
Usually, when adding new %<migrations>s, %<schema>s should be
updated too (unless the migration isn't changing the DB schema
@ -29,14 +35,18 @@ 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
if non_geo_migration_created && !non_geo_db_schema_updated
warn format(SCHEMA_NOT_UPDATED_MESSAGE, migrations: 'migrations', schema: gitlab.html_link("db/schema.rb"))
warn format(format_str, migrations: 'migrations', schema: gitlab_danger.html_link("db/schema.rb"))
end
if geo_migration_created && !geo_db_schema_updated
warn format(SCHEMA_NOT_UPDATED_MESSAGE, migrations: 'Geo migrations', schema: gitlab.html_link("ee/db/geo/schema.rb"))
warn format(format_str, migrations: 'Geo migrations', schema: gitlab_danger.html_link("ee/db/geo/schema.rb"))
end
return unless gitlab_danger.ci?
db_paths_to_review = helper.changes_by_category[:database]
if gitlab.mr_labels.include?('database') || db_paths_to_review.any?

22
danger/documentation/Dangerfile
View File

@ -6,20 +6,22 @@ unless docs_paths_to_review.empty?
message 'This merge request adds or changes files that require a review ' \
'from the Technical Writing team.'
markdown(<<~MARKDOWN)
## Documentation review
if GitlabDanger.new(helper.gitlab_helper).ci?
markdown(<<~MARKDOWN)
## Documentation review
The following files require a review from a technical writer:
The following files require a review from a technical writer:
* #{docs_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")}
* #{docs_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")}
The review does not need to block merging this merge request. See the:
The review does not need to block merging this merge request. See the:
- [DevOps stages](https://about.gitlab.com/handbook/product/categories/#devops-stages) for the appropriate technical writer for this review.
- [Documentation workflows](https://docs.gitlab.com/ee/development/documentation/workflow.html) for information on when to assign a merge request for review.
MARKDOWN
- [DevOps stages](https://about.gitlab.com/handbook/product/categories/#devops-stages) for the appropriate technical writer for this review.
- [Documentation workflows](https://docs.gitlab.com/ee/development/documentation/workflow.html) for information on when to assign a merge request for review.
MARKDOWN
unless gitlab.mr_labels.include?('Documentation')
warn 'This merge request is missing the ~Documentation label.'
unless gitlab.mr_labels.include?('Documentation')
warn 'This merge request is missing the ~Documentation label.'
end
end
end

24
danger/duplicate_yarn_dependencies/Dangerfile
View File

@ -1,6 +1,6 @@
# frozen_string_literal: true
return unless helper.all_changed_files.include? 'yarn.lock'
return unless helper.all_changed_files.include?('yarn.lock')
duplicate = `node_modules/.bin/yarn-deduplicate --list --strategy fewer yarn.lock`
.split(/$/)
@ -11,17 +11,19 @@ return if duplicate.empty?
warn 'This merge request has introduced duplicated yarn dependencies.'
markdown(<<~MARKDOWN)
## Duplicate yarn dependencies
if GitlabDanger.new(helper.gitlab_helper).ci?
markdown(<<~MARKDOWN)
## Duplicate yarn dependencies
The following dependencies should be de-duplicated:
The following dependencies should be de-duplicated:
* #{duplicate.map { |path| "`#{path}`" }.join("\n* ")}
* #{duplicate.map { |path| "`#{path}`" }.join("\n* ")}
Please run the following command and commit the changes to `yarn.lock`:
Please run the following command and commit the changes to `yarn.lock`:
```
node_modules/.bin/yarn-deduplicate --strategy fewer yarn.lock \\
&& yarn install
```
MARKDOWN
```
node_modules/.bin/yarn-deduplicate --strategy fewer yarn.lock \\
&& yarn install
```
MARKDOWN
end

22
danger/eslint/Dangerfile
View File

@ -13,17 +13,19 @@ return if eslint_candidates.empty?
warn 'This merge request changed files with disabled eslint rules. Please consider fixing them.'
markdown(<<~MARKDOWN)
## Disabled eslint rules
if GitlabDanger.new(helper.gitlab_helper).ci?
markdown(<<~MARKDOWN)
## Disabled eslint rules
The following files have disabled `eslint` rules. Please consider fixing them:
The following files have disabled `eslint` rules. Please consider fixing them:
* #{eslint_candidates.map { |path| "`#{path}`" }.join("\n* ")}
* #{eslint_candidates.map { |path| "`#{path}`" }.join("\n* ")}
Run the following command for more details
Run the following command for more details
```
node_modules/.bin/eslint --report-unused-disable-directives --no-inline-config \\
#{eslint_candidates.map { |path| " '#{path}'" }.join(" \\\n")}
```
MARKDOWN
```
node_modules/.bin/eslint --report-unused-disable-directives --no-inline-config \\
#{eslint_candidates.map { |path| " '#{path}'" }.join(" \\\n")}
```
MARKDOWN
end

12
danger/frozen_string/Dangerfile
View File

@ -16,11 +16,13 @@ if files_to_fix.any?
warn 'This merge request adds files that do not enforce frozen string literal. ' \
'See https://gitlab.com/gitlab-org/gitlab-ce/issues/47424 for more information.'
markdown(<<~MARKDOWN)
## Enable Frozen String Literal
if GitlabDanger.new(helper.gitlab_helper).ci?
markdown(<<~MARKDOWN)
## Enable Frozen String Literal
The following files should have `#{MAGIC_COMMENT}` on the first line:
The following files should have `#{MAGIC_COMMENT}` on the first line:
* #{files_to_fix.map { |path| "`#{path}`" }.join("\n* ")}
MARKDOWN
* #{files_to_fix.map { |path| "`#{path}`" }.join("\n* ")}
MARKDOWN
end
end

19
danger/gemfile/Dangerfile
View File

@ -1,5 +1,9 @@
GEMFILE_LOCK_NOT_UPDATED_MESSAGE = <<~MSG.freeze
**%<gemfile>s was updated but %<gemfile_lock>s wasn't updated.**
GEMFILE_LOCK_NOT_UPDATED_MESSAGE_SHORT = <<~MSG.freeze
%<gemfile>s was updated but %<gemfile_lock>s wasn't updated.
MSG
GEMFILE_LOCK_NOT_UPDATED_MESSAGE_FULL = <<~MSG.freeze
**#{GEMFILE_LOCK_NOT_UPDATED_MESSAGE_SHORT}**
Usually, when %<gemfile>s is updated, you should run
```
@ -19,5 +23,14 @@ gemfile_modified = git.modified_files.include?("Gemfile")
gemfile_lock_modified = git.modified_files.include?("Gemfile.lock")
if gemfile_modified && !gemfile_lock_modified
warn format(GEMFILE_LOCK_NOT_UPDATED_MESSAGE, gemfile: gitlab.html_link("Gemfile"), gemfile_lock: gitlab.html_link("Gemfile.lock"))
gitlab_danger = GitlabDanger.new(helper.gitlab_helper)
format_str = gitlab_danger.ci? ? GEMFILE_LOCK_NOT_UPDATED_MESSAGE_FULL : GEMFILE_LOCK_NOT_UPDATED_MESSAGE_SHORT
message = format(format_str,
gemfile: gitlab_danger.html_link("Gemfile"),
gemfile_lock: gitlab_danger.html_link("Gemfile.lock")
)
warn(message)
end

26
danger/prettier/Dangerfile
View File

@ -19,21 +19,23 @@ return if unpretty.empty?
warn 'This merge request changed frontend files without pretty printing them.'
markdown(<<~MARKDOWN)
## Pretty print Frontend files
if GitlabDanger.new(helper.gitlab_helper).ci?
markdown(<<~MARKDOWN)
## Pretty print Frontend files
The following files should have been pretty printed with `prettier`:
The following files should have been pretty printed with `prettier`:
* #{unpretty.map { |path| "`#{path}`" }.join("\n* ")}
* #{unpretty.map { |path| "`#{path}`" }.join("\n* ")}
Please run
Please run
```
node_modules/.bin/prettier --write \\
#{unpretty.map { |path| " '#{path}'" }.join(" \\\n")}
```
```
node_modules/.bin/prettier --write \\
#{unpretty.map { |path| " '#{path}'" }.join(" \\\n")}
```
Also consider auto-formatting [on-save].
Also consider auto-formatting [on-save].
[on-save]: https://docs.gitlab.com/ee/development/new_fe_guide/style/prettier.html
MARKDOWN
[on-save]: https://docs.gitlab.com/ee/development/new_fe_guide/style/prettier.html
MARKDOWN
end

11
lib/gitlab/danger/helper.rb
View File

@ -38,8 +38,17 @@ module Gitlab
ENV['CI_PROJECT_NAME'] == 'gitlab-ee' || File.exist?('../../CHANGELOG-EE.md')
end
def gitlab_helper
# Unfortunately the following does not work:
# - respond_to?(:gitlab)
# - respond_to?(:gitlab, true)
gitlab
rescue NoMethodError
nil
end
def release_automation?
gitlab.mr_author == RELEASE_TOOLS_BOT
gitlab_helper&.mr_author == RELEASE_TOOLS_BOT
end
def project_name

54
lib/gitlab_danger.rb Normal file
View File

@ -0,0 +1,54 @@
# frozen_string_literal: true
class GitlabDanger
LOCAL_RULES ||= %w[
changes_size
gemfile
documentation
frozen_string
duplicate_yarn_dependencies
prettier
eslint
database
].freeze
CI_ONLY_RULES ||= %w[
metadata
changelog
specs
commit_messages
roulette
single_codebase
gitlab_ui_wg
ce_ee_vue_templates
only_documentation
].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

17
lib/tasks/gitlab_danger.rake Normal file
View File

@ -0,0 +1,17 @@
desc 'Run local Danger rules'
task :danger_local do
require 'gitlab_danger'
require 'gitlab/popen'
puts("#{GitlabDanger.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)
else
puts(output)
exit(1)
end
end

52
spec/lib/gitlab/danger/helper_spec.rb
View File

@ -11,16 +11,62 @@ describe Gitlab::Danger::Helper do
class FakeDanger
include Gitlab::Danger::Helper
attr_reader :git
attr_reader :git, :gitlab
def initialize(git:)
def initialize(git:, gitlab:)
@git = git
@gitlab = gitlab
end
end
let(:fake_git) { double('fake-git') }
subject(:helper) { FakeDanger.new(git: fake_git) }
let(:mr_author) { nil }
let(:fake_gitlab) { double('fake-gitlab', mr_author: mr_author) }
subject(:helper) { FakeDanger.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
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 }

76
spec/lib/gitlab_danger_spec.rb Normal file
View File

@ -0,0 +1,76 @@
# frozen_string_literal: true
require 'spec_helper'
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: changes_size, gemfile, documentation, frozen_string, duplicate_yarn_dependencies, prettier, eslint, database')
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