From fc0a2622a00b99a0b20ac7cfd0342879c1e3044a Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 3 Mar 2023 17:59:07 +0000 Subject: Add latest changes from gitlab-org/gitlab@15-9-stable-ee --- .gitlab/merge_request_templates/Stable Branch.md | 18 +++++ danger/qa_selector/Dangerfile | 2 + danger/stable_branch_patch/Dangerfile | 4 +- danger/z_metadata/Dangerfile | 6 -- spec/tooling/danger/stable_branch_spec.rb | 99 +++++++++++++++++------- tooling/danger/stable_branch.rb | 45 ++++++++--- 6 files changed, 125 insertions(+), 49 deletions(-) create mode 100644 .gitlab/merge_request_templates/Stable Branch.md diff --git a/.gitlab/merge_request_templates/Stable Branch.md b/.gitlab/merge_request_templates/Stable Branch.md new file mode 100644 index 00000000000..e584296cbb1 --- /dev/null +++ b/.gitlab/merge_request_templates/Stable Branch.md @@ -0,0 +1,18 @@ + + +## What does this MR do and why? + +_Describe in detail what merge request is being backported and why_ + +## MR acceptance checklist + +This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability. + +* [ ] This MR is backporting a bug fix, documentation update, or spec fix, previously merged in the default branch. +* [ ] The original MR has been deployed to GitLab.com (not applicable for documentation or spec changes). +* [ ] Ensure the `e2e:package-and-test` job has either succeeded or been approved by a Software Engineer in Test. + +/assign me diff --git a/danger/qa_selector/Dangerfile b/danger/qa_selector/Dangerfile index b781f390a1a..f049915dc42 100644 --- a/danger/qa_selector/Dangerfile +++ b/danger/qa_selector/Dangerfile @@ -1,5 +1,7 @@ # frozen_string_literal: true +return if helper.stable_branch? + data_qa_selectors = /qa_selector|data-qa-selector/ deprecated_qa_selectors = /(?!.*\bdata-qa-)(?=class=.*qa-.*|class: .*qa-.*)/ diff --git a/danger/stable_branch_patch/Dangerfile b/danger/stable_branch_patch/Dangerfile index 258daa7d1fa..2ac1b14c91f 100644 --- a/danger/stable_branch_patch/Dangerfile +++ b/danger/stable_branch_patch/Dangerfile @@ -1,8 +1,8 @@ # frozen_string_literal: true -if stable_branch.non_security_stable_branch? +if stable_branch.encourage_package_and_qa_execution? markdown(<<~MARKDOWN) - ### QA `e2e:package-and-test` + ## QA `e2e:package-and-test` **@#{helper.mr_author}, the `package-and-test` job must complete before merging this merge request.*** diff --git a/danger/z_metadata/Dangerfile b/danger/z_metadata/Dangerfile index 9140bb7d988..1c36c02b69e 100644 --- a/danger/z_metadata/Dangerfile +++ b/danger/z_metadata/Dangerfile @@ -17,9 +17,3 @@ has_milestone = !gitlab.mr_json["milestone"].nil? unless has_milestone || (helper.security_mr? && helper.mr_target_branch == default_branch) warn "This merge request does not refer to an existing milestone.", sticky: false end - -has_pick_into_stable_label = helper.mr_labels.find { |label| label.start_with?('Pick into') } - -if helper.mr_target_branch != default_branch && !has_pick_into_stable_label && !helper.security_mr? - warn "Most of the time, merge requests should target `#{default_branch}`. Otherwise, please set the relevant `Pick into X.Y` label." -end diff --git a/spec/tooling/danger/stable_branch_spec.rb b/spec/tooling/danger/stable_branch_spec.rb index 9eee077d493..f4008e09ef2 100644 --- a/spec/tooling/danger/stable_branch_spec.rb +++ b/spec/tooling/danger/stable_branch_spec.rb @@ -92,11 +92,19 @@ RSpec.describe Tooling::Danger::StableBranch, feature_category: :delivery do let(:pipeline_bridges_response) do [ - { 'name' => 'e2e:package-and-test', - 'status' => 'success' } + { + 'name' => 'e2e:package-and-test', + 'status' => 'success', + 'downstream_pipeline' => { + 'id' => '123', + 'status' => package_and_qa_state + } + } ] end + let(:package_and_qa_state) { 'success' } + let(:parsed_response) do [ { 'version' => '15.1.1' }, @@ -154,6 +162,13 @@ RSpec.describe Tooling::Danger::StableBranch, feature_category: :delivery do it_behaves_like 'with a failure', described_class::BUG_ERROR_MESSAGE end + context 'with only documentation changes and no bug label' do + let(:bug_label_present) { false } + let(:changes_by_category_response) { { docs: ['foo.md'] } } + + it_behaves_like 'without a failure' + end + context 'with a pipeline:expedite label' do let(:pipeline_expedite_label_present) { true } @@ -169,31 +184,26 @@ RSpec.describe Tooling::Danger::StableBranch, feature_category: :delivery do end context 'when package-and-test job is in manual state' do - described_class::FAILING_PACKAGE_AND_TEST_STATUSES.each do |status| - let(:pipeline_bridges_response) do - [ - { 'name' => 'e2e:package-and-test', - 'status' => status } - ] - end + let(:package_and_qa_state) { 'manual' } - it_behaves_like 'with a failure', described_class::NEEDS_PACKAGE_AND_TEST_MESSAGE - it_behaves_like 'bypassing when flaky test or docs only' - end + it_behaves_like 'with a failure', described_class::NEEDS_PACKAGE_AND_TEST_MESSAGE + it_behaves_like 'bypassing when flaky test or docs only' end context 'when package-and-test job is in a non-successful state' do - let(:pipeline_bridges_response) do - [ - { 'name' => 'e2e:package-and-test', - 'status' => 'running' } - ] - end + let(:package_and_qa_state) { 'running' } it_behaves_like 'with a warning', described_class::WARN_PACKAGE_AND_TEST_MESSAGE it_behaves_like 'bypassing when flaky test or docs only' end + context 'when package-and-test job is canceled' do + let(:package_and_qa_state) { 'canceled' } + + it_behaves_like 'with a failure', described_class::NEEDS_PACKAGE_AND_TEST_MESSAGE + it_behaves_like 'bypassing when flaky test or docs only' + end + context 'when no pipeline is found' do before do allow(gitlab_gem_client).to receive(:mr_json).and_return({}) @@ -266,23 +276,54 @@ RSpec.describe Tooling::Danger::StableBranch, feature_category: :delivery do end end - describe '#non_security_stable_branch?' do - subject { stable_branch.non_security_stable_branch? } - - where(:stable_branch?, :security_mr?, :expected_result) do - true | true | false - false | true | false - true | false | true - false | false | false + describe '#encourage_package_and_qa_execution?' do + subject { stable_branch.encourage_package_and_qa_execution? } + + where(:stable_branch?, :security_mr?, :documentation?, :flaky?, :result) do + # security merge requests + true | true | true | true | false + true | true | true | false | false + true | true | false | true | false + true | true | false | false | false + # canonical merge requests with doc and flaky changes only + true | false | true | true | false + true | false | true | false | false + true | false | false | true | false + # canonical merge requests with app code + true | false | false | false | true end with_them do before do - allow(fake_helper).to receive(:mr_target_branch).and_return(stable_branch? ? '15-1-stable-ee' : 'main') - allow(fake_helper).to receive(:security_mr?).and_return(security_mr?) + allow(fake_helper) + .to receive(:mr_target_branch) + .and_return(stable_branch? ? '15-1-stable-ee' : 'main') + + allow(fake_helper) + .to receive(:security_mr?) + .and_return(security_mr?) + + allow(fake_helper) + .to receive(:has_only_documentation_changes?) + .and_return(documentation?) + + changes_by_category = + if documentation? + { docs: ['foo.md'] } + else + { graphql: ['bar.rb'] } + end + + allow(fake_helper) + .to receive(:changes_by_category) + .and_return(changes_by_category) + + allow(fake_helper) + .to receive(:mr_has_labels?) + .and_return(flaky?) end - it { is_expected.to eq(expected_result) } + it { is_expected.to eq(result) } end end end diff --git a/tooling/danger/stable_branch.rb b/tooling/danger/stable_branch.rb index 8fac1cc5fbf..2d1a4ef0845 100644 --- a/tooling/danger/stable_branch.rb +++ b/tooling/danger/stable_branch.rb @@ -46,21 +46,22 @@ module Tooling MSG NEEDS_PACKAGE_AND_TEST_MESSAGE = <<~MSG - The `e2e:package-and-test` job is not present or needs to be triggered manually. Please start the `e2e:package-and-test` - job and re-run `danger-review`. + The `e2e:package-and-test` job is not present, has been canceled, or needs to be automatically triggered. + Please ensure the job is present in the latest pipeline, if necessary, retry the `danger-review` job. + Read the "QA e2e:package-and-test" section for more details. MSG WARN_PACKAGE_AND_TEST_MESSAGE = <<~MSG - The `e2e:package-and-test` job needs to succeed or have approval from a Software Engineer in Test. See the section below - for more details. + **The `e2e:package-and-test` job needs to succeed or have approval from a Software Engineer in Test.** + Read the "QA e2e:package-and-test" section for more details. MSG # rubocop:disable Style/SignalException def check! - return unless non_security_stable_branch? + return unless valid_stable_branch? fail FEATURE_ERROR_MESSAGE if has_feature_label? - fail BUG_ERROR_MESSAGE unless has_bug_label? + fail BUG_ERROR_MESSAGE unless bug_fixes_only? warn VERSION_WARNING_MESSAGE unless targeting_patchable_version? @@ -78,22 +79,27 @@ module Tooling end # rubocop:enable Style/SignalException - def non_security_stable_branch? - !!stable_target_branch && !helper.security_mr? + def encourage_package_and_qa_execution? + valid_stable_branch? && + !has_only_documentation_changes? && + !has_flaky_failure_label? end private + def valid_stable_branch? + !!stable_target_branch && !helper.security_mr? + end + def package_and_test_status mr_head_pipeline_id = gitlab.mr_json.dig('head_pipeline', 'id') return unless mr_head_pipeline_id - pipeline_bridges = gitlab.api.pipeline_bridges(helper.mr_target_project_id, mr_head_pipeline_id) - package_and_test_pipeline = pipeline_bridges&.find { |j| j['name'] == 'e2e:package-and-test' } + pipeline = package_and_test_pipeline(mr_head_pipeline_id) - return unless package_and_test_pipeline + return unless pipeline - package_and_test_pipeline['status'] + pipeline['status'] end def stable_target_branch @@ -116,6 +122,10 @@ module Tooling helper.mr_has_labels?('failure::flaky-test') end + def bug_fixes_only? + has_bug_label? || has_only_documentation_changes? + end + def has_only_documentation_changes? categories_changed = helper.changes_by_category.keys return false unless categories_changed.size == 1 @@ -192,6 +202,17 @@ module Tooling def version_to_minor_string(version) "#{version[:major]}.#{version[:minor]}" end + + def package_and_test_pipeline(mr_head_pipeline_id) + package_and_test_bridge = gitlab + .api + .pipeline_bridges(helper.mr_target_project_id, mr_head_pipeline_id) + &.find { |bridge| bridge['name'] == 'e2e:package-and-test' } + + return unless package_and_test_bridge + + package_and_test_bridge['downstream_pipeline'] + end end end end -- cgit v1.2.1