diff --git a/changelogs/unreleased/fix-gb-backwards-compatibility-coverage-ci-yml.yml b/changelogs/unreleased/fix-gb-backwards-compatibility-coverage-ci-yml.yml new file mode 100644 index 0000000000000000000000000000000000000000..df7e3776700bca27de954c98fade3542e5b92cc1 --- /dev/null +++ b/changelogs/unreleased/fix-gb-backwards-compatibility-coverage-ci-yml.yml @@ -0,0 +1,4 @@ +--- +title: Preserve backward compatibility CI/CD and disallow setting `coverage` regexp in global context +merge_request: 8981 +author: diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 7b9cce32867006c914b47766aa315fa43915f9b1..cd492d167475690b1389be4c0b0319bff0f4e371 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -76,7 +76,6 @@ There are a few reserved `keywords` that **cannot** be used as job names: | after_script | no | Define commands that run after each job's script | | variables | no | Define build variables | | cache | no | Define list of files that should be cached between subsequent runs | -| coverage | no | Define coverage settings for all jobs | ### image and services @@ -279,23 +278,6 @@ cache: untracked: true ``` -### coverage - -`coverage` allows you to configure how coverage will be filtered out from the -build outputs. Setting this up globally will make all the jobs to use this -setting for output filtering and extracting the coverage information from your -builds. - -Regular expressions are the only valid kind of value expected here. So, using -surrounding `/` is mandatory in order to consistently and explicitly represent -a regular expression string. You must escape special characters if you want to -match them literally. - -A simple example: -```yaml -coverage: /\(\d+\.\d+\) covered\./ -``` - ## Jobs `.gitlab-ci.yml` allows you to specify an unlimited number of jobs. Each job @@ -337,7 +319,7 @@ job_name: | before_script | no | Override a set of commands that are executed before build | | after_script | no | Override a set of commands that are executed after build | | environment | no | Defines a name of environment to which deployment is done by this build | -| coverage | no | Define coverage settings for a given job | +| coverage | no | Define code coverage settings for a given job | ### script @@ -1012,25 +994,23 @@ job: - execute this after my script ``` -### job coverage +### coverage -This entry is pretty much the same as described in the global context in -[`coverage`](#coverage). The only difference is that, by setting it inside -the job level, whatever is set in there will take precedence over what has -been defined in the global level. A quick example of one overriding the -other would be: +`coverage` allows you to configure how code coverage will be extracted from the +job output. -```yaml -coverage: /\(\d+\.\d+\) covered\./ +Regular expressions are the only valid kind of value expected here. So, using +surrounding `/` is mandatory in order to consistently and explicitly represent +a regular expression string. You must escape special characters if you want to +match them literally. + +A simple example: +```yaml job1: coverage: /Code coverage: \d+\.\d+/ ``` -In the example above, considering the context of the job `job1`, the coverage -regex that would be used is `/Code coverage: \d+\.\d+/` instead of -`/\(\d+\.\d+\) covered\./`. - ## Git Strategy > Introduced in GitLab 8.9 as an experimental feature. May change or be removed diff --git a/lib/gitlab/ci/config/entry/global.rb b/lib/gitlab/ci/config/entry/global.rb index ede97cc05044d6096bb1cdfbffc37fe11610764a..a4ec8f0ff2ffb493502f0989e180a9184b67c41e 100644 --- a/lib/gitlab/ci/config/entry/global.rb +++ b/lib/gitlab/ci/config/entry/global.rb @@ -33,11 +33,8 @@ module Gitlab entry :cache, Entry::Cache, description: 'Configure caching between build jobs.' - entry :coverage, Entry::Coverage, - description: 'Coverage configuration for this pipeline.' - helpers :before_script, :image, :services, :after_script, - :variables, :stages, :types, :cache, :coverage, :jobs + :variables, :stages, :types, :cache, :jobs def compose!(_deps = nil) super(self) do diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 49349035b3b55d197c5ba25f485fbd4830f65193..008c15c4de3b198ea6c2810b0206172bef79ac46 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -5,27 +5,18 @@ module Ci let(:path) { 'path' } describe '#build_attributes' do - context 'Coverage entry' do + describe 'coverage entry' do subject { described_class.new(config, path).build_attributes(:rspec) } - let(:config_base) { { rspec: { script: "rspec" } } } - let(:config) { YAML.dump(config_base) } - - context 'when config has coverage set at the global scope' do - before do - config_base.update(coverage: '/\(\d+\.\d+\) covered/') - end - - context "and 'rspec' job doesn't have coverage set" do - it { is_expected.to include(coverage_regex: '\(\d+\.\d+\) covered') } + describe 'code coverage regexp' do + let(:config) do + YAML.dump(rspec: { script: 'rspec', + coverage: '/Code coverage: \d+\.\d+/' }) end - context "but 'rspec' job also has coverage set" do - before do - config_base[:rspec][:coverage] = '/Code coverage: \d+\.\d+/' - end - - it { is_expected.to include(coverage_regex: 'Code coverage: \d+\.\d+') } + it 'includes coverage regexp in build attributes' do + expect(subject) + .to include(coverage_regex: 'Code coverage: \d+\.\d+') end end end diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb index d4f1780b17446c01d0571e03e68a63d197e5559c..432a99dce33f12ed1b574b2b92583d780d579454 100644 --- a/spec/lib/gitlab/ci/config/entry/global_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb @@ -10,10 +10,10 @@ describe Gitlab::Ci::Config::Entry::Global do context 'when filtering all the entry/node names' do it 'contains the expected node names' do - node_names = described_class.nodes.keys - expect(node_names).to match_array(%i[before_script image services - after_script variables stages - types cache coverage]) + expect(described_class.nodes.keys) + .to match_array(%i[before_script image services + after_script variables stages + types cache]) end end end @@ -40,7 +40,7 @@ describe Gitlab::Ci::Config::Entry::Global do end it 'creates node object for each entry' do - expect(global.descendants.count).to eq 9 + expect(global.descendants.count).to eq 8 end it 'creates node object using valid class' do @@ -181,7 +181,7 @@ describe Gitlab::Ci::Config::Entry::Global do describe '#nodes' do it 'instantizes all nodes' do - expect(global.descendants.count).to eq 9 + expect(global.descendants.count).to eq 8 end it 'contains unspecified nodes' do