Simplify coverage setting and comply to some requests in code review

6 files changed, 28 insertions, 62 deletions

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index a8c0721bbcc..0a264c0e228 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -286,24 +286,11 @@ 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.
 
-#### coverage:output_filter
-
-For now, there is only the `output_filter` directive expected to be inside the
-`coverage` entry. And it is expected to be a regular expression.
-
-So, in the end, you're going to have something like the following:
+Regular expressions are used by default. So using surrounding `/` is optional, given it'll always be read as a regular expression. Don't forget to escape special characters whenever you want to match them in the regular expression.
 
+A simple example:
 ```yaml
-coverage:
-  output_filter: /\(\d+\.\d+\) covered\./
-```
-
-It's worth to keep in mind that the surrounding `/` is optional. So, the above
-example is the same as the following:
-
-```yaml
-coverage:
-  output_filter: \(\d+\.\d+\) covered\.
+coverage: \(\d+\.\d+\) covered\.
 ```
 
 ## Jobs
@@ -347,7 +334,6 @@ 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 |
-| environment   | no | Defines a name of environment to which deployment is done by this build |
 | coverage      | no | Define coverage settings for a given job |
 
 ### script
@@ -1032,17 +1018,15 @@ been defined in the global level. A quick example of one overwritting the
 other would be:
 
 ```yaml
-coverage:
-  output_filter: /\(\d+\.\d+\) covered\./
+coverage: \(\d+\.\d+\) covered\.
 
 job1:
-  coverage:
-    output_filter: /Code coverage: \d+\.\d+/
+  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\./`.
+regex that would be used is `Code coverage: \d+\.\d+` instead of
+`\(\d+\.\d+\) covered\.`.
 
 ## Git Strategy
 
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 02944e0385a..649ee4d018b 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -61,7 +61,7 @@ module Ci
         allow_failure: job[:allow_failure] || false,
         when: job[:when] || 'on_success',
         environment: job[:environment_name],
-        coverage_regex: job[:coverage][:output_filter],
+        coverage_regex: job[:coverage],
         yaml_variables: yaml_variables(name),
         options: {
           image: job[:image],
diff --git a/lib/gitlab/ci/config/entry/coverage.rb b/lib/gitlab/ci/config/entry/coverage.rb
index 41e1d6e0c86..aa738fcfd11 100644
--- a/lib/gitlab/ci/config/entry/coverage.rb
+++ b/lib/gitlab/ci/config/entry/coverage.rb
@@ -8,27 +8,17 @@ module Gitlab
         class Coverage < Node
           include Validatable
 
-          ALLOWED_KEYS = %i[output_filter]
-
           validations do
-            validates :config, type: Hash
-            validates :config, allowed_keys: ALLOWED_KEYS
-            validates :output_filter, regexp: true
+            validates :config, regexp: true
           end
 
-          def output_filter
-            output_filter_value = @config[:output_filter].to_s
-
-            if output_filter_value.start_with?('/') && output_filter_value.end_with?('/')
-              output_filter_value[1...-1]
+          def value
+            if @config.start_with?('/') && @config.end_with?('/')
+              @config[1...-1]
             else
-              @config[:output_filter]
+              @config
             end
           end
-
-          def value
-            @config.merge(output_filter: output_filter)
-          end
         end
       end
     end
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index eb2d9c6e0e3..ac706216d5a 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -9,26 +9,17 @@ module Ci
         subject { described_class.new(config, path).build_attributes(:rspec) }
 
         let(:config_base) { { rspec: { script: "rspec" } } }
-        let(:config)      { YAML.dump(config_base) }
+        let(:config) { YAML.dump(config_base) }
 
         context 'when config has coverage set at the global scope' do
-          before do
-            config_base.update(
-              coverage: { output_filter: '\(\d+\.\d+\) covered' }
-            )
-          end
+          before { config_base.update(coverage: '\(\d+\.\d+\) covered') }
 
-          context 'and \'rspec\' job doesn\'t have coverage set' do
+          context "and 'rspec' job doesn't have coverage set" do
             it { is_expected.to include(coverage_regex: '\(\d+\.\d+\) covered') }
           end
 
           context 'but \'rspec\' job also has coverage set' do
-            before do
-              config_base[:rspec].update(
-                coverage: { output_filter: '/Code coverage: \d+\.\d+/' }
-              )
-            end
-
+            before { config_base[:rspec][:coverage] = '/Code coverage: \d+\.\d+/' }
             it { is_expected.to include(coverage_regex: 'Code coverage: \d+\.\d+') }
           end
         end
diff --git a/spec/lib/gitlab/ci/config/entry/coverage_spec.rb b/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
index 9e59755d9f8..0549dbc732b 100644
--- a/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
@@ -5,35 +5,35 @@ describe Gitlab::Ci::Config::Entry::Coverage do
 
   describe 'validations' do
     context 'when entry config value is correct' do
-      let(:config) { { output_filter: 'Code coverage: \d+\.\d+' } }
+      let(:config) { 'Code coverage: \d+\.\d+' }
 
       describe '#value' do
         subject { entry.value }
-        it      { is_expected.to eq config }
+        it { is_expected.to eq config }
       end
 
       describe '#errors' do
         subject { entry.errors }
-        it      { is_expected.to be_empty }
+        it { is_expected.to be_empty }
       end
 
       describe '#valid?' do
         subject { entry }
-        it      { is_expected.to be_valid }
+        it { is_expected.to be_valid }
       end
     end
 
     context 'when entry value is not correct' do
-      let(:config) { { output_filter: '(malformed regexp' } }
+      let(:config) { '(malformed regexp' }
 
       describe '#errors' do
         subject { entry.errors }
-        it      { is_expected.to include /coverage output filter must be a regular expression/ }
+        it { is_expected.to include /coverage config must be a regular expression/ }
       end
 
       describe '#valid?' do
         subject { entry }
-        it      { is_expected.not_to be_valid }
+        it { is_expected.not_to be_valid }
       end
     end
   end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 9e5481017e2..7c054dd95f5 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -222,9 +222,10 @@ describe Ci::Build, :models do
   end
 
   describe '#coverage_regex' do
-    subject             { build.coverage_regex }
+    subject { build.coverage_regex }
+
     let(:project_regex) { '\(\d+\.\d+\) covered' }
-    let(:build_regex)   { 'Code coverage: \d+\.\d+' }
+    let(:build_regex) { 'Code coverage: \d+\.\d+' }
 
     context 'when project has build_coverage_regex set' do
       before { project.build_coverage_regex = project_regex }
@@ -235,7 +236,7 @@ describe Ci::Build, :models do
 
       context 'but coverage_regex attribute is also set' do
         before { build.coverage_regex = build_regex }
-        it     { is_expected.to eq(build_regex) }
+        it { is_expected.to eq(build_regex) }
       end
     end