diff --git a/spec/tooling/danger/ci_jobs_dependency_validation_spec.rb b/spec/tooling/danger/ci_jobs_dependency_validation_spec.rb
index 140745aed9c374f5d3240d1e26536c57c2ba8b48..1a0bba3da17f68dc1d335b4889971a8abbc623a6 100644
--- a/spec/tooling/danger/ci_jobs_dependency_validation_spec.rb
+++ b/spec/tooling/danger/ci_jobs_dependency_validation_spec.rb
@@ -66,95 +66,116 @@
allow(ci_jobs_dependency_validation.helper).to receive(:mr_target_branch).and_return('master')
allow(ci_jobs_dependency_validation.helper).to receive(:mr_source_project_id).and_return('1')
allow(ci_jobs_dependency_validation.helper).to receive(:mr_target_project_id).and_return('1')
+ allow($stdout).to receive(:puts)
end
describe '#output_message' do
- shared_examples 'empty message' do |output, num_of_jobs_in_target_branch, num_of_jobs_in_source_branch|
- it 'returns empty string and prints the correct messages to stdout' do
- default_output = <<~OUTPUT
- Initializing #{num_of_jobs_in_target_branch} jobs from master ci config...
- Initializing #{num_of_jobs_in_source_branch} jobs from feature_branch ci config...
- Looking for misconfigured dependent jobs for setup-test-env...
- Detected 0 dependent jobs with misconfigured rules.
- Looking for misconfigured dependent jobs for compile-test-assets...
- Detected 0 dependent jobs with misconfigured rules.
- Looking for misconfigured dependent jobs for retrieve-tests-metadata...
- Detected 0 dependent jobs with misconfigured rules.
- Looking for misconfigured dependent jobs for build-gdk-image...
- Detected 0 dependent jobs with misconfigured rules.
- OUTPUT
-
- expect { expect(ci_jobs_dependency_validation.output_message).to eq('') }.tap do |expectation|
- expected_output = output == :default_stdout_output ? default_output : output
- expectation.to output(expected_output).to_stdout unless expected_output.nil?
+ shared_examples 'output message' do |warning|
+ it 'outputs messages' do
+ if warning
+ expect(ci_jobs_dependency_validation).to receive(:warn).with(described_class::FAILED_VALIDATION_WARNING)
+ else
+ expect(ci_jobs_dependency_validation).not_to receive(:warn)
end
+
+ expect(ci_jobs_dependency_validation.output_message).to eq(expected_message)
end
end
context 'when not in ci environment' do
let(:ci) { false }
+ let(:expected_message) { '' }
- it_behaves_like 'empty message', nil
+ it_behaves_like 'output message'
end
context 'when in ci environment' do
context 'with no ci changes' do
+ let(:expected_message) { '' }
+
before do
allow(ci_jobs_dependency_validation.helper).to receive(:has_ci_changes?).and_return(false)
end
- it_behaves_like 'empty message'
+ it_behaves_like 'output message'
end
- context 'when target branch jobs is empty' do
- let(:source_branch_merged_yaml) { YAML.dump({}) }
+ context 'with api fails to retrieve jobs from target branch' do
+ let(:error_msg) { '404 not found' }
- it_behaves_like 'empty message'
- end
-
- context 'when retrieving target branch jobs fails' do
before do
- allow(ci_jobs_dependency_validation).to receive_message_chain(:gitlab, :api, :get)
- .with("/projects/1/ci/lint", query: query).and_raise('404 Not Found')
+ allow(
+ ci_jobs_dependency_validation
+ ).to receive_message_chain(:gitlab, :api, :get).with("/projects/1/ci/lint", query: {}).and_raise(error_msg)
end
- it 'prints the failure but does not break' do
+ it 'warns validation is skipped and outputs empty message' do
+ expect(ci_jobs_dependency_validation).to receive(:warn).with(
+ "#{described_class::SKIPPED_VALIDATION_WARNING}: #{error_msg}"
+ )
expect { expect(ci_jobs_dependency_validation.output_message).to eq('') }.tap do |expectation|
expectation
.to output(<<~MSG).to_stdout
- Initializing 1 jobs from master ci config...
- 404 Not Found
- Initializing 0 jobs from feature_branch ci config...
+ Initializing 0 jobs from master ci config...
MSG
end
end
end
- context 'when source branch jobs is empty' do
- let(:master_merged_yaml) { YAML.dump({}) }
-
- it_behaves_like 'empty message'
- end
+ context 'with api fails to retrieve jobs from source branch' do
+ let(:error_msg) { '404 not found' }
- context 'when retrieving source branch jobs fails' do
before do
- allow(ci_jobs_dependency_validation).to receive_message_chain(:gitlab, :api, :get)
- .with("/projects/1/ci/lint", query: {}).and_raise('404 Not Found')
+ allow(
+ ci_jobs_dependency_validation
+ ).to receive_message_chain(:gitlab, :api, :get).with("/projects/1/ci/lint", query: query).and_raise(error_msg)
end
- it 'prints the failure but does not break' do
+ it 'warns validation is skipped and outputs empty message' do
+ expect(ci_jobs_dependency_validation).to receive(:warn).with(
+ "#{described_class::SKIPPED_VALIDATION_WARNING}: #{error_msg}"
+ )
+
expect { expect(ci_jobs_dependency_validation.output_message).to eq('') }.tap do |expectation|
expectation
.to output(<<~MSG).to_stdout
- 404 Not Found
- Initializing 0 jobs from master ci config...
+ Initializing 1 jobs from master ci config...
+ Initializing 0 jobs from feature_branch ci config...
MSG
end
end
end
+ context 'with api returns nil for merged yaml' do
+ let(:source_branch_merged_yaml) { nil }
+
+ it 'warns validation is skipped and outputs empty message' do
+ expect(ci_jobs_dependency_validation).to receive(:warn).with(
+ "#{described_class::SKIPPED_VALIDATION_WARNING}: no implicit conversion of nil into String"
+ )
+
+ expect(ci_jobs_dependency_validation.output_message).to eq('')
+ end
+ end
+
+ context 'when target branch jobs is empty' do
+ let(:source_branch_merged_yaml) { YAML.dump({}) }
+ let(:expected_message) { '' }
+
+ it_behaves_like 'output message'
+ end
+
+ context 'when source branch jobs is empty' do
+ let(:master_merged_yaml) { YAML.dump({}) }
+ let(:expected_message) { '' }
+
+ it_behaves_like 'output message'
+ end
+
context 'when jobs do not have dependencies' do
- it_behaves_like 'empty message', :default_stdout_output, 1, 4
+ let(:expected_message) { described_class::VALIDATION_PASSED_OUTPUT }
+
+ it_behaves_like 'output message'
end
context 'when needed jobs are missing is source branch' do
@@ -162,23 +183,51 @@
YAML.dump({ 'job1' => job_config_base.merge({ 'rules' => rules_with_new_condition }) })
end
- it 'returns warning for the missing jobs' do
- expect(ci_jobs_dependency_validation.output_message).to eq(
- <<~MARKDOWN.chomp
- Unable to find job setup-test-env in feature_branch. Skipping.
- Unable to find job compile-test-assets in feature_branch. Skipping.
- Unable to find job retrieve-tests-metadata in feature_branch. Skipping.
- Unable to find job build-gdk-image in feature_branch. Skipping.
- MARKDOWN
- )
+ let(:expected_message) do
+ <<~MARKDOWN
+ ### CI Jobs Dependency Validation
+
+ | name | validation status |
+ | ------ | --------------- |
+ | `setup-test-env` | :warning: Skipped |
+ | `compile-test-assets` | :warning: Skipped |
+ | `retrieve-tests-metadata` | :warning: Skipped |
+ | `build-gdk-image` | :warning: Skipped |
+ | `build-assets-image` | :warning: Skipped |
+ | `build-qa-image` | :warning: Skipped |
+
+ - :warning: Unable to find job `setup-test-env` in branch `feature_branch`.
+ If this job has been removed, please delete it from `Tooling::Danger::CiJobsDependencyValidation::VALIDATED_JOB_NAMES`.
+ Validation skipped.
+
+ - :warning: Unable to find job `compile-test-assets` in branch `feature_branch`.
+ If this job has been removed, please delete it from `Tooling::Danger::CiJobsDependencyValidation::VALIDATED_JOB_NAMES`.
+ Validation skipped.
+
+ - :warning: Unable to find job `retrieve-tests-metadata` in branch `feature_branch`.
+ If this job has been removed, please delete it from `Tooling::Danger::CiJobsDependencyValidation::VALIDATED_JOB_NAMES`.
+ Validation skipped.
+
+ - :warning: Unable to find job `build-gdk-image` in branch `feature_branch`.
+ If this job has been removed, please delete it from `Tooling::Danger::CiJobsDependencyValidation::VALIDATED_JOB_NAMES`.
+ Validation skipped.
+
+ - :warning: Unable to find job `build-assets-image` in branch `feature_branch`.
+ If this job has been removed, please delete it from `Tooling::Danger::CiJobsDependencyValidation::VALIDATED_JOB_NAMES`.
+ Validation skipped.
+
+ - :warning: Unable to find job `build-qa-image` in branch `feature_branch`.
+ If this job has been removed, please delete it from `Tooling::Danger::CiJobsDependencyValidation::VALIDATED_JOB_NAMES`.
+ Validation skipped.
+ MARKDOWN
end
+
+ it_behaves_like 'output message', true
end
context 'when job1 in branch needs one other job to run' do
let(:job_name) { 'job1' }
let(:needed_job_name) { 'setup-test-env' }
- let(:needed_job_config) { job_config_base }
- let(:needed_job) { { needed_job_name => needed_job_config } }
let(:source_branch_merged_yaml) do
YAML.dump(source_branch_jobs_base.merge(
@@ -193,62 +242,90 @@
context 'with a hidden job' do
let(:job_name) { '.job1' }
+ let(:expected_message) { described_class::VALIDATION_PASSED_OUTPUT }
- it_behaves_like 'empty message', :default_stdout_output, 1, 5
+ it_behaves_like 'output message'
end
- context 'with a ignored job' do
+ context 'with a global keyword' do
let(:job_name) { 'default' }
+ let(:expected_message) { described_class::VALIDATION_PASSED_OUTPUT }
- it_behaves_like 'empty message', :default_stdout_output, 1, 5
+ it_behaves_like 'output message'
end
context 'when the dependent job config has not changed (identical in master and in branch)' do
let(:master_merged_yaml) { source_branch_merged_yaml }
+ let(:expected_message) { described_class::VALIDATION_PASSED_OUTPUT }
- it_behaves_like 'empty message', :default_stdout_output, 5, 5
+ it_behaves_like 'output message'
end
context 'when VALIDATED_JOB_NAMES does not contain the needed job' do
let(:needed_job_name) { 'not-recognized' }
+ let(:expected_message) { described_class::VALIDATION_PASSED_OUTPUT }
- it_behaves_like 'empty message', :default_stdout_output, 1, 5
+ it_behaves_like 'output message'
end
context 'when VALIDATED_JOB_NAMES contains the needed job and dependent job config changed' do
context 'when the added rule is also present in its needed job' do
- let(:needed_job_config) { job_config_base.merge({ 'rules' => rules_with_new_condition }) }
+ let(:source_branch_merged_yaml) do
+ YAML.dump(source_branch_jobs_base.merge({
+ job_name => job_config_base.merge({
+ 'rules' => rules_with_new_condition,
+ 'needs' => [needed_job_name]
+ }),
+ needed_job_name => { 'rules' => rules_with_new_condition, 'needs' => [] }
+ }))
+ end
+
+ let(:expected_message) { described_class::VALIDATION_PASSED_OUTPUT }
- it_behaves_like 'empty message'
+ it_behaves_like 'output message'
end
context 'when an added rule is missing in its needed job' do
- it 'returns warning' do
- expect(ci_jobs_dependency_validation.output_message).to eq(
- <<~MARKDOWN
- **This MR adds new rules to the following dependent jobs for `setup-test-env`:**
+ let(:expected_message) do
+ <<~MARKDOWN
+ ### CI Jobs Dependency Validation
+
+ | name | validation status |
+ | ------ | --------------- |
+ | `setup-test-env` | 🚨 Failed (1) |
+ | `compile-test-assets` | :white_check_mark: Passed |
+ | `retrieve-tests-metadata` | :white_check_mark: Passed |
+ | `build-gdk-image` | :white_check_mark: Passed |
+ | `build-assets-image` | :white_check_mark: Passed |
+ | `build-qa-image` | :white_check_mark: Passed |
- `job1`:
+ - 🚨 **New rules were detected in the following jobs but missing in `setup-test-env`:**
- ```yaml
- - if: $NEW_VAR == "true"
- ```
+ <details><summary>Click to expand details</summary>
- Please ensure the changes are included in the rules for `setup-test-env` to avoid yaml syntax error!
+ `job1`:
- <details><summary>Click to expand rules for setup-test-env to confirm if the new conditions are present</summary>
+ ```yaml
+ - if: $NEW_VAR == "true"
+ ```
- ```yaml
- - if: $CI_MERGE_REQUEST_EVENT_TYPE == "merged_result" || $CI_MERGE_REQUEST_EVENT_TYPE
- == "detached"
- changes:
- - doc/index.md
- ```
+ Here are the rules for `setup-test-env`:
- </details>
- MARKDOWN
- )
+ ```yaml
+ - if: $CI_MERGE_REQUEST_EVENT_TYPE == "merged_result" || $CI_MERGE_REQUEST_EVENT_TYPE
+ == "detached"
+ changes:
+ - doc/index.md
+ ```
+
+ </details>
+
+ To avoid CI config errors, please verify if the new rules should be added to `setup-test-env`.
+ Please add a comment if rules should not be added.
+ MARKDOWN
end
+
+ it_behaves_like 'output message', true
end
end
end
@@ -264,7 +341,9 @@
))
end
- it_behaves_like 'empty message', :default_stdout_output, 1, 7
+ let(:expected_message) { described_class::VALIDATION_PASSED_OUTPUT }
+
+ it_behaves_like 'output message'
end
context 'when dependent job has a rule that is not a hash' do
@@ -279,7 +358,9 @@
)
end
- it_behaves_like 'empty message', :default_stdout_output, 1, 5
+ let(:expected_message) { described_class::VALIDATION_PASSED_OUTPUT }
+
+ it_behaves_like 'output message'
end
context 'when dependent job have an added rule but condition reads "when: never"' do
@@ -295,22 +376,12 @@
)
end
- it_behaves_like 'empty message', <<~OUTPUT
- Initializing 1 jobs from master ci config...
- Initializing 5 jobs from feature_branch ci config...
- Looking for misconfigured dependent jobs for setup-test-env...
- Detected 0 jobs with applicable rule changes.
- Detected 0 dependent jobs with misconfigured rules.
- Looking for misconfigured dependent jobs for compile-test-assets...
- Detected 0 dependent jobs with misconfigured rules.
- Looking for misconfigured dependent jobs for retrieve-tests-metadata...
- Detected 0 dependent jobs with misconfigured rules.
- Looking for misconfigured dependent jobs for build-gdk-image...
- Detected 0 dependent jobs with misconfigured rules.
- OUTPUT
+ let(:expected_message) { described_class::VALIDATION_PASSED_OUTPUT }
+
+ it_behaves_like 'output message'
end
- context 'when dependent job have modified rules, but its attributes has nested arrays' do
+ context 'when dependent job have modified rules, but its attributes have nested arrays' do
let(:source_branch_merged_yaml) do
YAML.dump(
source_branch_jobs_base.merge({
@@ -322,37 +393,56 @@
)
end
- it 'correctly parses input yaml and returns warning' do
- expected_markdown = %w[setup-test-env compile-test-assets retrieve-tests-metadata].map do |job_name|
+ let(:message_preview) do
+ <<~MARKDOWN
+ ### CI Jobs Dependency Validation
+
+ | name | validation status |
+ | ------ | --------------- |
+ | `setup-test-env` | 🚨 Failed (1) |
+ | `compile-test-assets` | 🚨 Failed (1) |
+ | `retrieve-tests-metadata` | 🚨 Failed (1) |
+ | `build-gdk-image` | :white_check_mark: Passed |
+ | `build-assets-image` | :white_check_mark: Passed |
+ | `build-qa-image` | :white_check_mark: Passed |
+
+ MARKDOWN
+ end
+
+ let(:expected_message) do
+ %w[setup-test-env compile-test-assets retrieve-tests-metadata].map do |job_name|
<<~MARKDOWN
- **This MR adds new rules to the following dependent jobs for `#{job_name}`:**
+ - 🚨 **New rules were detected in the following jobs but missing in `#{job_name}`:**
- `job1`:
+ <details><summary>Click to expand details</summary>
- ```yaml
- - if: 'true'
- when: always
- - if: $NEW_VAR == "true"
- ```
+ `job1`:
- Please ensure the changes are included in the rules for `#{job_name}` to avoid yaml syntax error!
+ ```yaml
+ - if: 'true'
+ when: always
+ - if: $NEW_VAR == "true"
+ ```
- <details><summary>Click to expand rules for #{job_name} to confirm if the new conditions are present</summary>
+ Here are the rules for `#{job_name}`:
- ```yaml
- - if: $CI_MERGE_REQUEST_EVENT_TYPE == "merged_result" || $CI_MERGE_REQUEST_EVENT_TYPE
- == "detached"
- changes:
- - doc/index.md
- ```
+ ```yaml
+ - if: $CI_MERGE_REQUEST_EVENT_TYPE == "merged_result" || $CI_MERGE_REQUEST_EVENT_TYPE
+ == "detached"
+ changes:
+ - doc/index.md
+ ```
- </details>
+ </details>
- MARKDOWN
- end.join('').chomp
+ To avoid CI config errors, please verify if the new rules should be added to `#{job_name}`.
+ Please add a comment if rules should not be added.
- expect(ci_jobs_dependency_validation.output_message).to eq(expected_markdown)
+ MARKDOWN
+ end.join('').prepend(message_preview).chomp
end
+
+ it_behaves_like 'output message', true
end
end
end
diff --git a/tooling/danger/ci_jobs_dependency_validation.rb b/tooling/danger/ci_jobs_dependency_validation.rb
index f84922a4333770be895c3f73ee84d13f60af158c..88a118d4a77cd0630d43d854c7bc0de201752f6d 100644
--- a/tooling/danger/ci_jobs_dependency_validation.rb
+++ b/tooling/danger/ci_jobs_dependency_validation.rb
@@ -3,9 +3,19 @@
module Tooling
module Danger
module CiJobsDependencyValidation
- VALIDATED_JOB_NAMES = %w[setup-test-env compile-test-assets retrieve-tests-metadata build-gdk-image].freeze
+ VALIDATED_JOB_NAMES = %w[
+ setup-test-env
+ compile-test-assets
+ retrieve-tests-metadata
+ build-gdk-image
+ build-assets-image
+ build-qa-image
+ ].freeze
GLOBAL_KEYWORDS = %w[workflow variables stages default].freeze
DEFAULT_BRANCH_NAME = 'master'
+ FAILED_VALIDATION_WARNING = 'Please review warnings in the *CI Jobs Dependency Validation* section below.'
+ SKIPPED_VALIDATION_WARNING = 'Job dependency validation is skipped due to error fetching merged CI yaml'
+ VALIDATION_PASSED_OUTPUT = ':white_check_mark: No warnings found in ci job dependencies.'
Job = Struct.new(:name, :rules, :needs, keyword_init: true) do
def self.parse_rules_from_yaml(name, jobs_yaml)
@@ -28,7 +38,6 @@ def self.attribute_values(jobs_yaml, name, attribute)
end
def self.ignore?(job_name)
- # hidden jobs are extended by other jobs thus their rules will be verified in the extending jobs
GLOBAL_KEYWORDS.include?(job_name) || job_name.start_with?('.')
end
@@ -42,9 +51,27 @@ def dependent_jobs(jobs)
def output_message
return '' if !helper.ci? || !helper.has_ci_changes? || target_branch_jobs.empty? || source_branch_jobs.empty?
- VALIDATED_JOB_NAMES.filter_map do |needed_job_name|
- construct_warning_message(needed_job_name)
- end.join("\n")
+ validation_statuses = VALIDATED_JOB_NAMES.to_h do |job_name|
+ [job_name, { skipped: false, failed: 0 }]
+ end
+
+ output = VALIDATED_JOB_NAMES.filter_map do |needed_job_name|
+ validate(needed_job_name, validation_statuses)
+ end.join("\n").chomp
+
+ return VALIDATION_PASSED_OUTPUT if output == ''
+
+ warn FAILED_VALIDATION_WARNING
+
+ <<~MARKDOWN
+ ### CI Jobs Dependency Validation
+
+ | name | validation status |
+ | ------ | --------------- |
+ #{construct_summary_table(validation_statuses)}
+
+ #{output}
+ MARKDOWN
end
private
@@ -70,7 +97,7 @@ def fetch_jobs_yaml(project_id, ref)
YAML.load(api_response['merged_yaml'], aliases: true)
rescue StandardError => e
- puts e.message
+ warn "#{SKIPPED_VALIDATION_WARNING}: #{e.message}"
{}
end
@@ -99,43 +126,70 @@ def query_params(ref)
ref == DEFAULT_BRANCH_NAME ? {} : ref_query_params
end
- def construct_warning_message(needed_job_name)
+ def validate(needed_job_name, validation_statuses)
needed_job_in_source_branch = source_branch_jobs.find { |job| job.name == needed_job_name }
needed_job_in_target_branch = target_branch_jobs.find { |job| job.name == needed_job_name }
if needed_job_in_source_branch.nil?
- return "Unable to find job #{needed_job_name} in #{source_branch}. Skipping."
- end
+ validation_statuses[needed_job_name][:skipped] = true
- puts "Looking for misconfigured dependent jobs for #{needed_job_name}..."
+ return <<~MARKDOWN
+ - :warning: Unable to find job `#{needed_job_name}` in branch `#{source_branch}`.
+ If this job has been removed, please delete it from `Tooling::Danger::CiJobsDependencyValidation::VALIDATED_JOB_NAMES`.
+ Validation skipped.
+ MARKDOWN
+ end
- warnings = changed_jobs_warnings(
+ failures = validation_failures(
needed_job_in_source_branch: needed_job_in_source_branch,
needed_job_in_target_branch: needed_job_in_target_branch
)
- puts "Detected #{warnings.count} dependent jobs with misconfigured rules."
+ failed_count = failures.count
- return if warnings.empty?
+ return if failed_count == 0
+
+ validation_statuses[needed_job_name][:failed] = failed_count
<<~MSG
- **This MR adds new rules to the following dependent jobs for `#{needed_job_name}`:**
+ - 🚨 **New rules were detected in the following jobs but missing in `#{needed_job_name}`:**
- #{warnings.join("\n")}
+ <details><summary>Click to expand details</summary>
- Please ensure the changes are included in the rules for `#{needed_job_name}` to avoid yaml syntax error!
+ #{failures.join("\n")}
- <details><summary>Click to expand rules for #{needed_job_name} to confirm if the new conditions are present</summary>
+ Here are the rules for `#{needed_job_name}`:
```yaml
#{dump_yaml(needed_job_in_source_branch.rules)}
```
</details>
+
+ To avoid CI config errors, please verify if the new rules should be added to `#{needed_job_name}`.
+ Please add a comment if rules should not be added.
MSG
end
- def changed_jobs_warnings(needed_job_in_source_branch:, needed_job_in_target_branch:)
+ def construct_summary_table(validation_statuses)
+ validation_statuses.map do |job_name, statuses|
+ skipped, failed_count = statuses.values_at(:skipped, :failed)
+
+ summary = if skipped
+ ":warning: Skipped"
+ elsif failed_count == 0
+ ":white_check_mark: Passed"
+ else
+ "🚨 Failed (#{failed_count})"
+ end
+
+ <<~MARKDOWN.chomp
+ | `#{job_name}` | #{summary} |
+ MARKDOWN
+ end.join("\n")
+ end
+
+ def validation_failures(needed_job_in_source_branch:, needed_job_in_target_branch:)
dependent_jobs_new = needed_job_in_source_branch&.dependent_jobs(source_branch_jobs) || []
dependent_jobs_old = needed_job_in_target_branch&.dependent_jobs(target_branch_jobs) || []
@@ -150,8 +204,6 @@ def changed_jobs_warnings(needed_job_in_source_branch:, needed_job_in_target_bra
dependent_job_with_rule_change.rules - dependent_job_old.rules
end
- puts "Detected #{report_candidates.count} jobs with applicable rule changes."
-
rules_to_report = exact_rules_missing_in_needed_job(
needed_job: needed_job_in_source_branch,
rules: report_candidates