diff --git a/Dangerfile b/Dangerfile
index b65a9074078a022bc08c4e9dd893d15fb2809978..7879c14b31e0503effd868f77da67eac8d830537 100644
--- a/Dangerfile
+++ b/Dangerfile
@@ -5,6 +5,7 @@ require_relative 'lib/gitlab/danger/request_helper'
danger.import_plugin('danger/plugins/helper.rb')
danger.import_plugin('danger/plugins/roulette.rb')
+danger.import_plugin('danger/plugins/changelog.rb')
unless helper.release_automation?
GitlabDanger.new(helper.gitlab_helper).rule_names.each do |file|
diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile
index f8e31323f412175e20a57973a8d98a28da7f2342..62b41d14bee2aadf813f125ff754544cb21c5e1c 100644
--- a/danger/changelog/Dangerfile
+++ b/danger/changelog/Dangerfile
@@ -3,7 +3,6 @@
require 'yaml'
-NO_CHANGELOG_LABELS = %w[backstage ci-build meta].freeze
SEE_DOC = "See [the documentation](https://docs.gitlab.com/ce/development/changelog.html)."
CREATE_CHANGELOG_MESSAGE = <<~MSG
You can create one with:
@@ -21,18 +20,6 @@ bin/changelog --ee -m %<mr_iid>s "%<mr_title>s"
Note: Merge requests with %<labels>s do not trigger this check.
MSG
-def ee_changelog?(changelog_path)
- changelog_path =~ /unreleased-ee/
-end
-
-def ce_port_changelog?(changelog_path)
- helper.ee? && !ee_changelog?(changelog_path)
-end
-
-def categories_need_changelog?
- (helper.changes_by_category.keys - %i[docs none]).any?
-end
-
def check_changelog(path)
yaml = YAML.safe_load(File.read(path))
@@ -41,7 +28,7 @@ def check_changelog(path)
if yaml["merge_request"].nil?
message "Consider setting `merge_request` to #{gitlab.mr_json["iid"]} in #{gitlab.html_link(path)}. #{SEE_DOC}"
- elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !ce_port_changelog?(path)
+ elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !changelog.ce_port_changelog?(path)
fail "Merge request ID was not set to #{gitlab.mr_json["iid"]}! #{SEE_DOC}"
end
rescue Psych::SyntaxError, Psych::DisallowedClass, Psych::BadAlias
@@ -51,27 +38,18 @@ rescue StandardError => e
warn "There was a problem trying to check the Changelog. Exception: #{e.name} - #{e.message}"
end
-def presented_no_changelog_labels
- NO_CHANGELOG_LABELS.map { |label| "~#{label}" }.join(', ')
-end
-
-def sanitized_mr_title
- gitlab.mr_json["title"].gsub(/^WIP: */, '').gsub(/`/, '\\\`')
-end
-
-changelog_needed = categories_need_changelog? && (gitlab.mr_labels & NO_CHANGELOG_LABELS).empty?
-changelog_found = git.added_files.find { |path| path =~ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} }
-
if git.modified_files.include?("CHANGELOG.md")
fail "**CHANGELOG.md was edited.** Please remove the additions and create a CHANGELOG entry.\n\n" +
- format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: presented_no_changelog_labels)
+ format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: changelog.sanitized_mr_title, labels: changelog.presented_no_changelog_labels)
end
-if changelog_needed
+changelog_found = changelog.found
+
+if changelog.needed?
if changelog_found
check_changelog(changelog_found)
else
message "**[CHANGELOG missing](https://docs.gitlab.com/ce/development/changelog.html)**: If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message.\n\n" +
- format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: presented_no_changelog_labels)
+ format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: changelog.sanitized_mr_title, labels: changelog.presented_no_changelog_labels)
end
end
diff --git a/danger/plugins/changelog.rb b/danger/plugins/changelog.rb
new file mode 100644
index 0000000000000000000000000000000000000000..84f399e9e9799c01deda0a7fd6ad834b981f3a7d
--- /dev/null
+++ b/danger/plugins/changelog.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+require_relative '../../lib/gitlab/danger/changelog'
+
+module Danger
+ class Changelog < Plugin
+ # Put the helper code somewhere it can be tested
+ include Gitlab::Danger::Changelog
+ end
+end
diff --git a/lib/gitlab/danger/changelog.rb b/lib/gitlab/danger/changelog.rb
new file mode 100644
index 0000000000000000000000000000000000000000..b53516081beb4d2fe555a79b059fc1b07c24e557
--- /dev/null
+++ b/lib/gitlab/danger/changelog.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Danger
+ module Changelog
+ NO_CHANGELOG_LABELS = %w[backstage ci-build meta].freeze
+ NO_CHANGELOG_CATEGORIES = %i[docs none].freeze
+
+ def needed?
+ categories_need_changelog? && (gitlab.mr_labels & NO_CHANGELOG_LABELS).empty?
+ end
+
+ def found
+ git.added_files.find { |path| path =~ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} }
+ end
+
+ def presented_no_changelog_labels
+ NO_CHANGELOG_LABELS.map { |label| "~#{label}" }.join(', ')
+ end
+
+ def sanitized_mr_title
+ gitlab.mr_json["title"].gsub(/^WIP: */, '').gsub(/`/, '\\\`')
+ end
+
+ def ee_changelog?(changelog_path)
+ changelog_path =~ /unreleased-ee/
+ end
+
+ def ce_port_changelog?(changelog_path)
+ helper.ee? && !ee_changelog?(changelog_path)
+ end
+
+ private
+
+ def categories_need_changelog?
+ (helper.changes_by_category.keys - NO_CHANGELOG_CATEGORIES).any?
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/danger/changelog_spec.rb b/spec/lib/gitlab/danger/changelog_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..888094eaf6e143174f6bef0a47716dc02c9bbe9a
--- /dev/null
+++ b/spec/lib/gitlab/danger/changelog_spec.rb
@@ -0,0 +1,163 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rspec-parameterized'
+require_relative 'danger_spec_helper'
+
+require 'gitlab/danger/changelog'
+
+describe Gitlab::Danger::Changelog do
+ using RSpec::Parameterized::TableSyntax
+ include DangerSpecHelper
+
+ let(:added_files) { nil }
+ let(:fake_git) { double('fake-git', added_files: added_files) }
+
+ let(:mr_labels) { nil }
+ let(:mr_json) { nil }
+ let(:fake_gitlab) { double('fake-gitlab', mr_labels: mr_labels, mr_json: mr_json) }
+
+ let(:changes_by_category) { nil }
+ let(:ee?) { false }
+ let(:fake_helper) { double('fake-helper', changes_by_category: changes_by_category, ee?: ee?) }
+
+ let(:fake_danger) { new_fake_danger.include(described_class) }
+
+ subject(:changelog) { fake_danger.new(git: fake_git, gitlab: fake_gitlab, helper: fake_helper) }
+
+ describe '#needed?' do
+ subject { changelog.needed? }
+
+ [
+ { docs: nil },
+ { none: nil },
+ { docs: nil, none: nil }
+ ].each do |categories|
+ let(:changes_by_category) { categories }
+ it "is falsy when categories don't require a changelog" do
+ is_expected.to be_falsy
+ end
+ end
+
+ where(:categories, :labels) do
+ { backend: nil } | %w[backend backstage]
+ { frontend: nil, docs: nil } | ['ci-build']
+ { engineering_productivity: nil, none: nil } | ['meta']
+ end
+
+ with_them do
+ let(:changes_by_category) { categories }
+ let(:mr_labels) { labels }
+
+ it "is falsy when labels require no changelog" do
+ is_expected.to be_falsy
+ end
+ end
+
+ where(:categories, :labels) do
+ { frontend: nil, docs: nil } | ['database::review pending', 'feature']
+ { backend: nil } | ['backend', 'technical debt']
+ { engineering_productivity: nil, none: nil } | ['frontend']
+ end
+
+ with_them do
+ let(:changes_by_category) { categories }
+ let(:mr_labels) { labels }
+
+ it "is truthy when categories and labels require a changelog" do
+ is_expected.to be_truthy
+ end
+ end
+ end
+
+ describe '#found' do
+ subject { changelog.found }
+
+ context 'added files contain a changelog' do
+ [
+ 'changelogs/unreleased/entry.md',
+ 'ee/changelogs/unreleased/entry.md',
+ 'changelogs/unreleased-ee/entry.md',
+ 'ee/changelogs/unreleased-ee/entry.md'
+ ].each do |file_path|
+ let(:added_files) { [file_path] }
+
+ it { is_expected.to be_truthy }
+ end
+ end
+
+ context 'added files do not contain a changelog' do
+ [
+ 'app/models/model.rb',
+ 'app/assets/javascripts/file.js'
+ ].each do |file_path|
+ let(:added_files) { [file_path] }
+ it { is_expected.to eq(nil) }
+ end
+ end
+ end
+
+ describe '#presented_no_changelog_labels' do
+ subject { changelog.presented_no_changelog_labels }
+
+ it 'returns the labels formatted' do
+ is_expected.to eq('~backstage, ~ci-build, ~meta')
+ end
+ end
+
+ describe '#sanitized_mr_title' do
+ subject { changelog.sanitized_mr_title }
+
+ [
+ 'WIP: My MR title',
+ 'My MR title'
+ ].each do |mr_title|
+ let(:mr_json) { { "title" => mr_title } }
+ it { is_expected.to eq("My MR title") }
+ end
+ end
+
+ describe '#ee_changelog?' do
+ context 'is ee changelog' do
+ [
+ 'changelogs/unreleased-ee/entry.md',
+ 'ee/changelogs/unreleased-ee/entry.md'
+ ].each do |file_path|
+ subject { changelog.ee_changelog?(file_path) }
+
+ it { is_expected.to be_truthy }
+ end
+ end
+
+ context 'is not ee changelog' do
+ [
+ 'changelogs/unreleased/entry.md',
+ 'ee/changelogs/unreleased/entry.md'
+ ].each do |file_path|
+ subject { changelog.ee_changelog?(file_path) }
+
+ it { is_expected.to be_falsy }
+ end
+ end
+ end
+
+ describe '#ce_port_changelog?' do
+ where(:helper_ee?, :file_path, :expected) do
+ true | 'changelogs/unreleased-ee/entry.md' | false
+ true | 'ee/changelogs/unreleased-ee/entry.md' | false
+ false | 'changelogs/unreleased-ee/entry.md' | false
+ false | 'ee/changelogs/unreleased-ee/entry.md' | false
+ true | 'changelogs/unreleased/entry.md' | true
+ true | 'ee/changelogs/unreleased/entry.md' | true
+ false | 'changelogs/unreleased/entry.md' | false
+ false | 'ee/changelogs/unreleased/entry.md' | false
+ end
+
+ with_them do
+ let(:ee?) { helper_ee? }
+ subject { changelog.ce_port_changelog?(file_path) }
+
+ it { is_expected.to eq(expected) }
+ end
+ end
+end
diff --git a/spec/lib/gitlab/danger/danger_spec_helper.rb b/spec/lib/gitlab/danger/danger_spec_helper.rb
new file mode 100644
index 0000000000000000000000000000000000000000..b1e84b3c13de8ad3cbac472a4b3f37bf1e087543
--- /dev/null
+++ b/spec/lib/gitlab/danger/danger_spec_helper.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module DangerSpecHelper
+ def new_fake_danger
+ Class.new do
+ attr_reader :git, :gitlab, :helper
+
+ # rubocop:disable Gitlab/ModuleWithInstanceVariables
+ def initialize(git: nil, gitlab: nil, helper: nil)
+ @git = git
+ @gitlab = gitlab
+ @helper = helper
+ end
+ # rubocop:enable Gitlab/ModuleWithInstanceVariables
+ end
+ end
+end
diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb
index 8056418e6972e9d4fa41e0b1096159d16af738cc..d7e67444fca151b1c87123ba0077c912fbbe3baf 100644
--- a/spec/lib/gitlab/danger/helper_spec.rb
+++ b/spec/lib/gitlab/danger/helper_spec.rb
@@ -2,29 +2,22 @@
require 'fast_spec_helper'
require 'rspec-parameterized'
+require_relative 'danger_spec_helper'
require 'gitlab/danger/helper'
describe Gitlab::Danger::Helper do
using RSpec::Parameterized::TableSyntax
-
- class FakeDanger
- include Gitlab::Danger::Helper
-
- attr_reader :git, :gitlab
-
- def initialize(git:, gitlab:)
- @git = git
- @gitlab = gitlab
- end
- end
+ include DangerSpecHelper
let(:fake_git) { double('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) }
+ let(:fake_danger) { new_fake_danger.include(described_class) }
+
+ subject(:helper) { fake_danger.new(git: fake_git, gitlab: fake_gitlab) }
describe '#gitlab_helper' do
context 'when gitlab helper is not available' do