diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-03-16 18:18:33 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-03-16 18:18:33 +0000 |
commit | f64a639bcfa1fc2bc89ca7db268f594306edfd7c (patch) | |
tree | a2c3c2ebcc3b45e596949db485d6ed18ffaacfa1 /spec/tooling | |
parent | bfbc3e0d6583ea1a91f627528bedc3d65ba4b10f (diff) | |
download | gitlab-ce-f64a639bcfa1fc2bc89ca7db268f594306edfd7c.tar.gz |
Add latest changes from gitlab-org/gitlab@13-10-stable-eev13.10.0-rc40
Diffstat (limited to 'spec/tooling')
22 files changed, 1194 insertions, 2253 deletions
diff --git a/spec/tooling/danger/base_linter_spec.rb b/spec/tooling/danger/base_linter_spec.rb deleted file mode 100644 index 54d8f3dc1f7..00000000000 --- a/spec/tooling/danger/base_linter_spec.rb +++ /dev/null @@ -1,192 +0,0 @@ -# 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 diff --git a/spec/tooling/danger/changelog_spec.rb b/spec/tooling/danger/changelog_spec.rb index c0eca67ce92..b74039b3cd1 100644 --- a/spec/tooling/danger/changelog_spec.rb +++ b/spec/tooling/danger/changelog_spec.rb @@ -1,51 +1,76 @@ # frozen_string_literal: true -require_relative 'danger_spec_helper' +require 'gitlab-dangerfiles' +require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/changelog' +require_relative '../../../tooling/danger/project_helper' RSpec.describe Tooling::Danger::Changelog do - include DangerSpecHelper + include_context "with dangerfile" - let(:added_files) { nil } - let(:fake_git) { double('fake-git', added_files: added_files) } + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:fake_project_helper) { double('fake-project-helper', helper: fake_helper).tap { |h| h.class.include(Tooling::Danger::ProjectHelper) } } - let(:mr_labels) { nil } - let(:mr_json) { nil } - let(:fake_gitlab) { double('fake-gitlab', mr_labels: mr_labels, mr_json: mr_json) } + subject(:changelog) { fake_danger.new(helper: fake_helper) } - 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?) } + before do + allow(changelog).to receive(:project_helper).and_return(fake_project_helper) + end + + describe '#required_reasons' do + subject { changelog.required_reasons } + + context "added files contain a migration" do + let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } + + it { is_expected.to include(:db_changes) } + end + + context "removed files contains a feature flag" do + let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } - let(:fake_danger) { new_fake_danger.include(described_class) } + it { is_expected.to include(:feature_flag_removed) } + end + + context "added files do not contain a migration" do + let(:changes) { changes_class.new([change_class.new('foo', :added, :frontend)]) } + + it { is_expected.to be_empty } + end + + context "removed files do not contain a feature flag" do + let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) } - subject(:changelog) { fake_danger.new(git: fake_git, gitlab: fake_gitlab, helper: fake_helper) } + it { is_expected.to be_empty } + end + end describe '#required?' do subject { changelog.required? } context 'added files contain a migration' do - [ - 'db/migrate/20200000000000_new_migration.rb', - 'db/post_migrate/20200000000000_new_migration.rb' - ].each do |file_path| - let(:added_files) { [file_path] } + let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } - it { is_expected.to be_truthy } - end + it { is_expected.to be_truthy } + end + + context "removed files contains a feature flag" do + let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } + + it { is_expected.to be_truthy } end context 'added files do not contain a migration' do - [ - 'app/models/model.rb', - 'app/assets/javascripts/file.js' - ].each do |file_path| - let(:added_files) { [file_path] } + let(:changes) { changes_class.new([change_class.new('foo', :added, :frontend)]) } - it { is_expected.to be_falsey } - end + it { is_expected.to be_falsey } + end + + context "removed files do not contain a feature flag" do + let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) } + + it { is_expected.to be_falsey } end end @@ -58,8 +83,7 @@ RSpec.describe Tooling::Danger::Changelog do subject { changelog.optional? } context 'when MR contains only categories requiring no changelog' do - let(:changes_by_category) { { category_without_changelog => nil } } - let(:mr_labels) { [] } + let(:changes) { changes_class.new([change_class.new('foo', :modified, category_without_changelog)]) } it 'is falsey' do is_expected.to be_falsy @@ -67,7 +91,7 @@ RSpec.describe Tooling::Danger::Changelog do end context 'when MR contains a label that require no changelog' do - let(:changes_by_category) { { category_with_changelog => nil } } + let(:changes) { changes_class.new([change_class.new('foo', :modified, category_with_changelog)]) } let(:mr_labels) { [label_with_changelog, label_without_changelog] } it 'is falsey' do @@ -76,29 +100,28 @@ RSpec.describe Tooling::Danger::Changelog do end context 'when MR contains a category that require changelog and a category that require no changelog' do - let(:changes_by_category) { { category_with_changelog => nil, category_without_changelog => nil } } - let(:mr_labels) { [] } + let(:changes) { changes_class.new([change_class.new('foo', :modified, category_with_changelog), change_class.new('foo', :modified, category_without_changelog)]) } - it 'is truthy' do - is_expected.to be_truthy + context 'with no labels' do + it 'is truthy' do + is_expected.to be_truthy + end end - end - context 'when MR contains a category that require changelog and a category that require no changelog with changelog label' do - let(:changes_by_category) { { category_with_changelog => nil, category_without_changelog => nil } } - let(:mr_labels) { ['feature'] } + context 'with changelog label' do + let(:mr_labels) { ['feature'] } - it 'is truthy' do - is_expected.to be_truthy + it 'is truthy' do + is_expected.to be_truthy + end end - end - context 'when MR contains a category that require changelog and a category that require no changelog with no changelog label' do - let(:changes_by_category) { { category_with_changelog => nil, category_without_changelog => nil } } - let(:mr_labels) { ['tooling'] } + context 'with no changelog label' do + let(:mr_labels) { ['tooling'] } - it 'is truthy' do - is_expected.to be_falsey + it 'is truthy' do + is_expected.to be_falsey + end end end end @@ -107,54 +130,39 @@ RSpec.describe Tooling::Danger::Changelog do subject { changelog.found } context 'added files contain a changelog' do - [ - 'changelogs/unreleased/entry.yml', - 'ee/changelogs/unreleased/entry.yml' - ].each do |file_path| - let(:added_files) { [file_path] } + let(:changes) { changes_class.new([change_class.new('foo', :added, :changelog)]) } - it { is_expected.to be_truthy } - end + it { is_expected.to be_truthy } 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 + let(:changes) { changes_class.new([change_class.new('foo', :added, :backend)]) } + + it { is_expected.to eq(nil) } end end describe '#ee_changelog?' do subject { changelog.ee_changelog? } - before do - allow(changelog).to receive(:found).and_return(file_path) - end - context 'is ee changelog' do - let(:file_path) { 'ee/changelogs/unreleased/entry.yml' } + let(:changes) { changes_class.new([change_class.new('ee/changelogs/unreleased/entry.yml', :added, :changelog)]) } it { is_expected.to be_truthy } end context 'is not ee changelog' do - let(:file_path) { 'changelogs/unreleased/entry.yml' } + let(:changes) { changes_class.new([change_class.new('changelogs/unreleased/entry.yml', :added, :changelog)]) } it { is_expected.to be_falsy } end end describe '#modified_text' do - let(:mr_json) { { "iid" => 1234, "title" => sanitize_mr_title } } - subject { changelog.modified_text } context "when title is not changed from sanitization", :aggregate_failures do - let(:sanitize_mr_title) { 'Fake Title' } + let(:mr_title) { 'Fake Title' } specify do expect(subject).to include('CHANGELOG.md was edited') @@ -164,7 +172,7 @@ RSpec.describe Tooling::Danger::Changelog do end context "when title needs sanitization", :aggregate_failures do - let(:sanitize_mr_title) { 'DRAFT: Fake Title' } + let(:mr_title) { 'DRAFT: Fake Title' } specify do expect(subject).to include('CHANGELOG.md was edited') @@ -174,39 +182,46 @@ RSpec.describe Tooling::Danger::Changelog do end end - describe '#required_text' do - let(:mr_json) { { "iid" => 1234, "title" => sanitize_mr_title } } - - subject { changelog.required_text } + describe '#required_texts' do + let(:mr_title) { 'Fake Title' } - context "when title is not changed from sanitization", :aggregate_failures do - let(:sanitize_mr_title) { 'Fake Title' } + subject { changelog.required_texts } + shared_examples 'changelog required text' do |key| specify do - expect(subject).to include('CHANGELOG missing') - expect(subject).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject).not_to include('--ee') + expect(subject).to have_key(key) + expect(subject[key]).to include('CHANGELOG missing') + expect(subject[key]).to include('bin/changelog -m 1234 "Fake Title"') + expect(subject[key]).not_to include('--ee') end end - context "when title needs sanitization", :aggregate_failures do - let(:sanitize_mr_title) { 'DRAFT: Fake Title' } + context 'with a new migration file' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } - specify 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 + it_behaves_like 'changelog required text', :db_changes + end + + context "when title needs sanitization", :aggregate_failures do + let(:mr_title) { 'DRAFT: Fake Title' } + + it_behaves_like 'changelog required text', :db_changes end end + + context 'with a removed feature flag file' do + let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } + + it_behaves_like 'changelog required text', :feature_flag_removed + end end describe '#optional_text' do - let(:mr_json) { { "iid" => 1234, "title" => sanitize_mr_title } } - subject { changelog.optional_text } context "when title is not changed from sanitization", :aggregate_failures do - let(:sanitize_mr_title) { 'Fake Title' } + let(:mr_title) { 'Fake Title' } specify do expect(subject).to include('CHANGELOG missing') @@ -216,7 +231,7 @@ RSpec.describe Tooling::Danger::Changelog do end context "when title needs sanitization", :aggregate_failures do - let(:sanitize_mr_title) { 'DRAFT: Fake Title' } + let(:mr_title) { 'DRAFT: Fake Title' } specify do expect(subject).to include('CHANGELOG missing') diff --git a/spec/tooling/danger/commit_linter_spec.rb b/spec/tooling/danger/commit_linter_spec.rb deleted file mode 100644 index 694e524af21..00000000000 --- a/spec/tooling/danger/commit_linter_spec.rb +++ /dev/null @@ -1,241 +0,0 @@ -# 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 diff --git a/spec/tooling/danger/danger_spec_helper.rb b/spec/tooling/danger/danger_spec_helper.rb deleted file mode 100644 index b1e84b3c13d..00000000000 --- a/spec/tooling/danger/danger_spec_helper.rb +++ /dev/null @@ -1,17 +0,0 @@ -# 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/tooling/danger/emoji_checker_spec.rb b/spec/tooling/danger/emoji_checker_spec.rb deleted file mode 100644 index bbd957b3d00..00000000000 --- a/spec/tooling/danger/emoji_checker_spec.rb +++ /dev/null @@ -1,37 +0,0 @@ -# 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 diff --git a/spec/tooling/danger/feature_flag_spec.rb b/spec/tooling/danger/feature_flag_spec.rb index db63116cc37..5e495cd43c6 100644 --- a/spec/tooling/danger/feature_flag_spec.rb +++ b/spec/tooling/danger/feature_flag_spec.rb @@ -1,29 +1,16 @@ # frozen_string_literal: true -require_relative 'danger_spec_helper' +require 'gitlab-dangerfiles' +require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/feature_flag' RSpec.describe Tooling::Danger::FeatureFlag do - include DangerSpecHelper + include_context "with dangerfile" - let(:added_files) { nil } - 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(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } - 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(: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) } + subject(:feature_flag) { fake_danger.new(git: fake_git) } describe '#feature_flag_files' do let(:feature_flag_files) do diff --git a/spec/tooling/danger/helper_spec.rb b/spec/tooling/danger/helper_spec.rb deleted file mode 100644 index c338d138352..00000000000 --- a/spec/tooling/danger/helper_spec.rb +++ /dev/null @@ -1,682 +0,0 @@ -# frozen_string_literal: true - -require 'fast_spec_helper' -require 'rspec-parameterized' -require_relative 'danger_spec_helper' - -require_relative '../../../tooling/danger/helper' - -RSpec.describe Tooling::Danger::Helper do - using RSpec::Parameterized::TableSyntax - include DangerSpecHelper - - let(:fake_git) { double('fake-git') } - - let(:mr_author) { nil } - let(:fake_gitlab) { double('fake-gitlab', mr_author: mr_author) } - - 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 - 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 - - context 'when danger gitlab plugin is not available' do - it 'returns nil' do - invalid_danger = Class.new do - include Tooling::Danger::Helper - end.new - - expect(invalid_danger.gitlab_helper).to be_nil - 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 } - - it 'interprets a list of changes from the danger git plugin' do - expect(fake_git).to receive(:added_files) { %w[a b c.old] } - expect(fake_git).to receive(:modified_files) { %w[d e] } - expect(fake_git) - .to receive(:renamed_files) - .at_least(:once) - .and_return([{ before: 'c.old', after: 'c.new' }]) - - is_expected.to contain_exactly('a', 'b', 'c.new', 'd', 'e') - end - end - - describe '#changed_lines' do - subject { helper.changed_lines('changed_file.rb') } - - before do - allow(fake_git).to receive(:diff_for_file).with('changed_file.rb').and_return(diff) - end - - context 'when file has diff' do - let(:diff) { double(:diff, patch: "+ # New change here\n+ # New change there") } - - it 'returns file changes' do - is_expected.to eq(['+ # New change here', '+ # New change there']) - end - end - - context 'when file has no diff (renamed without changes)' do - let(:diff) { nil } - - it 'returns a blank array' do - is_expected.to eq([]) - end - end - end - - describe "changed_files" do - it 'returns list of changed files matching given regex' do - expect(helper).to receive(:all_changed_files).and_return(%w[migration.rb usage_data.rb]) - - expect(helper.changed_files(/usage_data/)).to contain_exactly('usage_data.rb') - end - end - - describe '#all_ee_changes' do - subject { helper.all_ee_changes } - - it 'returns all changed files starting with ee/' do - expect(helper).to receive(:all_changed_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 - end - - describe '#ee?' do - subject { helper.ee? } - - it 'returns true if CI_PROJECT_NAME if set to gitlab' do - stub_env('CI_PROJECT_NAME', 'gitlab') - expect(Dir).not_to receive(:exist?) - - is_expected.to be_truthy - end - - it 'delegates to CHANGELOG-EE.md existence if CI_PROJECT_NAME is set to something else' do - stub_env('CI_PROJECT_NAME', 'something else') - expect(Dir).to receive(:exist?).with(File.expand_path('../../../../ee', __dir__)) { true } - - is_expected.to be_truthy - end - - it 'returns true if ee exists' do - stub_env('CI_PROJECT_NAME', nil) - expect(Dir).to receive(:exist?).with(File.expand_path('../../../../ee', __dir__)) { true } - - is_expected.to be_truthy - end - - it "returns false if ee doesn't exist" do - stub_env('CI_PROJECT_NAME', nil) - expect(Dir).to receive(:exist?).with(File.expand_path('../../../../ee', __dir__)) { false } - - is_expected.to be_falsy - end - end - - describe '#project_name' do - subject { helper.project_name } - - it 'returns gitlab if ee? returns true' do - expect(helper).to receive(:ee?) { true } - - is_expected.to eq('gitlab') - end - - it 'returns gitlab-ce if ee? returns false' do - expect(helper).to receive(:ee?) { false } - - is_expected.to eq('gitlab-foss') - end - end - - describe '#markdown_list' do - it 'creates a markdown list of items' do - items = %w[a b] - - expect(helper.markdown_list(items)).to eq("* `a`\n* `b`") - end - - it 'wraps items in <details> when there are more than 10 items' do - items = ('a'..'k').to_a - - expect(helper.markdown_list(items)).to match(%r{<details>[^<]+</details>}) - end - end - - describe '#changes_by_category' do - it 'categorizes changed files' do - expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/migrate/foo lib/gitlab/database/foo.rb qa/foo ee/changelogs/foo.yml] } - allow(fake_git).to receive(:modified_files) { [] } - allow(fake_git).to receive(:renamed_files) { [] } - - expect(helper.changes_by_category).to eq( - backend: %w[foo.rb], - database: %w[db/migrate/foo lib/gitlab/database/foo.rb], - frontend: %w[foo.js], - none: %w[ee/changelogs/foo.yml foo.md], - qa: %w[qa/foo], - unknown: %w[foo] - ) - end - end - - describe '#categories_for_file' do - before do - allow(fake_git).to receive(:diff_for_file).with('usage_data.rb') { double(:diff, patch: "+ count(User.active)") } - end - - where(:path, :expected_categories) do - 'usage_data.rb' | [:database, :backend] - 'doc/foo.md' | [:docs] - 'CONTRIBUTING.md' | [:docs] - 'LICENSE' | [:docs] - 'MAINTENANCE.md' | [:docs] - 'PHILOSOPHY.md' | [:docs] - 'PROCESS.md' | [:docs] - 'README.md' | [:docs] - - 'ee/doc/foo' | [:unknown] - 'ee/README' | [:unknown] - - 'app/assets/foo' | [:frontend] - 'app/views/foo' | [:frontend] - 'public/foo' | [:frontend] - 'scripts/frontend/foo' | [:frontend] - 'spec/javascripts/foo' | [:frontend] - 'spec/frontend/bar' | [:frontend] - 'vendor/assets/foo' | [:frontend] - 'babel.config.js' | [:frontend] - 'jest.config.js' | [:frontend] - 'package.json' | [:frontend] - 'yarn.lock' | [:frontend] - 'config/foo.js' | [:frontend] - 'config/deep/foo.js' | [:frontend] - - 'ee/app/assets/foo' | [:frontend] - 'ee/app/views/foo' | [:frontend] - 'ee/spec/javascripts/foo' | [:frontend] - 'ee/spec/frontend/bar' | [:frontend] - - '.gitlab/ci/frontend.gitlab-ci.yml' | %i[frontend engineering_productivity] - - 'app/models/foo' | [:backend] - 'bin/foo' | [:backend] - 'config/foo' | [:backend] - 'lib/foo' | [:backend] - 'rubocop/foo' | [:backend] - '.rubocop.yml' | [:backend] - '.rubocop_todo.yml' | [:backend] - '.rubocop_manual_todo.yml' | [:backend] - 'spec/foo' | [:backend] - 'spec/foo/bar' | [:backend] - - 'ee/app/foo' | [:backend] - 'ee/bin/foo' | [:backend] - 'ee/spec/foo' | [:backend] - 'ee/spec/foo/bar' | [:backend] - - 'spec/features/foo' | [:test] - 'ee/spec/features/foo' | [:test] - 'spec/support/shared_examples/features/foo' | [:test] - 'ee/spec/support/shared_examples/features/foo' | [:test] - 'spec/support/shared_contexts/features/foo' | [:test] - 'ee/spec/support/shared_contexts/features/foo' | [:test] - 'spec/support/helpers/features/foo' | [:test] - 'ee/spec/support/helpers/features/foo' | [:test] - - 'generator_templates/foo' | [:backend] - 'vendor/languages.yml' | [:backend] - 'file_hooks/examples/' | [:backend] - - 'Gemfile' | [:backend] - 'Gemfile.lock' | [:backend] - 'Rakefile' | [:backend] - 'FOO_VERSION' | [:backend] - - 'Dangerfile' | [:engineering_productivity] - 'danger/commit_messages/Dangerfile' | [:engineering_productivity] - 'ee/danger/commit_messages/Dangerfile' | [:engineering_productivity] - 'danger/commit_messages/' | [:engineering_productivity] - 'ee/danger/commit_messages/' | [:engineering_productivity] - '.gitlab-ci.yml' | [:engineering_productivity] - '.gitlab/ci/cng.gitlab-ci.yml' | [:engineering_productivity] - '.gitlab/ci/ee-specific-checks.gitlab-ci.yml' | [:engineering_productivity] - 'scripts/foo' | [:engineering_productivity] - 'tooling/danger/foo' | [:engineering_productivity] - 'ee/tooling/danger/foo' | [:engineering_productivity] - 'lefthook.yml' | [:engineering_productivity] - '.editorconfig' | [:engineering_productivity] - 'tooling/bin/find_foss_tests' | [:engineering_productivity] - '.codeclimate.yml' | [:engineering_productivity] - '.gitlab/CODEOWNERS' | [:engineering_productivity] - - 'lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml' | [:ci_template] - 'lib/gitlab/ci/templates/dotNET-Core.yml' | [:ci_template] - - 'ee/FOO_VERSION' | [:unknown] - - 'db/schema.rb' | [:database] - 'db/structure.sql' | [:database] - 'db/migrate/foo' | [:database] - 'db/post_migrate/foo' | [:database] - 'ee/db/migrate/foo' | [:database] - 'ee/db/post_migrate/foo' | [:database] - 'ee/db/geo/migrate/foo' | [:database] - 'ee/db/geo/post_migrate/foo' | [:database] - 'app/models/project_authorization.rb' | [:database] - 'app/services/users/refresh_authorized_projects_service.rb' | [:database] - 'lib/gitlab/background_migration.rb' | [:database] - 'lib/gitlab/background_migration/foo' | [:database] - 'ee/lib/gitlab/background_migration/foo' | [:database] - 'lib/gitlab/database.rb' | [:database] - 'lib/gitlab/database/foo' | [:database] - 'ee/lib/gitlab/database/foo' | [:database] - 'lib/gitlab/github_import.rb' | [:database] - 'lib/gitlab/github_import/foo' | [:database] - 'lib/gitlab/sql/foo' | [:database] - 'rubocop/cop/migration/foo' | [:database] - - 'db/fixtures/foo.rb' | [:backend] - 'ee/db/fixtures/foo.rb' | [:backend] - 'doc/api/graphql/reference/gitlab_schema.graphql' | [:backend] - 'doc/api/graphql/reference/gitlab_schema.json' | [:backend] - - 'qa/foo' | [:qa] - 'ee/qa/foo' | [:qa] - - 'changelogs/foo' | [:none] - 'ee/changelogs/foo' | [:none] - 'locale/gitlab.pot' | [:none] - - 'FOO' | [:unknown] - 'foo' | [:unknown] - - 'foo/bar.rb' | [:backend] - 'foo/bar.js' | [:frontend] - 'foo/bar.txt' | [:none] - 'foo/bar.md' | [:none] - end - - with_them do - subject { helper.categories_for_file(path) } - - it { is_expected.to eq(expected_categories) } - end - - context 'having specific changes' do - 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 - - 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(file)).to eq(expected_categories) - end - end - end - end - end - - describe '#label_for_category' do - where(:category, :expected_label) do - :backend | '~backend' - :database | '~database' - :docs | '~documentation' - :foo | '~foo' - :frontend | '~frontend' - :none | '' - :qa | '~QA' - :engineering_productivity | '~"Engineering Productivity" for CI, Danger' - :ci_template | '~"ci::templates"' - end - - with_them do - subject { helper.label_for_category(category) } - - it { is_expected.to eq(expected_label) } - end - end - - describe '#new_teammates' do - it 'returns an array of Teammate' do - usernames = %w[filipa iamphil] - - teammates = helper.new_teammates(usernames) - - expect(teammates.map(&:username)).to eq(usernames) - end - end - - describe '#mr_title' do - it 'returns "" when `gitlab_helper` is unavailable' do - expect(helper).to receive(:gitlab_helper).and_return(nil) - - expect(helper.mr_title).to eq('') - end - - it 'returns the MR title when `gitlab_helper` is available' do - mr_title = 'My MR title' - expect(fake_gitlab).to receive(:mr_json) - .and_return('title' => mr_title) - - expect(helper.mr_title).to eq(mr_title) - end - end - - describe '#mr_web_url' do - it 'returns "" when `gitlab_helper` is unavailable' do - expect(helper).to receive(:gitlab_helper).and_return(nil) - - expect(helper.mr_web_url).to eq('') - end - - it 'returns the MR web_url when `gitlab_helper` is available' do - mr_web_url = 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/1' - expect(fake_gitlab).to receive(:mr_json) - .and_return('web_url' => mr_web_url) - - expect(helper.mr_web_url).to eq(mr_web_url) - end - end - - describe '#mr_target_branch' do - it 'returns "" when `gitlab_helper` is unavailable' do - expect(helper).to receive(:gitlab_helper).and_return(nil) - - expect(helper.mr_target_branch).to eq('') - end - - it 'returns the MR web_url when `gitlab_helper` is available' do - mr_target_branch = 'main' - expect(fake_gitlab).to receive(:mr_json) - .and_return('target_branch' => mr_target_branch) - - expect(helper.mr_target_branch).to eq(mr_target_branch) - end - end - - describe '#security_mr?' do - it 'returns false when on a normal merge request' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('web_url' => 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/1') - - expect(helper).not_to be_security_mr - end - - it 'returns true when on a security merge request' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('web_url' => 'https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1') - - expect(helper).to be_security_mr - end - end - - describe '#draft_mr?' do - it 'returns true for a draft MR' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('title' => 'Draft: My MR title') - - expect(helper).to be_draft_mr - end - - it 'returns false for non draft MR' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('title' => 'My MR title') - - expect(helper).not_to be_draft_mr - end - end - - describe '#cherry_pick_mr?' do - context 'when MR title does not mention a cherry-pick' do - it 'returns false' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('title' => 'Add feature xyz') - - expect(helper).not_to be_cherry_pick_mr - end - end - - context 'when MR title mentions a cherry-pick' do - [ - 'Cherry Pick !1234', - 'cherry-pick !1234', - 'CherryPick !1234' - ].each do |mr_title| - it 'returns true' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('title' => mr_title) - - expect(helper).to be_cherry_pick_mr - end - end - end - end - - describe '#run_all_rspec_mr?' do - context 'when MR title does not mention RUN ALL RSPEC' do - it 'returns false' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('title' => 'Add feature xyz') - - expect(helper).not_to be_run_all_rspec_mr - end - end - - context 'when MR title mentions RUN ALL RSPEC' do - it 'returns true' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('title' => 'Add feature xyz RUN ALL RSPEC') - - expect(helper).to be_run_all_rspec_mr - end - end - end - - describe '#run_as_if_foss_mr?' do - context 'when MR title does not mention RUN AS-IF-FOSS' do - it 'returns false' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('title' => 'Add feature xyz') - - expect(helper).not_to be_run_as_if_foss_mr - end - end - - context 'when MR title mentions RUN AS-IF-FOSS' do - it 'returns true' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('title' => 'Add feature xyz RUN AS-IF-FOSS') - - expect(helper).to be_run_as_if_foss_mr - end - end - end - - describe '#stable_branch?' do - it 'returns false when `gitlab_helper` is unavailable' do - expect(helper).to receive(:gitlab_helper).and_return(nil) - - expect(helper).not_to be_stable_branch - end - - context 'when MR target branch is not a stable branch' do - it 'returns false' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('target_branch' => 'my-feature-branch') - - expect(helper).not_to be_stable_branch - end - end - - context 'when MR target branch is a stable branch' do - %w[ - 13-1-stable-ee - 13-1-stable-ee-patch-1 - ].each do |target_branch| - it 'returns true' do - expect(fake_gitlab).to receive(:mr_json) - .and_return('target_branch' => target_branch) - - expect(helper).to be_stable_branch - end - end - end - end - - describe '#mr_has_label?' do - it 'returns false when `gitlab_helper` is unavailable' do - expect(helper).to receive(:gitlab_helper).and_return(nil) - - expect(helper.mr_has_labels?('telemetry')).to be_falsey - end - - context 'when mr has labels' do - before do - mr_labels = ['telemetry', 'telemetry::reviewed'] - expect(fake_gitlab).to receive(:mr_labels).and_return(mr_labels) - end - - it 'returns true with a matched label' do - expect(helper.mr_has_labels?('telemetry')).to be_truthy - end - - it 'returns false with unmatched label' do - expect(helper.mr_has_labels?('database')).to be_falsey - end - - it 'returns true with an array of labels' do - expect(helper.mr_has_labels?(['telemetry', 'telemetry::reviewed'])).to be_truthy - end - - it 'returns true with multi arguments with matched labels' do - expect(helper.mr_has_labels?('telemetry', 'telemetry::reviewed')).to be_truthy - end - - it 'returns false with multi arguments with unmatched labels' do - expect(helper.mr_has_labels?('telemetry', 'telemetry::non existing')).to be_falsey - end - end - end - - describe '#labels_list' do - let(:labels) { ['telemetry', 'telemetry::reviewed'] } - - it 'composes the labels string' do - expect(helper.labels_list(labels)).to eq('~"telemetry", ~"telemetry::reviewed"') - end - - context 'when passing a separator' do - it 'composes the labels string with the given separator' do - expect(helper.labels_list(labels, sep: ' ')).to eq('~"telemetry" ~"telemetry::reviewed"') - end - end - - it 'returns empty string for empty array' do - expect(helper.labels_list([])).to eq('') - end - end - - describe '#prepare_labels_for_mr' do - it 'composes the labels string' do - mr_labels = ['telemetry', 'telemetry::reviewed'] - - expect(helper.prepare_labels_for_mr(mr_labels)).to eq('/label ~"telemetry" ~"telemetry::reviewed"') - end - - it 'returns empty string for empty array' do - expect(helper.prepare_labels_for_mr([])).to eq('') - end - end - - describe '#has_ci_changes?' do - context 'when .gitlab/ci is changed' do - it 'returns true' do - expect(helper).to receive(:all_changed_files).and_return(%w[migration.rb .gitlab/ci/test.yml]) - - expect(helper.has_ci_changes?).to be_truthy - end - end - - context 'when .gitlab-ci.yml is changed' do - it 'returns true' do - expect(helper).to receive(:all_changed_files).and_return(%w[migration.rb .gitlab-ci.yml]) - - expect(helper.has_ci_changes?).to be_truthy - end - end - - context 'when neither .gitlab/ci/ or .gitlab-ci.yml is changed' do - it 'returns false' do - expect(helper).to receive(:all_changed_files).and_return(%w[migration.rb nested/.gitlab-ci.yml]) - - expect(helper.has_ci_changes?).to be_falsey - end - end - end - - describe '#group_label' do - it 'returns nil when no group label is present' do - expect(helper.group_label(%w[foo bar])).to be_nil - end - - it 'returns the group label when a group label is present' do - expect(helper.group_label(['foo', 'group::source code', 'bar'])).to eq('group::source code') - end - end -end diff --git a/spec/tooling/danger/merge_request_linter_spec.rb b/spec/tooling/danger/merge_request_linter_spec.rb deleted file mode 100644 index 3273b6b3d07..00000000000 --- a/spec/tooling/danger/merge_request_linter_spec.rb +++ /dev/null @@ -1,54 +0,0 @@ -# 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 diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb new file mode 100644 index 00000000000..a8fda901b4a --- /dev/null +++ b/spec/tooling/danger/project_helper_spec.rb @@ -0,0 +1,260 @@ +# frozen_string_literal: true + +require 'rspec-parameterized' +require 'gitlab-dangerfiles' +require 'danger/helper' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../danger/plugins/project_helper' + +RSpec.describe Tooling::Danger::ProjectHelper do + include_context "with dangerfile" + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:fake_helper) { Danger::Helper.new(project_helper) } + + subject(:project_helper) { fake_danger.new(git: fake_git) } + + before do + allow(project_helper).to receive(:helper).and_return(fake_helper) + end + + describe '#changes' do + it 'returns an array of Change objects' do + expect(project_helper.changes).to all(be_an(Gitlab::Dangerfiles::Change)) + end + + it 'groups changes by change type' do + changes = project_helper.changes + + expect(changes.added.files).to eq(added_files) + expect(changes.modified.files).to eq(modified_files) + expect(changes.deleted.files).to eq(deleted_files) + expect(changes.renamed_before.files).to eq([renamed_before_file]) + expect(changes.renamed_after.files).to eq([renamed_after_file]) + end + end + + describe '#categories_for_file' do + using RSpec::Parameterized::TableSyntax + + before do + allow(fake_git).to receive(:diff_for_file).with('usage_data.rb') { double(:diff, patch: "+ count(User.active)") } + end + + where(:path, :expected_categories) do + 'usage_data.rb' | [:database, :backend] + 'doc/foo.md' | [:docs] + 'CONTRIBUTING.md' | [:docs] + 'LICENSE' | [:docs] + 'MAINTENANCE.md' | [:docs] + 'PHILOSOPHY.md' | [:docs] + 'PROCESS.md' | [:docs] + 'README.md' | [:docs] + + 'ee/doc/foo' | [:unknown] + 'ee/README' | [:unknown] + + 'app/assets/foo' | [:frontend] + 'app/views/foo' | [:frontend] + 'public/foo' | [:frontend] + 'scripts/frontend/foo' | [:frontend] + 'spec/javascripts/foo' | [:frontend] + 'spec/frontend/bar' | [:frontend] + 'vendor/assets/foo' | [:frontend] + 'babel.config.js' | [:frontend] + 'jest.config.js' | [:frontend] + 'package.json' | [:frontend] + 'yarn.lock' | [:frontend] + 'config/foo.js' | [:frontend] + 'config/deep/foo.js' | [:frontend] + + 'ee/app/assets/foo' | [:frontend] + 'ee/app/views/foo' | [:frontend] + 'ee/spec/javascripts/foo' | [:frontend] + 'ee/spec/frontend/bar' | [:frontend] + + '.gitlab/ci/frontend.gitlab-ci.yml' | %i[frontend engineering_productivity] + + 'app/models/foo' | [:backend] + 'bin/foo' | [:backend] + 'config/foo' | [:backend] + 'lib/foo' | [:backend] + 'rubocop/foo' | [:backend] + '.rubocop.yml' | [:backend] + '.rubocop_todo.yml' | [:backend] + '.rubocop_manual_todo.yml' | [:backend] + 'spec/foo' | [:backend] + 'spec/foo/bar' | [:backend] + + 'ee/app/foo' | [:backend] + 'ee/bin/foo' | [:backend] + 'ee/spec/foo' | [:backend] + 'ee/spec/foo/bar' | [:backend] + + 'spec/features/foo' | [:test] + 'ee/spec/features/foo' | [:test] + 'spec/support/shared_examples/features/foo' | [:test] + 'ee/spec/support/shared_examples/features/foo' | [:test] + 'spec/support/shared_contexts/features/foo' | [:test] + 'ee/spec/support/shared_contexts/features/foo' | [:test] + 'spec/support/helpers/features/foo' | [:test] + 'ee/spec/support/helpers/features/foo' | [:test] + + 'generator_templates/foo' | [:backend] + 'vendor/languages.yml' | [:backend] + 'file_hooks/examples/' | [:backend] + + 'Gemfile' | [:backend] + 'Gemfile.lock' | [:backend] + 'Rakefile' | [:backend] + 'FOO_VERSION' | [:backend] + + 'Dangerfile' | [:engineering_productivity] + 'danger/commit_messages/Dangerfile' | [:engineering_productivity] + 'ee/danger/commit_messages/Dangerfile' | [:engineering_productivity] + 'danger/commit_messages/' | [:engineering_productivity] + 'ee/danger/commit_messages/' | [:engineering_productivity] + '.gitlab-ci.yml' | [:engineering_productivity] + '.gitlab/ci/cng.gitlab-ci.yml' | [:engineering_productivity] + '.gitlab/ci/ee-specific-checks.gitlab-ci.yml' | [:engineering_productivity] + 'scripts/foo' | [:engineering_productivity] + 'tooling/danger/foo' | [:engineering_productivity] + 'ee/tooling/danger/foo' | [:engineering_productivity] + 'lefthook.yml' | [:engineering_productivity] + '.editorconfig' | [:engineering_productivity] + 'tooling/bin/find_foss_tests' | [:engineering_productivity] + '.codeclimate.yml' | [:engineering_productivity] + '.gitlab/CODEOWNERS' | [:engineering_productivity] + + 'lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml' | [:ci_template] + 'lib/gitlab/ci/templates/dotNET-Core.yml' | [:ci_template] + + 'ee/FOO_VERSION' | [:unknown] + + 'db/schema.rb' | [:database] + 'db/structure.sql' | [:database] + 'db/migrate/foo' | [:database, :migration] + 'db/post_migrate/foo' | [:database, :migration] + 'ee/db/geo/migrate/foo' | [:database, :migration] + 'ee/db/geo/post_migrate/foo' | [:database, :migration] + 'app/models/project_authorization.rb' | [:database] + 'app/services/users/refresh_authorized_projects_service.rb' | [:database] + 'lib/gitlab/background_migration.rb' | [:database] + 'lib/gitlab/background_migration/foo' | [:database] + 'ee/lib/gitlab/background_migration/foo' | [:database] + 'lib/gitlab/database.rb' | [:database] + 'lib/gitlab/database/foo' | [:database] + 'ee/lib/gitlab/database/foo' | [:database] + 'lib/gitlab/github_import.rb' | [:database] + 'lib/gitlab/github_import/foo' | [:database] + 'lib/gitlab/sql/foo' | [:database] + 'rubocop/cop/migration/foo' | [:database] + + 'db/fixtures/foo.rb' | [:backend] + 'ee/db/fixtures/foo.rb' | [:backend] + + 'qa/foo' | [:qa] + 'ee/qa/foo' | [:qa] + + 'changelogs/foo' | [:none] + 'ee/changelogs/foo' | [:none] + 'locale/gitlab.pot' | [:none] + + 'FOO' | [:unknown] + 'foo' | [:unknown] + + 'foo/bar.rb' | [:backend] + 'foo/bar.js' | [:frontend] + 'foo/bar.txt' | [:none] + 'foo/bar.md' | [:none] + end + + with_them do + subject { project_helper.categories_for_file(path) } + + it { is_expected.to eq(expected_categories) } + end + + context 'having specific changes' do + 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 + + 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(project_helper.categories_for_file(file)).to eq(expected_categories) + end + end + end + end + end + + 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, commit_messages, database, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css') + 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 + before do + expect(fake_helper).to receive(:ci?).and_return(false) + end + + it 'returns local only rules' do + expect(project_helper.rule_names).to match_array(described_class::LOCAL_RULES) + end + end + + context 'when running under CI' do + before do + expect(fake_helper).to receive(:ci?).and_return(true) + end + + it 'returns all rules' do + expect(project_helper.rule_names).to eq(described_class::LOCAL_RULES | described_class::CI_ONLY_RULES) + end + end + end + + describe '#all_ee_changes' 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]) + + is_expected.to match_array(%w[ee/wine.rb ee/lib/ido.rb]) + end + end + + describe '#project_name' do + subject { project_helper.project_name } + + it 'returns gitlab if ee? returns true' do + expect(project_helper).to receive(:ee?) { true } + + is_expected.to eq('gitlab') + end + + it 'returns gitlab-ce if ee? returns false' do + expect(project_helper).to receive(:ee?) { false } + + is_expected.to eq('gitlab-foss') + end + end +end diff --git a/spec/tooling/danger/roulette_spec.rb b/spec/tooling/danger/roulette_spec.rb deleted file mode 100644 index 1e500a1ed08..00000000000 --- a/spec/tooling/danger/roulette_spec.rb +++ /dev/null @@ -1,429 +0,0 @@ -# frozen_string_literal: true - -require 'webmock/rspec' -require 'timecop' - -require_relative '../../../tooling/danger/roulette' -require 'active_support/testing/time_helpers' - -RSpec.describe Tooling::Danger::Roulette do - include ActiveSupport::Testing::TimeHelpers - - around do |example| - travel_to(Time.utc(2020, 06, 22, 10)) { example.run } - end - - let(:backend_available) { true } - let(:backend_tz_offset_hours) { 2.0 } - let(:backend_maintainer) do - Tooling::Danger::Teammate.new( - 'username' => 'backend-maintainer', - 'name' => 'Backend maintainer', - 'role' => 'Backend engineer', - 'projects' => { 'gitlab' => 'maintainer backend' }, - 'available' => backend_available, - 'tz_offset_hours' => backend_tz_offset_hours - ) - end - - let(:frontend_reviewer) do - Tooling::Danger::Teammate.new( - 'username' => 'frontend-reviewer', - 'name' => 'Frontend reviewer', - 'role' => 'Frontend engineer', - 'projects' => { 'gitlab' => 'reviewer frontend' }, - 'available' => true, - 'tz_offset_hours' => 2.0 - ) - end - - let(:frontend_maintainer) do - Tooling::Danger::Teammate.new( - 'username' => 'frontend-maintainer', - 'name' => 'Frontend maintainer', - 'role' => 'Frontend engineer', - 'projects' => { 'gitlab' => "maintainer frontend" }, - 'available' => true, - 'tz_offset_hours' => 2.0 - ) - end - - let(:software_engineer_in_test) do - Tooling::Danger::Teammate.new( - 'username' => 'software-engineer-in-test', - 'name' => 'Software Engineer in Test', - 'role' => 'Software Engineer in Test, Create:Source Code', - 'projects' => { 'gitlab' => 'maintainer qa', 'gitlab-qa' => 'maintainer' }, - 'available' => true, - 'tz_offset_hours' => 2.0 - ) - end - - let(:engineering_productivity_reviewer) do - Tooling::Danger::Teammate.new( - 'username' => 'eng-prod-reviewer', - 'name' => 'EP engineer', - 'role' => 'Engineering Productivity', - 'projects' => { 'gitlab' => 'reviewer backend' }, - 'available' => true, - 'tz_offset_hours' => 2.0 - ) - end - - let(:ci_template_reviewer) do - Tooling::Danger::Teammate.new( - 'username' => 'ci-template-maintainer', - 'name' => 'CI Template engineer', - 'role' => '~"ci::templates"', - 'projects' => { 'gitlab' => 'reviewer ci_template' }, - 'available' => true, - 'tz_offset_hours' => 2.0 - ) - end - - let(:teammates) do - [ - backend_maintainer.to_h, - frontend_maintainer.to_h, - frontend_reviewer.to_h, - software_engineer_in_test.to_h, - engineering_productivity_reviewer.to_h, - ci_template_reviewer.to_h - ] - end - - let(:teammate_json) do - teammates.to_json - end - - subject(:roulette) { Object.new.extend(described_class) } - - describe 'Spin#==' do - it 'compares Spin attributes' do - spin1 = described_class::Spin.new(:backend, frontend_reviewer, frontend_maintainer, false, false) - spin2 = described_class::Spin.new(:backend, frontend_reviewer, frontend_maintainer, false, false) - spin3 = described_class::Spin.new(:backend, frontend_reviewer, frontend_maintainer, false, true) - spin4 = described_class::Spin.new(:backend, frontend_reviewer, frontend_maintainer, true, false) - spin5 = described_class::Spin.new(:backend, frontend_reviewer, backend_maintainer, false, false) - spin6 = described_class::Spin.new(:backend, backend_maintainer, frontend_maintainer, false, false) - spin7 = described_class::Spin.new(:frontend, frontend_reviewer, frontend_maintainer, false, false) - - expect(spin1).to eq(spin2) - expect(spin1).not_to eq(spin3) - expect(spin1).not_to eq(spin4) - expect(spin1).not_to eq(spin5) - expect(spin1).not_to eq(spin6) - expect(spin1).not_to eq(spin7) - end - end - - describe '#spin' do - let!(:project) { 'gitlab' } - let!(:mr_source_branch) { 'a-branch' } - let!(:mr_labels) { ['backend', 'devops::create'] } - let!(:author) { Tooling::Danger::Teammate.new('username' => 'johndoe') } - let(:timezone_experiment) { false } - let(:spins) do - # Stub the request at the latest time so that we can modify the raw data, e.g. available fields. - WebMock - .stub_request(:get, described_class::ROULETTE_DATA_URL) - .to_return(body: teammate_json) - - subject.spin(project, categories, timezone_experiment: timezone_experiment) - end - - before do - allow(subject).to receive(:mr_author_username).and_return(author.username) - allow(subject).to receive(:mr_labels).and_return(mr_labels) - allow(subject).to receive(:mr_source_branch).and_return(mr_source_branch) - end - - context 'when timezone_experiment == false' do - context 'when change contains backend category' do - let(:categories) { [:backend] } - - it 'assigns backend reviewer and maintainer' do - expect(spins[0].reviewer).to eq(engineering_productivity_reviewer) - expect(spins[0].maintainer).to eq(backend_maintainer) - expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false)]) - end - - context 'when teammate is not available' do - let(:backend_available) { false } - - it 'assigns backend reviewer and no maintainer' do - expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, nil, false, false)]) - end - end - end - - context 'when change contains frontend category' do - let(:categories) { [:frontend] } - - it 'assigns frontend reviewer and maintainer' do - expect(spins).to eq([described_class::Spin.new(:frontend, frontend_reviewer, frontend_maintainer, false, false)]) - end - end - - context 'when change contains many categories' do - let(:categories) { [:frontend, :test, :qa, :engineering_productivity, :ci_template, :backend] } - - it 'has a deterministic sorting order' do - expect(spins.map(&:category)).to eq categories.sort - end - end - - context 'when change contains QA category' do - let(:categories) { [:qa] } - - it 'assigns QA maintainer' do - expect(spins).to eq([described_class::Spin.new(:qa, nil, software_engineer_in_test, false, false)]) - end - end - - context 'when change contains QA category and another category' do - let(:categories) { [:backend, :qa] } - - it 'assigns QA maintainer' do - expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false), described_class::Spin.new(:qa, nil, software_engineer_in_test, :maintainer, false)]) - end - - context 'and author is an SET' do - let!(:author) { Tooling::Danger::Teammate.new('username' => software_engineer_in_test.username) } - - it 'assigns QA reviewer' do - expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false), described_class::Spin.new(:qa, nil, nil, false, false)]) - end - end - end - - context 'when change contains Engineering Productivity category' do - let(:categories) { [:engineering_productivity] } - - it 'assigns Engineering Productivity reviewer and fallback to backend maintainer' do - expect(spins).to eq([described_class::Spin.new(:engineering_productivity, engineering_productivity_reviewer, backend_maintainer, false, false)]) - end - end - - context 'when change contains CI/CD Template category' do - let(:categories) { [:ci_template] } - - it 'assigns CI/CD Template reviewer and fallback to backend maintainer' do - expect(spins).to eq([described_class::Spin.new(:ci_template, ci_template_reviewer, backend_maintainer, false, false)]) - end - end - - context 'when change contains test category' do - let(:categories) { [:test] } - - it 'assigns corresponding SET' do - expect(spins).to eq([described_class::Spin.new(:test, software_engineer_in_test, nil, :maintainer, false)]) - end - end - end - - context 'when timezone_experiment == true' do - let(:timezone_experiment) { true } - - context 'when change contains backend category' do - let(:categories) { [:backend] } - - it 'assigns backend reviewer and maintainer' do - expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, true)]) - end - - context 'when teammate is not in a good timezone' do - let(:backend_tz_offset_hours) { 5.0 } - - it 'assigns backend reviewer and no maintainer' do - expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, nil, false, true)]) - end - end - end - - context 'when change includes a category with timezone disabled' do - let(:categories) { [:backend] } - - before do - stub_const("#{described_class}::INCLUDE_TIMEZONE_FOR_CATEGORY", backend: false) - end - - it 'assigns backend reviewer and maintainer' do - expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false)]) - end - - context 'when teammate is not in a good timezone' do - let(:backend_tz_offset_hours) { 5.0 } - - it 'assigns backend reviewer and maintainer' do - expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false)]) - end - end - end - end - end - - RSpec::Matchers.define :match_teammates do |expected| - match do |actual| - expected.each do |expected_person| - actual_person_found = actual.find { |actual_person| actual_person.name == expected_person.username } - - actual_person_found && - actual_person_found.name == expected_person.name && - actual_person_found.role == expected_person.role && - actual_person_found.projects == expected_person.projects - end - end - end - - describe '#team' do - subject(:team) { roulette.team } - - context 'HTTP failure' do - before do - WebMock - .stub_request(:get, described_class::ROULETTE_DATA_URL) - .to_return(status: 404) - end - - it 'raises a pretty error' do - expect { team }.to raise_error(/Failed to read/) - end - end - - context 'JSON failure' do - before do - WebMock - .stub_request(:get, described_class::ROULETTE_DATA_URL) - .to_return(body: 'INVALID JSON') - end - - it 'raises a pretty error' do - expect { team }.to raise_error(/Failed to parse/) - end - end - - context 'success' do - before do - WebMock - .stub_request(:get, described_class::ROULETTE_DATA_URL) - .to_return(body: teammate_json) - end - - it 'returns an array of teammates' do - is_expected.to match_teammates([ - backend_maintainer, - frontend_reviewer, - frontend_maintainer, - software_engineer_in_test, - engineering_productivity_reviewer, - ci_template_reviewer - ]) - end - - it 'memoizes the result' do - expect(team.object_id).to eq(roulette.team.object_id) - end - end - end - - describe '#project_team' do - subject { roulette.project_team('gitlab-qa') } - - before do - WebMock - .stub_request(:get, described_class::ROULETTE_DATA_URL) - .to_return(body: teammate_json) - end - - it 'filters team by project_name' do - is_expected.to match_teammates([ - software_engineer_in_test - ]) - end - end - - describe '#spin_for_person' do - let(:person_tz_offset_hours) { 0.0 } - let(:person1) do - Tooling::Danger::Teammate.new( - 'username' => 'user1', - 'available' => true, - 'tz_offset_hours' => person_tz_offset_hours - ) - end - - let(:person2) do - Tooling::Danger::Teammate.new( - 'username' => 'user2', - 'available' => true, - 'tz_offset_hours' => person_tz_offset_hours) - end - - let(:author) do - Tooling::Danger::Teammate.new( - 'username' => 'johndoe', - 'available' => true, - 'tz_offset_hours' => 0.0) - end - - let(:unavailable) do - Tooling::Danger::Teammate.new( - 'username' => 'janedoe', - 'available' => false, - 'tz_offset_hours' => 0.0) - end - - before do - allow(subject).to receive(:mr_author_username).and_return(author.username) - end - - (-4..4).each do |utc_offset| - context "when local hour for person is #{10 + utc_offset} (offset: #{utc_offset})" do - let(:person_tz_offset_hours) { utc_offset } - - [false, true].each do |timezone_experiment| - context "with timezone_experiment == #{timezone_experiment}" do - it 'returns a random person' do - persons = [person1, person2] - - selected = subject.spin_for_person(persons, random: Random.new, timezone_experiment: timezone_experiment) - - expect(persons.map(&:username)).to include(selected.username) - end - end - end - end - end - - ((-12..-5).to_a + (5..12).to_a).each do |utc_offset| - context "when local hour for person is #{10 + utc_offset} (offset: #{utc_offset})" do - let(:person_tz_offset_hours) { utc_offset } - - [false, true].each do |timezone_experiment| - context "with timezone_experiment == #{timezone_experiment}" do - it 'returns a random person or nil' do - persons = [person1, person2] - - selected = subject.spin_for_person(persons, random: Random.new, timezone_experiment: timezone_experiment) - - if timezone_experiment - expect(selected).to be_nil - else - expect(persons.map(&:username)).to include(selected.username) - end - end - end - end - end - end - - it 'excludes unavailable persons' do - expect(subject.spin_for_person([unavailable], random: Random.new)).to be_nil - end - - it 'excludes mr.author' do - expect(subject.spin_for_person([author], random: Random.new)).to be_nil - end - end -end diff --git a/spec/tooling/danger/sidekiq_queues_spec.rb b/spec/tooling/danger/sidekiq_queues_spec.rb index c5fc8592621..9bffc7ee93d 100644 --- a/spec/tooling/danger/sidekiq_queues_spec.rb +++ b/spec/tooling/danger/sidekiq_queues_spec.rb @@ -1,20 +1,21 @@ # frozen_string_literal: true require 'rspec-parameterized' -require_relative 'danger_spec_helper' +require 'gitlab-dangerfiles' +require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/sidekiq_queues' RSpec.describe Tooling::Danger::SidekiqQueues do - using RSpec::Parameterized::TableSyntax - include DangerSpecHelper + include_context "with dangerfile" - let(:fake_git) { double('fake-git') } - let(:fake_danger) { new_fake_danger.include(described_class) } + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } subject(:sidekiq_queues) { fake_danger.new(git: fake_git) } describe '#changed_queue_files' do + using RSpec::Parameterized::TableSyntax + 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) | %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml) diff --git a/spec/tooling/danger/teammate_spec.rb b/spec/tooling/danger/teammate_spec.rb deleted file mode 100644 index f3afdc6e912..00000000000 --- a/spec/tooling/danger/teammate_spec.rb +++ /dev/null @@ -1,225 +0,0 @@ -# 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 diff --git a/spec/tooling/danger/title_linting_spec.rb b/spec/tooling/danger/title_linting_spec.rb deleted file mode 100644 index 7bc1684cd87..00000000000 --- a/spec/tooling/danger/title_linting_spec.rb +++ /dev/null @@ -1,91 +0,0 @@ -# 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 diff --git a/spec/tooling/danger/weightage/maintainers_spec.rb b/spec/tooling/danger/weightage/maintainers_spec.rb deleted file mode 100644 index b99ffe706a4..00000000000 --- a/spec/tooling/danger/weightage/maintainers_spec.rb +++ /dev/null @@ -1,34 +0,0 @@ -# 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 diff --git a/spec/tooling/danger/weightage/reviewers_spec.rb b/spec/tooling/danger/weightage/reviewers_spec.rb deleted file mode 100644 index 5693ce7a10c..00000000000 --- a/spec/tooling/danger/weightage/reviewers_spec.rb +++ /dev/null @@ -1,63 +0,0 @@ -# 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 diff --git a/spec/tooling/gitlab_danger_spec.rb b/spec/tooling/gitlab_danger_spec.rb deleted file mode 100644 index 20ac40d1d2a..00000000000 --- a/spec/tooling/gitlab_danger_spec.rb +++ /dev/null @@ -1,76 +0,0 @@ -# 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 diff --git a/spec/tooling/rspec_flaky/config_spec.rb b/spec/tooling/rspec_flaky/config_spec.rb new file mode 100644 index 00000000000..12b5ed74cb2 --- /dev/null +++ b/spec/tooling/rspec_flaky/config_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require 'rspec-parameterized' +require_relative '../../support/helpers/stub_env' + +require_relative '../../../tooling/rspec_flaky/config' + +RSpec.describe RspecFlaky::Config, :aggregate_failures do + include StubENV + + before do + # Stub these env variables otherwise specs don't behave the same on the CI + stub_env('FLAKY_RSPEC_GENERATE_REPORT', nil) + stub_env('SUITE_FLAKY_RSPEC_REPORT_PATH', nil) + stub_env('FLAKY_RSPEC_REPORT_PATH', nil) + stub_env('NEW_FLAKY_RSPEC_REPORT_PATH', nil) + # Ensure the behavior is the same locally and on CI (where Rails is defined since we run this test as part of the whole suite), i.e. Rails isn't defined + allow(described_class).to receive(:rails_path).and_wrap_original do |method, path| + path + end + end + + describe '.generate_report?' do + context "when ENV['FLAKY_RSPEC_GENERATE_REPORT'] is not set" do + it 'returns false' do + expect(described_class).not_to be_generate_report + end + end + + context "when ENV['FLAKY_RSPEC_GENERATE_REPORT'] is set" do + using RSpec::Parameterized::TableSyntax + + where(:env_value, :result) do + '1' | true + 'true' | true + 'foo' | false + '0' | false + 'false' | false + end + + with_them do + before do + stub_env('FLAKY_RSPEC_GENERATE_REPORT', env_value) + end + + it 'returns false' do + expect(described_class.generate_report?).to be(result) + end + end + end + end + + describe '.suite_flaky_examples_report_path' do + context "when ENV['SUITE_FLAKY_RSPEC_REPORT_PATH'] is not set" do + it 'returns the default path' do + expect(described_class.suite_flaky_examples_report_path).to eq('rspec_flaky/suite-report.json') + end + end + + context "when ENV['SUITE_FLAKY_RSPEC_REPORT_PATH'] is set" do + before do + stub_env('SUITE_FLAKY_RSPEC_REPORT_PATH', 'foo/suite-report.json') + end + + it 'returns the value of the env variable' do + expect(described_class.suite_flaky_examples_report_path).to eq('foo/suite-report.json') + end + end + end + + describe '.flaky_examples_report_path' do + context "when ENV['FLAKY_RSPEC_REPORT_PATH'] is not set" do + it 'returns the default path' do + expect(described_class.flaky_examples_report_path).to eq('rspec_flaky/report.json') + end + end + + context "when ENV['FLAKY_RSPEC_REPORT_PATH'] is set" do + before do + stub_env('FLAKY_RSPEC_REPORT_PATH', 'foo/report.json') + end + + it 'returns the value of the env variable' do + expect(described_class.flaky_examples_report_path).to eq('foo/report.json') + end + end + end + + describe '.new_flaky_examples_report_path' do + context "when ENV['NEW_FLAKY_RSPEC_REPORT_PATH'] is not set" do + it 'returns the default path' do + expect(described_class.new_flaky_examples_report_path).to eq('rspec_flaky/new-report.json') + end + end + + context "when ENV['NEW_FLAKY_RSPEC_REPORT_PATH'] is set" do + before do + stub_env('NEW_FLAKY_RSPEC_REPORT_PATH', 'foo/new-report.json') + end + + it 'returns the value of the env variable' do + expect(described_class.new_flaky_examples_report_path).to eq('foo/new-report.json') + end + end + end +end diff --git a/spec/tooling/rspec_flaky/example_spec.rb b/spec/tooling/rspec_flaky/example_spec.rb new file mode 100644 index 00000000000..8ff280fd855 --- /dev/null +++ b/spec/tooling/rspec_flaky/example_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require_relative '../../../tooling/rspec_flaky/example' + +RSpec.describe RspecFlaky::Example do + let(:example_attrs) do + { + id: 'spec/foo/bar_spec.rb:2', + metadata: { + file_path: 'spec/foo/bar_spec.rb', + line_number: 2, + full_description: 'hello world' + }, + execution_result: double(status: 'passed', exception: 'BOOM!'), + attempts: 1 + } + end + + let(:rspec_example) { double(example_attrs) } + + describe '#initialize' do + shared_examples 'a valid Example instance' do + it 'returns valid attributes' do + example = described_class.new(args) + + expect(example.example_id).to eq(example_attrs[:id]) + end + end + + context 'when given an Rspec::Core::Example that responds to #example' do + let(:args) { double(example: rspec_example) } + + it_behaves_like 'a valid Example instance' + end + + context 'when given an Rspec::Core::Example that does not respond to #example' do + let(:args) { rspec_example } + + it_behaves_like 'a valid Example instance' + end + end + + subject { described_class.new(rspec_example) } + + describe '#uid' do + it 'returns a hash of the full description' do + expect(subject.uid).to eq(Digest::MD5.hexdigest("#{subject.description}-#{subject.file}")) + end + end + + describe '#example_id' do + it 'returns the ID of the RSpec::Core::Example' do + expect(subject.example_id).to eq(rspec_example.id) + end + end + + describe '#attempts' do + it 'returns the attempts of the RSpec::Core::Example' do + expect(subject.attempts).to eq(rspec_example.attempts) + end + end + + describe '#file' do + it 'returns the metadata[:file_path] of the RSpec::Core::Example' do + expect(subject.file).to eq(rspec_example.metadata[:file_path]) + end + end + + describe '#line' do + it 'returns the metadata[:line_number] of the RSpec::Core::Example' do + expect(subject.line).to eq(rspec_example.metadata[:line_number]) + end + end + + describe '#description' do + it 'returns the metadata[:full_description] of the RSpec::Core::Example' do + expect(subject.description).to eq(rspec_example.metadata[:full_description]) + end + end + + describe '#status' do + it 'returns the execution_result.status of the RSpec::Core::Example' do + expect(subject.status).to eq(rspec_example.execution_result.status) + end + end + + describe '#exception' do + it 'returns the execution_result.exception of the RSpec::Core::Example' do + expect(subject.exception).to eq(rspec_example.execution_result.exception) + end + end +end diff --git a/spec/tooling/rspec_flaky/flaky_example_spec.rb b/spec/tooling/rspec_flaky/flaky_example_spec.rb new file mode 100644 index 00000000000..ab652662c0b --- /dev/null +++ b/spec/tooling/rspec_flaky/flaky_example_spec.rb @@ -0,0 +1,185 @@ +# frozen_string_literal: true + +require 'active_support/testing/time_helpers' +require_relative '../../support/helpers/stub_env' + +require_relative '../../../tooling/rspec_flaky/flaky_example' + +RSpec.describe RspecFlaky::FlakyExample, :aggregate_failures do + include ActiveSupport::Testing::TimeHelpers + include StubENV + + let(:flaky_example_attrs) do + { + example_id: 'spec/foo/bar_spec.rb:2', + file: 'spec/foo/bar_spec.rb', + line: 2, + description: 'hello world', + first_flaky_at: 1234, + last_flaky_at: 2345, + last_flaky_job: 'https://gitlab.com/gitlab-org/gitlab-foss/-/jobs/12', + last_attempts_count: 2, + flaky_reports: 1 + } + end + + let(:example_attrs) do + { + uid: 'abc123', + example_id: flaky_example_attrs[:example_id], + file: flaky_example_attrs[:file], + line: flaky_example_attrs[:line], + description: flaky_example_attrs[:description], + status: 'passed', + exception: 'BOOM!', + attempts: flaky_example_attrs[:last_attempts_count] + } + end + + let(:example) { OpenStruct.new(example_attrs) } + + before do + # Stub these env variables otherwise specs don't behave the same on the CI + stub_env('CI_PROJECT_URL', nil) + stub_env('CI_JOB_ID', nil) + end + + describe '#initialize' do + shared_examples 'a valid FlakyExample instance' do + let(:flaky_example) { described_class.new(args) } + + it 'returns valid attributes' do + expect(flaky_example.uid).to eq(flaky_example_attrs[:uid]) + expect(flaky_example.file).to eq(flaky_example_attrs[:file]) + expect(flaky_example.line).to eq(flaky_example_attrs[:line]) + expect(flaky_example.description).to eq(flaky_example_attrs[:description]) + expect(flaky_example.first_flaky_at).to eq(expected_first_flaky_at) + expect(flaky_example.last_flaky_at).to eq(expected_last_flaky_at) + expect(flaky_example.last_attempts_count).to eq(flaky_example_attrs[:last_attempts_count]) + expect(flaky_example.flaky_reports).to eq(expected_flaky_reports) + end + end + + context 'when given an Rspec::Example' do + it_behaves_like 'a valid FlakyExample instance' do + let(:args) { example } + let(:expected_first_flaky_at) { nil } + let(:expected_last_flaky_at) { nil } + let(:expected_flaky_reports) { 0 } + end + end + + context 'when given a hash' do + it_behaves_like 'a valid FlakyExample instance' do + let(:args) { flaky_example_attrs } + let(:expected_flaky_reports) { flaky_example_attrs[:flaky_reports] } + let(:expected_first_flaky_at) { flaky_example_attrs[:first_flaky_at] } + let(:expected_last_flaky_at) { flaky_example_attrs[:last_flaky_at] } + end + end + end + + describe '#update_flakiness!' do + shared_examples 'an up-to-date FlakyExample instance' do + let(:flaky_example) { described_class.new(args) } + + it 'sets the first_flaky_at if none exists' do + args[:first_flaky_at] = nil + + freeze_time do + flaky_example.update_flakiness! + + expect(flaky_example.first_flaky_at).to eq(Time.now) + end + end + + it 'maintains the first_flaky_at if exists' do + flaky_example.update_flakiness! + expected_first_flaky_at = flaky_example.first_flaky_at + + travel_to(Time.now + 42) do + flaky_example.update_flakiness! + expect(flaky_example.first_flaky_at).to eq(expected_first_flaky_at) + end + end + + it 'updates the last_flaky_at' do + travel_to(Time.now + 42) do + the_future = Time.now + flaky_example.update_flakiness! + + expect(flaky_example.last_flaky_at).to eq(the_future) + end + end + + it 'updates the flaky_reports' do + expected_flaky_reports = flaky_example.first_flaky_at ? flaky_example.flaky_reports + 1 : 1 + + expect { flaky_example.update_flakiness! }.to change { flaky_example.flaky_reports }.by(1) + expect(flaky_example.flaky_reports).to eq(expected_flaky_reports) + end + + context 'when passed a :last_attempts_count' do + it 'updates the last_attempts_count' do + flaky_example.update_flakiness!(last_attempts_count: 42) + + expect(flaky_example.last_attempts_count).to eq(42) + end + end + + context 'when run on the CI' do + before do + stub_env('CI_PROJECT_URL', 'https://gitlab.com/gitlab-org/gitlab-foss') + stub_env('CI_JOB_ID', 42) + end + + it 'updates the last_flaky_job' do + flaky_example.update_flakiness! + + expect(flaky_example.last_flaky_job).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/jobs/42') + end + end + end + + context 'when given an Rspec::Example' do + it_behaves_like 'an up-to-date FlakyExample instance' do + let(:args) { example } + end + end + + context 'when given a hash' do + it_behaves_like 'an up-to-date FlakyExample instance' do + let(:args) { flaky_example_attrs } + end + end + end + + describe '#to_h' do + shared_examples 'a valid FlakyExample hash' do + let(:additional_attrs) { {} } + + it 'returns a valid hash' do + flaky_example = described_class.new(args) + final_hash = flaky_example_attrs.merge(additional_attrs) + + expect(flaky_example.to_h).to eq(final_hash) + end + end + + context 'when given an Rspec::Example' do + let(:args) { example } + + it_behaves_like 'a valid FlakyExample hash' do + let(:additional_attrs) do + { first_flaky_at: nil, last_flaky_at: nil, last_flaky_job: nil, flaky_reports: 0 } + end + end + end + + context 'when given a hash' do + let(:args) { flaky_example_attrs } + + it_behaves_like 'a valid FlakyExample hash' + end + end +end diff --git a/spec/tooling/rspec_flaky/flaky_examples_collection_spec.rb b/spec/tooling/rspec_flaky/flaky_examples_collection_spec.rb new file mode 100644 index 00000000000..823459e31b4 --- /dev/null +++ b/spec/tooling/rspec_flaky/flaky_examples_collection_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require_relative '../../../tooling/rspec_flaky/flaky_examples_collection' + +RSpec.describe RspecFlaky::FlakyExamplesCollection, :aggregate_failures do + let(:collection_hash) do + { + a: { example_id: 'spec/foo/bar_spec.rb:2' }, + b: { example_id: 'spec/foo/baz_spec.rb:3' } + } + end + + let(:collection_report) do + { + a: { + example_id: 'spec/foo/bar_spec.rb:2', + first_flaky_at: nil, + last_flaky_at: nil, + last_flaky_job: nil + }, + b: { + example_id: 'spec/foo/baz_spec.rb:3', + first_flaky_at: nil, + last_flaky_at: nil, + last_flaky_job: nil + } + } + end + + describe '#initialize' do + it 'accepts no argument' do + expect { described_class.new }.not_to raise_error + end + + it 'accepts a hash' do + expect { described_class.new(collection_hash) }.not_to raise_error + end + + it 'does not accept anything else' do + expect { described_class.new([1, 2, 3]) }.to raise_error(ArgumentError, "`collection` must be a Hash, Array given!") + end + end + + describe '#to_h' do + it 'calls #to_h on the values' do + collection = described_class.new(collection_hash) + + expect(collection.to_h).to eq(collection_report) + end + end + + describe '#-' do + it 'returns only examples that are not present in the given collection' do + collection1 = described_class.new(collection_hash) + collection2 = described_class.new( + a: { example_id: 'spec/foo/bar_spec.rb:2' }, + c: { example_id: 'spec/bar/baz_spec.rb:4' }) + + expect((collection2 - collection1).to_h).to eq( + c: { + example_id: 'spec/bar/baz_spec.rb:4', + first_flaky_at: nil, + last_flaky_at: nil, + last_flaky_job: nil + }) + end + + it 'fails if the given collection does not respond to `#key?`' do + collection = described_class.new(collection_hash) + + expect { collection - [1, 2, 3] }.to raise_error(ArgumentError, "`other` must respond to `#key?`, Array does not!") + end + end +end diff --git a/spec/tooling/rspec_flaky/listener_spec.rb b/spec/tooling/rspec_flaky/listener_spec.rb new file mode 100644 index 00000000000..429724a20cf --- /dev/null +++ b/spec/tooling/rspec_flaky/listener_spec.rb @@ -0,0 +1,227 @@ +# frozen_string_literal: true + +require 'active_support/testing/time_helpers' +require_relative '../../support/helpers/stub_env' + +require_relative '../../../tooling/rspec_flaky/listener' + +RSpec.describe RspecFlaky::Listener, :aggregate_failures do + include ActiveSupport::Testing::TimeHelpers + include StubENV + + let(:already_flaky_example_uid) { '6e869794f4cfd2badd93eb68719371d1' } + let(:suite_flaky_example_report) do + { + "#{already_flaky_example_uid}": { + example_id: 'spec/foo/bar_spec.rb:2', + file: 'spec/foo/bar_spec.rb', + line: 2, + description: 'hello world', + first_flaky_at: 1234, + last_flaky_at: 4321, + last_attempts_count: 3, + flaky_reports: 1, + last_flaky_job: nil + } + } + end + + let(:already_flaky_example_attrs) do + { + id: 'spec/foo/bar_spec.rb:2', + metadata: { + file_path: 'spec/foo/bar_spec.rb', + line_number: 2, + full_description: 'hello world' + }, + execution_result: double(status: 'passed', exception: nil) + } + end + + let(:already_flaky_example) { RspecFlaky::FlakyExample.new(suite_flaky_example_report[already_flaky_example_uid]) } + let(:new_example_attrs) do + { + id: 'spec/foo/baz_spec.rb:3', + metadata: { + file_path: 'spec/foo/baz_spec.rb', + line_number: 3, + full_description: 'hello GitLab' + }, + execution_result: double(status: 'passed', exception: nil) + } + end + + before do + # Stub these env variables otherwise specs don't behave the same on the CI + stub_env('CI_PROJECT_URL', nil) + stub_env('CI_JOB_ID', nil) + stub_env('SUITE_FLAKY_RSPEC_REPORT_PATH', nil) + end + + describe '#initialize' do + shared_examples 'a valid Listener instance' do + let(:expected_suite_flaky_examples) { {} } + + it 'returns a valid Listener instance' do + listener = described_class.new + + expect(listener.suite_flaky_examples.to_h).to eq(expected_suite_flaky_examples) + expect(listener.flaky_examples).to eq({}) + end + end + + context 'when no report file exists' do + it_behaves_like 'a valid Listener instance' + end + + context 'when SUITE_FLAKY_RSPEC_REPORT_PATH is set' do + let(:report_file_path) { 'foo/report.json' } + + before do + stub_env('SUITE_FLAKY_RSPEC_REPORT_PATH', report_file_path) + end + + context 'and report file exists' do + before do + expect(File).to receive(:exist?).with(report_file_path).and_return(true) + end + + it 'delegates the load to RspecFlaky::Report' do + report = RspecFlaky::Report.new(RspecFlaky::FlakyExamplesCollection.new(suite_flaky_example_report)) + + expect(RspecFlaky::Report).to receive(:load).with(report_file_path).and_return(report) + expect(described_class.new.suite_flaky_examples.to_h).to eq(report.flaky_examples.to_h) + end + end + + context 'and report file does not exist' do + before do + expect(File).to receive(:exist?).with(report_file_path).and_return(false) + end + + it 'return an empty hash' do + expect(RspecFlaky::Report).not_to receive(:load) + expect(described_class.new.suite_flaky_examples.to_h).to eq({}) + end + end + end + end + + describe '#example_passed' do + let(:rspec_example) { double(new_example_attrs) } + let(:notification) { double(example: rspec_example) } + let(:listener) { described_class.new(suite_flaky_example_report.to_json) } + + shared_examples 'a non-flaky example' do + it 'does not change the flaky examples hash' do + expect { listener.example_passed(notification) } + .not_to change { listener.flaky_examples } + end + end + + shared_examples 'an existing flaky example' do + let(:expected_flaky_example) do + { + example_id: 'spec/foo/bar_spec.rb:2', + file: 'spec/foo/bar_spec.rb', + line: 2, + description: 'hello world', + first_flaky_at: 1234, + last_attempts_count: 2, + flaky_reports: 2, + last_flaky_job: nil + } + end + + it 'changes the flaky examples hash' do + new_example = RspecFlaky::Example.new(rspec_example) + + travel_to(Time.now + 42) do + the_future = Time.now + expect { listener.example_passed(notification) } + .to change { listener.flaky_examples[new_example.uid].to_h } + expect(listener.flaky_examples[new_example.uid].to_h) + .to eq(expected_flaky_example.merge(last_flaky_at: the_future)) + end + end + end + + shared_examples 'a new flaky example' do + let(:expected_flaky_example) do + { + example_id: 'spec/foo/baz_spec.rb:3', + file: 'spec/foo/baz_spec.rb', + line: 3, + description: 'hello GitLab', + last_attempts_count: 2, + flaky_reports: 1, + last_flaky_job: nil + } + end + + it 'changes the all flaky examples hash' do + new_example = RspecFlaky::Example.new(rspec_example) + + travel_to(Time.now + 42) do + the_future = Time.now + expect { listener.example_passed(notification) } + .to change { listener.flaky_examples[new_example.uid].to_h } + expect(listener.flaky_examples[new_example.uid].to_h) + .to eq(expected_flaky_example.merge(first_flaky_at: the_future, last_flaky_at: the_future)) + end + end + end + + describe 'when the RSpec example does not respond to attempts' do + it_behaves_like 'a non-flaky example' + end + + describe 'when the RSpec example has 1 attempt' do + let(:rspec_example) { double(new_example_attrs.merge(attempts: 1)) } + + it_behaves_like 'a non-flaky example' + end + + describe 'when the RSpec example has 2 attempts' do + let(:rspec_example) { double(new_example_attrs.merge(attempts: 2)) } + + it_behaves_like 'a new flaky example' + + context 'with an existing flaky example' do + let(:rspec_example) { double(already_flaky_example_attrs.merge(attempts: 2)) } + + it_behaves_like 'an existing flaky example' + end + end + end + + describe '#dump_summary' do + let(:listener) { described_class.new(suite_flaky_example_report.to_json) } + let(:new_flaky_rspec_example) { double(new_example_attrs.merge(attempts: 2)) } + let(:already_flaky_rspec_example) { double(already_flaky_example_attrs.merge(attempts: 2)) } + let(:notification_new_flaky_rspec_example) { double(example: new_flaky_rspec_example) } + let(:notification_already_flaky_rspec_example) { double(example: already_flaky_rspec_example) } + + before do + allow(Kernel).to receive(:warn) + end + + context 'when a report file path is set by FLAKY_RSPEC_REPORT_PATH' do + it 'delegates the writes to RspecFlaky::Report' do + listener.example_passed(notification_new_flaky_rspec_example) + listener.example_passed(notification_already_flaky_rspec_example) + + report1 = double + report2 = double + + expect(RspecFlaky::Report).to receive(:new).with(listener.flaky_examples).and_return(report1) + expect(report1).to receive(:write).with(RspecFlaky::Config.flaky_examples_report_path) + + expect(RspecFlaky::Report).to receive(:new).with(listener.flaky_examples - listener.suite_flaky_examples).and_return(report2) + expect(report2).to receive(:write).with(RspecFlaky::Config.new_flaky_examples_report_path) + + listener.dump_summary(nil) + end + end + end +end diff --git a/spec/tooling/rspec_flaky/report_spec.rb b/spec/tooling/rspec_flaky/report_spec.rb new file mode 100644 index 00000000000..6c364cd5cd3 --- /dev/null +++ b/spec/tooling/rspec_flaky/report_spec.rb @@ -0,0 +1,135 @@ +# frozen_string_literal: true + +require 'tempfile' + +require_relative '../../../tooling/rspec_flaky/report' + +RSpec.describe RspecFlaky::Report, :aggregate_failures do + let(:thirty_one_days) { 3600 * 24 * 31 } + let(:collection_hash) do + { + a: { example_id: 'spec/foo/bar_spec.rb:2' }, + b: { example_id: 'spec/foo/baz_spec.rb:3', first_flaky_at: (Time.now - thirty_one_days).to_s, last_flaky_at: (Time.now - thirty_one_days).to_s } + } + end + + let(:suite_flaky_example_report) do + { + '6e869794f4cfd2badd93eb68719371d1': { + example_id: 'spec/foo/bar_spec.rb:2', + file: 'spec/foo/bar_spec.rb', + line: 2, + description: 'hello world', + first_flaky_at: 1234, + last_flaky_at: 4321, + last_attempts_count: 3, + flaky_reports: 1, + last_flaky_job: nil + } + } + end + + let(:flaky_examples) { RspecFlaky::FlakyExamplesCollection.new(collection_hash) } + let(:report) { described_class.new(flaky_examples) } + + before do + allow(Kernel).to receive(:warn) + end + + describe '.load' do + let!(:report_file) do + Tempfile.new(%w[rspec_flaky_report .json]).tap do |f| + f.write(JSON.pretty_generate(suite_flaky_example_report)) # rubocop:disable Gitlab/Json + f.rewind + end + end + + after do + report_file.close + report_file.unlink + end + + it 'loads the report file' do + expect(described_class.load(report_file.path).flaky_examples.to_h).to eq(suite_flaky_example_report) + end + end + + describe '.load_json' do + let(:report_json) do + JSON.pretty_generate(suite_flaky_example_report) # rubocop:disable Gitlab/Json + end + + it 'loads the report file' do + expect(described_class.load_json(report_json).flaky_examples.to_h).to eq(suite_flaky_example_report) + end + end + + describe '#initialize' do + it 'accepts a RspecFlaky::FlakyExamplesCollection' do + expect { report }.not_to raise_error + end + + it 'does not accept anything else' do + expect { described_class.new([1, 2, 3]) }.to raise_error(ArgumentError, "`flaky_examples` must be a RspecFlaky::FlakyExamplesCollection, Array given!") + end + end + + it 'delegates to #flaky_examples using SimpleDelegator' do + expect(report.__getobj__).to eq(flaky_examples) + end + + describe '#write' do + let(:report_file_path) { File.join('tmp', 'rspec_flaky_report.json') } + + before do + FileUtils.rm(report_file_path) if File.exist?(report_file_path) + end + + after do + FileUtils.rm(report_file_path) if File.exist?(report_file_path) + end + + context 'when RspecFlaky::Config.generate_report? is false' do + before do + allow(RspecFlaky::Config).to receive(:generate_report?).and_return(false) + end + + it 'does not write any report file' do + report.write(report_file_path) + + expect(File.exist?(report_file_path)).to be(false) + end + end + + context 'when RspecFlaky::Config.generate_report? is true' do + before do + allow(RspecFlaky::Config).to receive(:generate_report?).and_return(true) + end + + it 'delegates the writes to RspecFlaky::Report' do + report.write(report_file_path) + + expect(File.exist?(report_file_path)).to be(true) + expect(File.read(report_file_path)) + .to eq(JSON.pretty_generate(report.flaky_examples.to_h)) # rubocop:disable Gitlab/Json + end + end + end + + describe '#prune_outdated' do + it 'returns a new collection without the examples older than 30 days by default' do + new_report = flaky_examples.to_h.dup.tap { |r| r.delete(:b) } + new_flaky_examples = report.prune_outdated + + expect(new_flaky_examples).to be_a(described_class) + expect(new_flaky_examples.to_h).to eq(new_report) + expect(flaky_examples).to have_key(:b) + end + + it 'accepts a given number of days' do + new_flaky_examples = report.prune_outdated(days: 32) + + expect(new_flaky_examples.to_h).to eq(report.to_h) + end + end +end |