From 4555e1b21c365ed8303ffb7a3325d773c9b8bf31 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 19 May 2021 15:44:42 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-12-stable-ee --- spec/tooling/danger/changelog_spec.rb | 220 ++++++++++++++++++++++++++++- spec/tooling/danger/project_helper_spec.rb | 6 +- 2 files changed, 221 insertions(+), 5 deletions(-) (limited to 'spec/tooling') diff --git a/spec/tooling/danger/changelog_spec.rb b/spec/tooling/danger/changelog_spec.rb index 7ea2288fd45..0a6af3ea798 100644 --- a/spec/tooling/danger/changelog_spec.rb +++ b/spec/tooling/danger/changelog_spec.rb @@ -18,6 +18,204 @@ RSpec.describe Tooling::Danger::Changelog do allow(changelog).to receive(:project_helper).and_return(fake_project_helper) end + describe '#check_changelog_trailer' do + subject { changelog.check_changelog_trailer(commit) } + + context "when commit doesn't include a changelog trailer" do + let(:commit) { double('commit', message: "Hello world") } + + it { is_expected.to be_nil } + end + + context "when commit include a changelog trailer with no category" do + let(:commit) { double('commit', message: "Hello world\n\nChangelog:") } + + it { is_expected.to be_nil } + end + + context "when commit include a changelog trailer with an unknown category" do + let(:commit) { double('commit', message: "Hello world\n\nChangelog: foo", sha: "abc123") } + + it { is_expected.to have_attributes(errors: ["Commit #{commit.sha} uses an invalid changelog category: foo"]) } + end + + described_class::CATEGORIES.each do |category| + context "when commit include a changelog trailer with category set to '#{category}'" do + let(:commit) { double('commit', message: "Hello world\n\nChangelog: #{category}", sha: "abc123") } + + it { is_expected.to have_attributes(errors: []) } + end + end + end + + describe '#check_changelog_yaml' do + let(:changelog_path) { 'ee/changelogs/unreleased/entry.yml' } + let(:changes) { changes_class.new([change_class.new(changelog_path, :added, :changelog)]) } + let(:yaml_title) { 'Fix changelog Dangerfile to convert MR IID to a string before comparison' } + let(:yaml_merge_request) { 60899 } + let(:mr_iid) { '60899' } + let(:yaml_type) { 'fixed' } + let(:yaml) do + <<~YAML + --- + title: #{yaml_title} + merge_request: #{yaml_merge_request} + author: + type: #{yaml_type} + YAML + end + + before do + allow(changelog).to receive(:present?).and_return(true) + allow(changelog).to receive(:changelog_path).and_return(changelog_path) + allow(changelog).to receive(:read_file).with(changelog_path).and_return(yaml) + allow(fake_helper).to receive(:security_mr?).and_return(false) + allow(fake_helper).to receive(:mr_iid).and_return(mr_iid) + allow(fake_helper).to receive(:cherry_pick_mr?).and_return(false) + allow(fake_helper).to receive(:stable_branch?).and_return(false) + allow(fake_helper).to receive(:html_link).with(changelog_path).and_return(changelog_path) + end + + subject { changelog.check_changelog_yaml } + + context "when changelog is not present" do + before do + allow(changelog).to receive(:present?).and_return(false) + end + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + end + + context "when YAML is invalid" do + let(:yaml) { '{ foo bar]' } + + it { is_expected.to have_attributes(errors: ["#{changelog_path} isn't valid YAML! #{described_class::SEE_DOC}"]) } + end + + context "when a StandardError is raised" do + before do + allow(changelog).to receive(:read_file).and_raise(StandardError, "Fail!") + end + + it { is_expected.to have_attributes(warnings: ["There was a problem trying to check the Changelog. Exception: StandardError - Fail!"]) } + end + + context "when YAML title is nil" do + let(:yaml_title) { '' } + + it { is_expected.to have_attributes(errors: ["`title` should be set, in #{changelog_path}! #{described_class::SEE_DOC}"]) } + end + + context "when YAML type is nil" do + let(:yaml_type) { '' } + + it { is_expected.to have_attributes(errors: ["`type` should be set, in #{changelog_path}! #{described_class::SEE_DOC}"]) } + end + + context "when on a security MR" do + let(:yaml_merge_request) { '' } + + before do + allow(fake_helper).to receive(:security_mr?).and_return(true) + end + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + end + + context "when MR IID is empty" do + before do + allow(fake_helper).to receive(:mr_iid).and_return("") + end + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + end + + context "when YAML MR IID is empty" do + let(:yaml_merge_request) { '' } + + context "and YAML includes a merge_request: line" do + it { is_expected.to have_attributes(markdowns: [{ msg: format(described_class::SUGGEST_MR_COMMENT, mr_iid: fake_helper.mr_iid), file: changelog_path, line: 3 }]) } + end + + context "and YAML does not include a merge_request: line" do + let(:yaml) do + <<~YAML + --- + title: #{yaml_title} + author: + type: #{yaml_type} + YAML + end + + it { is_expected.to have_attributes(messages: ["Consider setting `merge_request` to #{mr_iid} in #{changelog_path}. #{described_class::SEE_DOC}"]) } + end + end + end + + describe '#check_changelog_path' do + let(:changelog_path) { 'changelog-path.yml' } + let(:foss_change) { nil } + let(:ee_change) { nil } + let(:changelog_change) { nil } + let(:changes) { changes_class.new([foss_change, ee_change, changelog_change].compact) } + + before do + allow(changelog).to receive(:present?).and_return(true) + end + + subject { changelog.check_changelog_path } + + context "when changelog is not present" do + before do + allow(changelog).to receive(:present?).and_return(false) + end + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + end + + context "with EE changes" do + let(:ee_change) { change_class.new('ee/app/models/foo.rb', :added, :backend) } + + context "and a non-EE changelog, and changelog not required" do + let(:changelog_change) { change_class.new('changelogs/unreleased/entry.yml', :added, :changelog) } + + before do + allow(changelog).to receive(:required?).and_return(false) + end + + it { is_expected.to have_attributes(warnings: ["This MR has a Changelog file outside `ee/`, but code changes in `ee/`. Consider moving the Changelog file into `ee/`."]) } + end + + context "and a EE changelog" do + let(:changelog_change) { change_class.new('ee/changelogs/unreleased/entry.yml', :added, :changelog) } + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + + context "and there are DB changes" do + let(:foss_change) { change_class.new('db/migrate/foo.rb', :added, :migration) } + + it { is_expected.to have_attributes(warnings: ["This MR has a Changelog file inside `ee/`, but there are database changes which [requires](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry) the Changelog placement to be outside of `ee/`. Consider moving the Changelog file outside `ee/`."]) } + end + end + end + + context "with no EE changes" do + let(:foss_change) { change_class.new('app/models/foo.rb', :added, :backend) } + + context "and a non-EE changelog" do + let(:changelog_change) { change_class.new('changelogs/unreleased/entry.yml', :added, :changelog) } + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + end + + context "and a EE changelog" do + let(:changelog_change) { change_class.new('ee/changelogs/unreleased/entry.yml', :added, :changelog) } + + it { is_expected.to have_attributes(warnings: ["This MR has a Changelog file in `ee/`, but no code changes in `ee/`. Consider moving the Changelog file outside `ee/`."]) } + end + end + end + describe '#required_reasons' do subject { changelog.required_reasons } @@ -126,8 +324,8 @@ RSpec.describe Tooling::Danger::Changelog do end end - describe '#found' do - subject { changelog.found } + describe '#present?' do + subject { changelog.present? } context 'added files contain a changelog' do let(:changes) { changes_class.new([change_class.new('foo', :added, :changelog)]) } @@ -138,7 +336,7 @@ RSpec.describe Tooling::Danger::Changelog do context 'added files do not contain a changelog' do let(:changes) { changes_class.new([change_class.new('foo', :added, :backend)]) } - it { is_expected.to eq(nil) } + it { is_expected.to be_falsy } end end @@ -158,6 +356,22 @@ RSpec.describe Tooling::Danger::Changelog do end end + describe '#changelog_path' do + subject { changelog.changelog_path } + + context 'added files contain a changelog' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :changelog)]) } + + it { is_expected.to eq('foo') } + end + + context 'added files do not contain a changelog' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :backend)]) } + + it { is_expected.to be_nil } + end + end + describe '#modified_text' do subject { changelog.modified_text } diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb index 5d106f08402..1d2ea0f5ba3 100644 --- a/spec/tooling/danger/project_helper_spec.rb +++ b/spec/tooling/danger/project_helper_spec.rb @@ -220,7 +220,7 @@ RSpec.describe Tooling::Danger::ProjectHelper do 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: changelog, changes_size, commit_messages, database, datateam, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css') + expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changelog, commit_messages, database, datateam, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css') end end @@ -256,7 +256,9 @@ RSpec.describe Tooling::Danger::ProjectHelper do subject { project_helper.all_ee_changes } it 'returns all changed files starting with ee/' do - expect(fake_helper).to receive(:all_changed_files).and_return(%w[fr/ee/beer.rb ee/wine.rb ee/lib/ido.rb ee.k]) + changes = double + expect(project_helper).to receive(:changes).and_return(changes) + expect(changes).to receive(:files).and_return(%w[fr/ee/beer.rb ee/wine.rb ee/lib/ido.rb ee.k]) is_expected.to match_array(%w[ee/wine.rb ee/lib/ido.rb]) end -- cgit v1.2.3