diff options
Diffstat (limited to 'spec/lib/gitlab/danger')
| -rw-r--r-- | spec/lib/gitlab/danger/base_linter_spec.rb | 53 | ||||
| -rw-r--r-- | spec/lib/gitlab/danger/changelog_spec.rb | 71 | ||||
| -rw-r--r-- | spec/lib/gitlab/danger/helper_spec.rb | 56 | ||||
| -rw-r--r-- | spec/lib/gitlab/danger/roulette_spec.rb | 63 | ||||
| -rw-r--r-- | spec/lib/gitlab/danger/teammate_spec.rb | 8 | ||||
| -rw-r--r-- | spec/lib/gitlab/danger/title_linting_spec.rb | 56 | ||||
| -rw-r--r-- | spec/lib/gitlab/danger/weightage/maintainers_spec.rb | 34 | ||||
| -rw-r--r-- | spec/lib/gitlab/danger/weightage/reviewers_spec.rb | 63 |
8 files changed, 276 insertions, 128 deletions
diff --git a/spec/lib/gitlab/danger/base_linter_spec.rb b/spec/lib/gitlab/danger/base_linter_spec.rb index bd0ceb5a125..0136a0278ae 100644 --- a/spec/lib/gitlab/danger/base_linter_spec.rb +++ b/spec/lib/gitlab/danger/base_linter_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'fast_spec_helper' +require 'rspec-parameterized' require_relative 'danger_spec_helper' require 'gitlab/danger/base_linter' @@ -70,19 +71,57 @@ RSpec.describe Gitlab::Danger::BaseLinter do end end - context 'when subject is a WIP' do + context 'when ignoring length issues for subject having not-ready wording' do + using RSpec::Parameterized::TableSyntax + let(:final_message) { 'A B C' } - # commit message with prefix will be over max length. commit message without prefix will be of maximum size - let(:commit_message) { described_class::WIP_PREFIX + final_message + 'D' * (described_class::MAX_LINE_LENGTH - final_message.size) } - it 'does not have any problems' do - commit_linter.lint_subject + 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 - expect(commit_linter.problems).to be_empty + 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 is too short and too long' do + 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 diff --git a/spec/lib/gitlab/danger/changelog_spec.rb b/spec/lib/gitlab/danger/changelog_spec.rb index 2da60f4f8bd..04c515f1205 100644 --- a/spec/lib/gitlab/danger/changelog_spec.rb +++ b/spec/lib/gitlab/danger/changelog_spec.rb @@ -150,41 +150,80 @@ RSpec.describe Gitlab::Danger::Changelog do end describe '#modified_text' do - let(:sanitize_mr_title) { 'Fake Title' } let(:mr_json) { { "iid" => 1234, "title" => sanitize_mr_title } } subject { changelog.modified_text } - it do - expect(subject).to include('CHANGELOG.md was edited') - expect(subject).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + context "when title is not changed from sanitization", :aggregate_failures do + let(:sanitize_mr_title) { 'Fake Title' } + + specify do + expect(subject).to include('CHANGELOG.md was edited') + expect(subject).to include('bin/changelog -m 1234 "Fake Title"') + expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + end + end + + context "when title needs sanitization", :aggregate_failures do + let(:sanitize_mr_title) { 'DRAFT: Fake Title' } + + specify do + expect(subject).to include('CHANGELOG.md was edited') + expect(subject).to include('bin/changelog -m 1234 "Fake Title"') + expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + end end end describe '#required_text' do - let(:sanitize_mr_title) { 'Fake Title' } let(:mr_json) { { "iid" => 1234, "title" => sanitize_mr_title } } subject { changelog.required_text } - it do - expect(subject).to include('CHANGELOG missing') - expect(subject).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject).not_to include('--ee') + context "when title is not changed from sanitization", :aggregate_failures do + let(:sanitize_mr_title) { 'Fake Title' } + + specify do + expect(subject).to include('CHANGELOG missing') + expect(subject).to include('bin/changelog -m 1234 "Fake Title"') + expect(subject).not_to include('--ee') + end + end + + context "when title needs sanitization", :aggregate_failures do + let(:sanitize_mr_title) { 'DRAFT: Fake Title' } + + specify do + expect(subject).to include('CHANGELOG missing') + expect(subject).to include('bin/changelog -m 1234 "Fake Title"') + expect(subject).not_to include('--ee') + end end end - describe 'optional_text' do - let(:sanitize_mr_title) { 'Fake Title' } + describe '#optional_text' do let(:mr_json) { { "iid" => 1234, "title" => sanitize_mr_title } } subject { changelog.optional_text } - it do - expect(subject).to include('CHANGELOG missing') - expect(subject).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + context "when title is not changed from sanitization", :aggregate_failures do + let(:sanitize_mr_title) { 'Fake Title' } + + specify do + expect(subject).to include('CHANGELOG missing') + expect(subject).to include('bin/changelog -m 1234 "Fake Title"') + expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + end + end + + context "when title needs sanitization", :aggregate_failures do + let(:sanitize_mr_title) { 'DRAFT: Fake Title' } + + specify do + expect(subject).to include('CHANGELOG missing') + expect(subject).to include('bin/changelog -m 1234 "Fake Title"') + expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + end end end end diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index a8f113a8cd1..bd5c746dd54 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -351,33 +351,23 @@ RSpec.describe Gitlab::Danger::Helper do end context 'having specific changes' do - it 'has database and backend categories' do - changed_files = ['usage_data.rb', 'lib/gitlab/usage_data.rb', 'ee/lib/ee/gitlab/usage_data.rb'] - - changed_files.each do |file| - allow(fake_git).to receive(:diff_for_file).with(file) { double(:diff, patch: "+ count(User.active)") } - - expect(helper.categories_for_file(file)).to eq([:database, :backend]) - end - end - - it 'has backend category' do - allow(fake_git).to receive(:diff_for_file).with('usage_data.rb') { double(:diff, patch: "+ alt_usage_data(User.active)") } - - expect(helper.categories_for_file('usage_data.rb')).to eq([:backend]) - end - - it 'has backend category for changes outside usage_data files' do - allow(fake_git).to receive(:diff_for_file).with('user.rb') { double(:diff, patch: "+ count(User.active)") } - - expect(helper.categories_for_file('user.rb')).to eq([:backend]) + where(:expected_categories, :patch, :changed_files) do + [:database, :backend] | '+ count(User.active)' | ['usage_data.rb', 'lib/gitlab/usage_data.rb', 'ee/lib/ee/gitlab/usage_data.rb'] + [:database, :backend] | '+ estimate_batch_distinct_count(User.active)' | ['usage_data.rb'] + [:backend] | '+ alt_usage_data(User.active)' | ['usage_data.rb'] + [:backend] | '+ count(User.active)' | ['user.rb'] + [:backend] | '+ count(User.active)' | ['usage_data/topology.rb'] + [:backend] | '+ foo_count(User.active)' | ['usage_data.rb'] end - it 'has backend category for files that are not usage_data.rb' do - changed_file = 'usage_data/topology.rb' - allow(fake_git).to receive(:diff_for_file).with(changed_file) { double(:diff, patch: "+ count(User.active)") } + with_them do + it 'has the correct categories' do + changed_files.each do |file| + allow(fake_git).to receive(:diff_for_file).with(file) { double(:diff, patch: patch) } - expect(helper.categories_for_file(changed_file)).to eq([:backend]) + expect(helper.categories_for_file(file)).to eq(expected_categories) + end + end end end end @@ -412,24 +402,6 @@ RSpec.describe Gitlab::Danger::Helper do end end - 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' - end - - with_them do - subject { helper.sanitize_mr_title(mr_title) } - - it { is_expected.to eq(expected_mr_title) } - end - end - describe '#security_mr?' do it 'returns false when `gitlab_helper` is unavailable' do expect(helper).to receive(:gitlab_helper).and_return(nil) diff --git a/spec/lib/gitlab/danger/roulette_spec.rb b/spec/lib/gitlab/danger/roulette_spec.rb index 561e108bf31..59ac3b12b6b 100644 --- a/spec/lib/gitlab/danger/roulette_spec.rb +++ b/spec/lib/gitlab/danger/roulette_spec.rb @@ -245,69 +245,6 @@ RSpec.describe Gitlab::Danger::Roulette do end end end - - describe 'reviewer suggestion probability' do - let(:reviewer) { teammate_with_capability('reviewer', 'reviewer backend') } - let(:hungry_reviewer) { teammate_with_capability('hungry_reviewer', 'reviewer backend', hungry: true) } - let(:traintainer) { teammate_with_capability('traintainer', 'trainee_maintainer backend') } - let(:hungry_traintainer) { teammate_with_capability('hungry_traintainer', 'trainee_maintainer backend', hungry: true) } - let(:teammates) do - [ - reviewer.to_h, - hungry_reviewer.to_h, - traintainer.to_h, - hungry_traintainer.to_h - ] - end - - let(:categories) { [:backend] } - - # This test is testing probability with inherent randomness. - # The variance is inversely related to sample size - # Given large enough sample size, the variance would be smaller, - # but the test would take longer. - # Given smaller sample size, the variance would be larger, - # but the test would take less time. - let!(:sample_size) { 500 } - let!(:variance) { 0.1 } - - before do - # This test needs actual randomness to simulate probabilities - allow(subject).to receive(:new_random).and_return(Random.new) - WebMock - .stub_request(:get, described_class::ROULETTE_DATA_URL) - .to_return(body: teammate_json) - end - - it 'has 1:2:3:4 probability of picking reviewer, hungry_reviewer, traintainer, hungry_traintainer' do - picks = Array.new(sample_size).map do - spins = subject.spin(project, categories, timezone_experiment: timezone_experiment) - spins.first.reviewer.name - end - - expect(probability(picks, 'reviewer')).to be_within(variance).of(0.1) - expect(probability(picks, 'hungry_reviewer')).to be_within(variance).of(0.2) - expect(probability(picks, 'traintainer')).to be_within(variance).of(0.3) - expect(probability(picks, 'hungry_traintainer')).to be_within(variance).of(0.4) - end - - def probability(picks, role) - picks.count(role).to_f / picks.length - end - - def teammate_with_capability(name, capability, hungry: false) - Gitlab::Danger::Teammate.new( - { - 'name' => name, - 'projects' => { - 'gitlab' => capability - }, - 'available' => true, - 'hungry' => hungry - } - ) - end - end end RSpec::Matchers.define :match_teammates do |expected| diff --git a/spec/lib/gitlab/danger/teammate_spec.rb b/spec/lib/gitlab/danger/teammate_spec.rb index eebe14ed5e1..9c066ba4c1b 100644 --- a/spec/lib/gitlab/danger/teammate_spec.rb +++ b/spec/lib/gitlab/danger/teammate_spec.rb @@ -121,6 +121,14 @@ RSpec.describe Gitlab::Danger::Teammate do 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'] } diff --git a/spec/lib/gitlab/danger/title_linting_spec.rb b/spec/lib/gitlab/danger/title_linting_spec.rb new file mode 100644 index 00000000000..b48d2c5e53d --- /dev/null +++ b/spec/lib/gitlab/danger/title_linting_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rspec-parameterized' + +require 'gitlab/danger/title_linting' + +RSpec.describe Gitlab::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 +end diff --git a/spec/lib/gitlab/danger/weightage/maintainers_spec.rb b/spec/lib/gitlab/danger/weightage/maintainers_spec.rb new file mode 100644 index 00000000000..066bb487fa2 --- /dev/null +++ b/spec/lib/gitlab/danger/weightage/maintainers_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'gitlab/danger/weightage/maintainers' + +RSpec.describe Gitlab::Danger::Weightage::Maintainers do + let(:multiplier) { Gitlab::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) { Gitlab::Danger::Weightage::BASE_REVIEWER_WEIGHT * multiplier } + let(:reduced_capacity_maintainer_count) { Gitlab::Danger::Weightage::BASE_REVIEWER_WEIGHT } + + subject(:weighted_maintainers) { described_class.new(maintainers).execute } + + describe '#execute' do + it 'weights the maintainers overall' do + expect(weighted_maintainers.count).to eq maintainer_count + reduced_capacity_maintainer_count + end + + it 'has total count of regular maintainers' do + expect(weighted_maintainers.count { |r| r.object_id == regular_maintainer.object_id }).to eq maintainer_count + end + + it 'has count of reduced capacity maintainers' do + expect(weighted_maintainers.count { |r| r.object_id == reduced_capacity_maintainer.object_id }).to eq reduced_capacity_maintainer_count + end + end +end diff --git a/spec/lib/gitlab/danger/weightage/reviewers_spec.rb b/spec/lib/gitlab/danger/weightage/reviewers_spec.rb new file mode 100644 index 00000000000..cca81f4d9b5 --- /dev/null +++ b/spec/lib/gitlab/danger/weightage/reviewers_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'gitlab/danger/weightage/reviewers' + +RSpec.describe Gitlab::Danger::Weightage::Reviewers do + let(:multiplier) { Gitlab::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) { Gitlab::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) { Gitlab::Danger::Weightage::BASE_REVIEWER_WEIGHT * multiplier } + let(:traintainer_count) { Gitlab::Danger::Weightage::BASE_REVIEWER_WEIGHT * described_class::TRAINTAINER_WEIGHT * multiplier } + let(:reduced_capacity_reviewer_count) { Gitlab::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 |