summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2023-03-03 17:59:07 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2023-03-03 17:59:07 +0000
commitfc0a2622a00b99a0b20ac7cfd0342879c1e3044a (patch)
treef3aa0a77dcb356a877c9e02dde3dd43072b7a1ba
parent623e40c9bbb8b9c8cbca620ad3a663b33c15abcf (diff)
downloadgitlab-ce-fc0a2622a00b99a0b20ac7cfd0342879c1e3044a.tar.gz
Add latest changes from gitlab-org/gitlab@15-9-stable-ee
-rw-r--r--.gitlab/merge_request_templates/Stable Branch.md18
-rw-r--r--danger/qa_selector/Dangerfile2
-rw-r--r--danger/stable_branch_patch/Dangerfile4
-rw-r--r--danger/z_metadata/Dangerfile6
-rw-r--r--spec/tooling/danger/stable_branch_spec.rb99
-rw-r--r--tooling/danger/stable_branch.rb45
6 files changed, 125 insertions, 49 deletions
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 @@
+<!--
+Merging into stable branches is reserved for GitLab patch releases
+https://docs.gitlab.com/ee/policy/maintenance.html#patch-releases
+-->
+
+## 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