Skip to content
代码片段 群组 项目
未验证 提交 4343376d 编辑于 作者: Jennifer Li's avatar Jennifer Li 提交者: GitLab
浏览文件

Improve output message and fix default return

Improve output message and specs

More refactor
上级 b51ab99d
No related branches found
No related tags found
无相关合并请求
......@@ -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
......
......@@ -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
......
0% 加载中 .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册