From 6a3d29c73d2578c7b2a40f2dfcb823b681a70f7e Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Mon, 14 Nov 2016 01:56:39 -0200
Subject: [PATCH 01/15] Add ability to define a coverage regex in the
 .gitlab-ci.yml

* Instead of using the proposed `coverage` key, this expects `coverage_regex`
---
 app/models/ci/build.rb                        |  7 +++--
 ...1114024742_add_coverage_regex_to_builds.rb | 13 ++++++++++
 db/schema.rb                                  |  1 +
 lib/ci/gitlab_ci_yaml_processor.rb            |  1 +
 lib/gitlab/ci/config/entry/job.rb             |  8 ++++--
 .../config/entry/legacy_validation_helpers.rb | 10 ++++---
 lib/gitlab/ci/config/entry/validators.rb      | 10 +++++++
 lib/gitlab/ci/config/node/regexp.rb           | 26 +++++++++++++++++++
 8 files changed, 69 insertions(+), 7 deletions(-)
 create mode 100644 db/migrate/20161114024742_add_coverage_regex_to_builds.rb
 create mode 100644 lib/gitlab/ci/config/node/regexp.rb

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 5fe8ddf69d7..4691b33ee9b 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -276,8 +276,7 @@ module Ci
 
     def update_coverage
       return unless project
-      coverage_regex = project.build_coverage_regex
-      return unless coverage_regex
+      return unless coverage_regex = self.coverage_regex
       coverage = extract_coverage(trace, coverage_regex)
 
       if coverage.is_a? Numeric
@@ -522,6 +521,10 @@ module Ci
       self.update(artifacts_expire_at: nil)
     end
 
+    def coverage_regex
+      read_attribute(:coverage_regex) || build_attributes_from_config[:coverage] || project.build_coverage_regex
+    end
+
     def when
       read_attribute(:when) || build_attributes_from_config[:when] || 'on_success'
     end
diff --git a/db/migrate/20161114024742_add_coverage_regex_to_builds.rb b/db/migrate/20161114024742_add_coverage_regex_to_builds.rb
new file mode 100644
index 00000000000..88aa5d52b39
--- /dev/null
+++ b/db/migrate/20161114024742_add_coverage_regex_to_builds.rb
@@ -0,0 +1,13 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddCoverageRegexToBuilds < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    add_column :ci_builds, :coverage_regex, :string
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 3c836db27fc..1cc9e7eec5e 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -215,6 +215,7 @@ ActiveRecord::Schema.define(version: 20170121130655) do
     t.datetime "queued_at"
     t.string "token"
     t.integer "lock_version"
+    t.string "coverage_regex"
   end
 
   add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 7463bd719d5..2f5ef4d36bd 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -61,6 +61,7 @@ module Ci
         allow_failure: job[:allow_failure] || false,
         when: job[:when] || 'on_success',
         environment: job[:environment_name],
+        coverage_regex: job[:coverage_regex],
         yaml_variables: yaml_variables(name),
         options: {
           image: job[:image],
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index a55362f0b6b..3c7ef99cefc 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -11,7 +11,7 @@ module Gitlab
 
           ALLOWED_KEYS = %i[tags script only except type image services allow_failure
                             type stage when artifacts cache dependencies before_script
-                            after_script variables environment]
+                            after_script variables environment coverage_regex]
 
           validations do
             validates :config, allowed_keys: ALLOWED_KEYS
@@ -71,9 +71,12 @@ module Gitlab
           entry :environment, Entry::Environment,
                description: 'Environment configuration for this job.'
 
+          node :coverage_regex, Node::Regexp,
+               description: 'Coverage scanning regex configuration for this job.'
+
           helpers :before_script, :script, :stage, :type, :after_script,
                   :cache, :image, :services, :only, :except, :variables,
-                  :artifacts, :commands, :environment
+                  :artifacts, :commands, :environment, :coverage_regex
 
           attributes :script, :tags, :allow_failure, :when, :dependencies
 
@@ -130,6 +133,7 @@ module Gitlab
               variables: variables_defined? ? variables_value : nil,
               environment: environment_defined? ? environment_value : nil,
               environment_name: environment_defined? ? environment_value[:name] : nil,
+              coverage_regex: coverage_regex_defined? ? coverage_regex_value : nil,
               artifacts: artifacts_value,
               after_script: after_script_value }
           end
diff --git a/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb b/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
index f01975aab5c..34e7052befc 100644
--- a/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
+++ b/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
@@ -28,17 +28,21 @@ module Gitlab
             value.is_a?(String) || value.is_a?(Symbol)
           end
 
+          def validate_regexp(value)
+            !!::Regexp.new(value)
+          rescue RegexpError
+            false
+          end
+
           def validate_string_or_regexp(value)
             return true if value.is_a?(Symbol)
             return false unless value.is_a?(String)
 
             if value.first == '/' && value.last == '/'
-              Regexp.new(value[1...-1])
+              validate_regexp(value[1...-1])
             else
               true
             end
-          rescue RegexpError
-            false
           end
 
           def validate_boolean(value)
diff --git a/lib/gitlab/ci/config/entry/validators.rb b/lib/gitlab/ci/config/entry/validators.rb
index 8632dd0e233..03a8205b081 100644
--- a/lib/gitlab/ci/config/entry/validators.rb
+++ b/lib/gitlab/ci/config/entry/validators.rb
@@ -54,6 +54,16 @@ module Gitlab
             end
           end
 
+          class RegexpValidator < ActiveModel::EachValidator
+            include LegacyValidationHelpers
+
+            def validate_each(record, attribute, value)
+              unless validate_regexp(value)
+                record.errors.add(attribute, 'must be a regular expression')
+              end
+            end
+          end
+
           class TypeValidator < ActiveModel::EachValidator
             def validate_each(record, attribute, value)
               type = options[:with]
diff --git a/lib/gitlab/ci/config/node/regexp.rb b/lib/gitlab/ci/config/node/regexp.rb
new file mode 100644
index 00000000000..7c5843eb8b6
--- /dev/null
+++ b/lib/gitlab/ci/config/node/regexp.rb
@@ -0,0 +1,26 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a Regular Expression.
+        #
+        class Regexp < Entry
+          include Validatable
+
+          validations do
+            validates :config, regexp: true
+          end
+
+          def value
+            if @config.first == '/' && @config.last == '/'
+              @config[1...-1]
+            else
+              @config
+            end
+          end
+        end
+      end
+    end
+  end
+end

From 646b9c54d043edf17924e82d8e80a56e18d14ce4 Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Fri, 18 Nov 2016 01:42:35 -0200
Subject: [PATCH 02/15] Comply to requests made in the review and adjust to the
 Entry/Node changes

This commit:
* Turns `coverage_regex` into `coverage` entry in yml file
* Fixes smaller requests from code reviewers for the previous commit
* This commit is temporary (will be squashed afterwards)

This commit does not (further commits will do though):
* Add global `coverage` entry handling in yml file as suggested by Grzegorz
* Add specs
* Create changelog
* Create docs
---
 app/models/ci/build.rb                                    | 6 +++---
 lib/ci/gitlab_ci_yaml_processor.rb                        | 2 +-
 .../ci/config/{node/regexp.rb => entry/coverage.rb}       | 4 ++--
 lib/gitlab/ci/config/entry/job.rb                         | 8 ++++----
 lib/gitlab/ci/config/entry/legacy_validation_helpers.rb   | 3 ++-
 5 files changed, 12 insertions(+), 11 deletions(-)
 rename lib/gitlab/ci/config/{node/regexp.rb => entry/coverage.rb} (90%)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 4691b33ee9b..46a6b4c724a 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -276,8 +276,8 @@ module Ci
 
     def update_coverage
       return unless project
-      return unless coverage_regex = self.coverage_regex
-      coverage = extract_coverage(trace, coverage_regex)
+      return unless regex = self.coverage_regex
+      coverage = extract_coverage(trace, regex)
 
       if coverage.is_a? Numeric
         update_attributes(coverage: coverage)
@@ -522,7 +522,7 @@ module Ci
     end
 
     def coverage_regex
-      read_attribute(:coverage_regex) || build_attributes_from_config[:coverage] || project.build_coverage_regex
+      read_attribute(:coverage_regex) || project.build_coverage_regex
     end
 
     def when
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 2f5ef4d36bd..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_regex],
+        coverage_regex: job[:coverage],
         yaml_variables: yaml_variables(name),
         options: {
           image: job[:image],
diff --git a/lib/gitlab/ci/config/node/regexp.rb b/lib/gitlab/ci/config/entry/coverage.rb
similarity index 90%
rename from lib/gitlab/ci/config/node/regexp.rb
rename to lib/gitlab/ci/config/entry/coverage.rb
index 7c5843eb8b6..88fc03db2d9 100644
--- a/lib/gitlab/ci/config/node/regexp.rb
+++ b/lib/gitlab/ci/config/entry/coverage.rb
@@ -1,11 +1,11 @@
 module Gitlab
   module Ci
     class Config
-      module Node
+      module Entry
         ##
         # Entry that represents a Regular Expression.
         #
-        class Regexp < Entry
+        class Coverage < Node
           include Validatable
 
           validations do
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index 3c7ef99cefc..bde6663344a 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -11,7 +11,7 @@ module Gitlab
 
           ALLOWED_KEYS = %i[tags script only except type image services allow_failure
                             type stage when artifacts cache dependencies before_script
-                            after_script variables environment coverage_regex]
+                            after_script variables environment coverage]
 
           validations do
             validates :config, allowed_keys: ALLOWED_KEYS
@@ -71,12 +71,12 @@ module Gitlab
           entry :environment, Entry::Environment,
                description: 'Environment configuration for this job.'
 
-          node :coverage_regex, Node::Regexp,
+          entry :coverage, Entry::Coverage,
                description: 'Coverage scanning regex configuration for this job.'
 
           helpers :before_script, :script, :stage, :type, :after_script,
                   :cache, :image, :services, :only, :except, :variables,
-                  :artifacts, :commands, :environment, :coverage_regex
+                  :artifacts, :commands, :environment, :coverage
 
           attributes :script, :tags, :allow_failure, :when, :dependencies
 
@@ -133,7 +133,7 @@ module Gitlab
               variables: variables_defined? ? variables_value : nil,
               environment: environment_defined? ? environment_value : nil,
               environment_name: environment_defined? ? environment_value[:name] : nil,
-              coverage_regex: coverage_regex_defined? ? coverage_regex_value : nil,
+              coverage: coverage_defined? ? coverage_value : nil,
               artifacts: artifacts_value,
               after_script: after_script_value }
           end
diff --git a/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb b/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
index 34e7052befc..98db4632dad 100644
--- a/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
+++ b/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
@@ -29,7 +29,8 @@ module Gitlab
           end
 
           def validate_regexp(value)
-            !!::Regexp.new(value)
+            Regexp.new(value)
+            true
           rescue RegexpError
             false
           end

From d0afc500e30ad0fe334d6dc16dd1766d8f6c523a Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Sat, 19 Nov 2016 22:48:02 -0200
Subject: [PATCH 03/15] Change expected `coverage` structure for CI
 configuration YAML file

Instead of using:
`coverage: /\(\d+.\d+%\) covered/`

This structure must be used now:
```
coverage:
  output_filter: /\(\d+.\d+%\) covered/`
```

The surrounding '/' is optional.
---
 lib/ci/gitlab_ci_yaml_processor.rb            |  2 +-
 lib/gitlab/ci/config/entry/coverage.rb        | 22 ++++++++++++++-----
 lib/gitlab/ci/config/entry/job.rb             |  2 +-
 .../config/entry/legacy_validation_helpers.rb |  4 ++--
 4 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 649ee4d018b..02944e0385a 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],
+        coverage_regex: job[:coverage][:output_filter],
         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 88fc03db2d9..e5da3cf23fd 100644
--- a/lib/gitlab/ci/config/entry/coverage.rb
+++ b/lib/gitlab/ci/config/entry/coverage.rb
@@ -8,16 +8,26 @@ module Gitlab
         class Coverage < Node
           include Validatable
 
+          ALLOWED_KEYS = %i[output_filter]
+
           validations do
-            validates :config, regexp: true
+            validates :config, type: Hash
+            validates :config, allowed_keys: ALLOWED_KEYS
+            validates :output_filter, 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]
+            else
+              value[:output_filter]
+            end
           end
 
           def value
-            if @config.first == '/' && @config.last == '/'
-              @config[1...-1]
-            else
-              @config
-            end
+            @config.merge(output_filter: output_filter)
           end
         end
       end
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index bde6663344a..69a5e6f433d 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -72,7 +72,7 @@ module Gitlab
                description: 'Environment configuration for this job.'
 
           entry :coverage, Entry::Coverage,
-               description: 'Coverage scanning regex configuration for this job.'
+               description: 'Coverage configuration for this job.'
 
           helpers :before_script, :script, :stage, :type, :after_script,
                   :cache, :image, :services, :only, :except, :variables,
diff --git a/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb b/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
index 98db4632dad..d8e74b15712 100644
--- a/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
+++ b/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
@@ -31,7 +31,7 @@ module Gitlab
           def validate_regexp(value)
             Regexp.new(value)
             true
-          rescue RegexpError
+          rescue RegexpError, TypeError
             false
           end
 
@@ -39,7 +39,7 @@ module Gitlab
             return true if value.is_a?(Symbol)
             return false unless value.is_a?(String)
 
-            if value.first == '/' && value.last == '/'
+            if value.start_with?('/') && value.end_with?('/')
               validate_regexp(value[1...-1])
             else
               true

From 9f97cc6515ac1254c443673c84700942690bbc15 Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Sun, 20 Nov 2016 00:05:49 -0200
Subject: [PATCH 04/15] Add `coverage` to the Global config entry as well

---
 lib/gitlab/ci/config/entry/global.rb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/gitlab/ci/config/entry/global.rb b/lib/gitlab/ci/config/entry/global.rb
index a4ec8f0ff2f..ede97cc0504 100644
--- a/lib/gitlab/ci/config/entry/global.rb
+++ b/lib/gitlab/ci/config/entry/global.rb
@@ -33,8 +33,11 @@ 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, :jobs
+                  :variables, :stages, :types, :cache, :coverage, :jobs
 
           def compose!(_deps = nil)
             super(self) do

From 94eb2f47c732dc9485aba4ebe52238e882a43473 Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Sun, 20 Nov 2016 02:18:58 -0200
Subject: [PATCH 05/15] Add changelog entry and extend the documentation
 accordingly

---
 changelogs/unreleased/issue-20428.yml |  4 +++
 doc/ci/yaml/README.md                 | 51 +++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100644 changelogs/unreleased/issue-20428.yml

diff --git a/changelogs/unreleased/issue-20428.yml b/changelogs/unreleased/issue-20428.yml
new file mode 100644
index 00000000000..60da1c14702
--- /dev/null
+++ b/changelogs/unreleased/issue-20428.yml
@@ -0,0 +1,4 @@
+---
+title: Add ability to define a coverage regex in the .gitlab-ci.yml
+merge_request: 7447
+author: Leandro Camargo
diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 75a0897eb15..a8c0721bbcc 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -76,6 +76,7 @@ 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
 
@@ -278,6 +279,33 @@ 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.
+
+#### 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:
+
+```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\.
+```
+
 ## Jobs
 
 `.gitlab-ci.yml` allows you to specify an unlimited number of jobs. Each job
@@ -319,6 +347,8 @@ 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
 
@@ -993,6 +1023,27 @@ job:
   - execute this after my script
 ```
 
+### job 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 overwritting the
+other would be:
+
+```yaml
+coverage:
+  output_filter: /\(\d+\.\d+\) covered\./
+
+job1:
+  coverage:
+    output_filter: /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

From 0713a7c3a9eb1bcfdf6adde0c3365549c19a3ee1 Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Mon, 21 Nov 2016 02:38:03 -0200
Subject: [PATCH 06/15] Add specs to cover the implemented feature and fix a
 small bug

---
 lib/gitlab/ci/config/entry/coverage.rb        |  2 +-
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  | 31 ++++++++++++++
 .../gitlab/ci/config/entry/coverage_spec.rb   | 40 +++++++++++++++++++
 .../lib/gitlab/ci/config/entry/global_spec.rb | 17 +++++---
 spec/lib/gitlab/ci/config/entry/job_spec.rb   | 14 +++++++
 spec/models/ci/build_spec.rb                  | 33 +++++++++++++++
 6 files changed, 131 insertions(+), 6 deletions(-)
 create mode 100644 spec/lib/gitlab/ci/config/entry/coverage_spec.rb

diff --git a/lib/gitlab/ci/config/entry/coverage.rb b/lib/gitlab/ci/config/entry/coverage.rb
index e5da3cf23fd..af12837130c 100644
--- a/lib/gitlab/ci/config/entry/coverage.rb
+++ b/lib/gitlab/ci/config/entry/coverage.rb
@@ -22,7 +22,7 @@ module Gitlab
             if output_filter_value.start_with?('/') && output_filter_value.end_with?('/')
               output_filter_value[1...-1]
             else
-              value[:output_filter]
+              @config[:output_filter]
             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 f824e2e1efe..eb2d9c6e0e3 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -4,6 +4,37 @@ module Ci
   describe GitlabCiYamlProcessor, lib: true do
     let(:path) { 'path' }
 
+    describe '#build_attributes' do
+      context '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: { output_filter: '\(\d+\.\d+\) covered' }
+            )
+          end
+
+          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
+
+            it { is_expected.to include(coverage_regex: 'Code coverage: \d+\.\d+') }
+          end
+        end
+      end
+    end
+
     describe "#builds_for_ref" do
       let(:type) { 'test' }
 
diff --git a/spec/lib/gitlab/ci/config/entry/coverage_spec.rb b/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
new file mode 100644
index 00000000000..9e59755d9f8
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
@@ -0,0 +1,40 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Entry::Coverage do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is correct' do
+      let(:config) { { output_filter: 'Code coverage: \d+\.\d+' } }
+
+      describe '#value' do
+        subject { entry.value }
+        it      { is_expected.to eq config }
+      end
+
+      describe '#errors' do
+        subject { entry.errors }
+        it      { is_expected.to be_empty }
+      end
+
+      describe '#valid?' do
+        subject { entry }
+        it      { is_expected.to be_valid }
+      end
+    end
+
+    context 'when entry value is not correct' do
+      let(:config) { { output_filter: '(malformed regexp' } }
+
+      describe '#errors' do
+        subject { entry.errors }
+        it      { is_expected.to include /coverage output filter must be a regular expression/ }
+      end
+
+      describe '#valid?' do
+        subject { entry }
+        it      { is_expected.not_to be_valid }
+      end
+    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 e64c8d46bd8..66a1380bc61 100644
--- a/spec/lib/gitlab/ci/config/entry/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb
@@ -4,12 +4,19 @@ describe Gitlab::Ci::Config::Entry::Global do
   let(:global) { described_class.new(hash) }
 
   describe '.nodes' do
-    it 'can contain global config keys' do
-      expect(described_class.nodes).to include :before_script
-    end
+    subject { described_class.nodes }
 
-    it 'returns a hash' do
-      expect(described_class.nodes).to be_a Hash
+    it { is_expected.to be_a Hash }
+
+    context 'when filtering all the entry/node names' do
+      subject { described_class.nodes.keys }
+
+      let(:result) do
+        %i[before_script image services after_script variables stages types
+           cache coverage]
+      end
+
+      it { is_expected.to match_array result }
     end
   end
 
diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb
index fc9b8b86dc4..d20f4ec207d 100644
--- a/spec/lib/gitlab/ci/config/entry/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb
@@ -3,6 +3,20 @@ require 'spec_helper'
 describe Gitlab::Ci::Config::Entry::Job do
   let(:entry) { described_class.new(config, name: :rspec) }
 
+  describe '.nodes' do
+    context 'when filtering all the entry/node names' do
+      subject { described_class.nodes.keys }
+
+      let(:result) do
+        %i[before_script script stage type after_script cache
+           image services only except variables artifacts
+           environment coverage]
+      end
+
+      it { is_expected.to match_array result }
+    end
+  end
+
   describe 'validations' do
     before { entry.compose! }
 
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index f031876e812..9e5481017e2 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -221,6 +221,39 @@ describe Ci::Build, :models do
     end
   end
 
+  describe '#coverage_regex' do
+    subject             { build.coverage_regex }
+    let(:project_regex) { '\(\d+\.\d+\) covered' }
+    let(:build_regex)   { 'Code coverage: \d+\.\d+' }
+
+    context 'when project has build_coverage_regex set' do
+      before { project.build_coverage_regex = project_regex }
+
+      context 'and coverage_regex attribute is not set' do
+        it { is_expected.to eq(project_regex) }
+      end
+
+      context 'but coverage_regex attribute is also set' do
+        before { build.coverage_regex = build_regex }
+        it     { is_expected.to eq(build_regex) }
+      end
+    end
+
+    context 'when neither project nor build has coverage regex set' do
+      it { is_expected.to be_nil }
+    end
+  end
+
+  describe '#update_coverage' do
+    it 'grants coverage_regex method is called inside of it' do
+      build.coverage_regex = '\(\d+.\d+\%\) covered'
+      allow(build).to receive(:trace) { 'Coverage 1033 / 1051 LOC (98.29%) covered' }
+      allow(build).to receive(:coverage_regex).and_call_original
+      allow(build).to receive(:update_attributes).with(coverage: 98.29) { true }
+      expect(build.update_coverage).to be true
+    end
+  end
+
   describe 'deployment' do
     describe '#last_deployment' do
       subject { build.last_deployment }

From bb12ee051f95ee747c0e2b98a85675de53dca8ea Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Mon, 21 Nov 2016 02:51:29 -0200
Subject: [PATCH 07/15] Fix wrong description for Coverage entry (in ruby
 comments)

---
 lib/gitlab/ci/config/entry/coverage.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/gitlab/ci/config/entry/coverage.rb b/lib/gitlab/ci/config/entry/coverage.rb
index af12837130c..41e1d6e0c86 100644
--- a/lib/gitlab/ci/config/entry/coverage.rb
+++ b/lib/gitlab/ci/config/entry/coverage.rb
@@ -3,7 +3,7 @@ module Gitlab
     class Config
       module Entry
         ##
-        # Entry that represents a Regular Expression.
+        # Entry that represents Coverage settings.
         #
         class Coverage < Node
           include Validatable

From f1e920ed86133bfea0abfc66ca44282813822073 Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Sat, 26 Nov 2016 01:02:08 -0200
Subject: [PATCH 08/15] Simplify coverage setting and comply to some requests
 in code review

---
 doc/ci/yaml/README.md                         | 30 +++++--------------
 lib/ci/gitlab_ci_yaml_processor.rb            |  2 +-
 lib/gitlab/ci/config/entry/coverage.rb        | 22 ++++----------
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  | 17 +++--------
 .../gitlab/ci/config/entry/coverage_spec.rb   | 14 ++++-----
 spec/models/ci/build_spec.rb                  |  7 +++--
 6 files changed, 29 insertions(+), 63 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,26 +8,16 @@ 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
-          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]
-            else
-              @config[:output_filter]
-            end
+            validates :config, regexp: true
           end
 
           def value
-            @config.merge(output_filter: output_filter)
+            if @config.start_with?('/') && @config.end_with?('/')
+              @config[1...-1]
+            else
+              @config
+            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
 

From 6323cd7203dbf1850e7939e81db4b1a9c6cf6d76 Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Mon, 5 Dec 2016 02:00:47 -0200
Subject: [PATCH 09/15] Comply to more requirements and requests made in the
 code review

---
 app/models/ci/build.rb                           |  2 +-
 doc/ci/yaml/README.md                            | 16 +++++++++-------
 lib/gitlab/ci/config/entry/coverage.rb           |  2 +-
 .../ci/config/entry/legacy_validation_helpers.rb |  5 ++---
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb     |  9 +++++++--
 spec/models/ci/build_spec.rb                     | 11 ++++++++---
 6 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 46a6b4c724a..951818ad561 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -522,7 +522,7 @@ module Ci
     end
 
     def coverage_regex
-      read_attribute(:coverage_regex) || project.build_coverage_regex
+      super || project.build_coverage_regex
     end
 
     def when
diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 0a264c0e228..5e2d9788f33 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -286,11 +286,13 @@ 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 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.
+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 literally.
 
 A simple example:
 ```yaml
-coverage: \(\d+\.\d+\) covered\.
+coverage: /\(\d+\.\d+\) covered\./
 ```
 
 ## Jobs
@@ -1014,19 +1016,19 @@ job:
 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 overwritting the
+been defined in the global level. A quick example of one overriding the
 other would be:
 
 ```yaml
-coverage: \(\d+\.\d+\) covered\.
+coverage: /\(\d+\.\d+\) covered\./
 
 job1:
-  coverage: 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/gitlab/ci/config/entry/coverage.rb b/lib/gitlab/ci/config/entry/coverage.rb
index aa738fcfd11..706bfc882de 100644
--- a/lib/gitlab/ci/config/entry/coverage.rb
+++ b/lib/gitlab/ci/config/entry/coverage.rb
@@ -13,7 +13,7 @@ module Gitlab
           end
 
           def value
-            if @config.start_with?('/') && @config.end_with?('/')
+            if @config.first == '/' && @config.last == '/'
               @config[1...-1]
             else
               @config
diff --git a/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb b/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
index d8e74b15712..9b9a0a8125a 100644
--- a/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
+++ b/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
@@ -29,8 +29,7 @@ module Gitlab
           end
 
           def validate_regexp(value)
-            Regexp.new(value)
-            true
+            !value.nil? && Regexp.new(value.to_s) && true
           rescue RegexpError, TypeError
             false
           end
@@ -39,7 +38,7 @@ module Gitlab
             return true if value.is_a?(Symbol)
             return false unless value.is_a?(String)
 
-            if value.start_with?('/') && value.end_with?('/')
+            if value.first == '/' && value.last == '/'
               validate_regexp(value[1...-1])
             else
               true
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index ac706216d5a..3ffcfaa1f29 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -12,14 +12,19 @@ module Ci
         let(:config) { YAML.dump(config_base) }
 
         context 'when config has coverage set at the global scope' do
-          before { config_base.update(coverage: '\(\d+\.\d+\) covered') }
+          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') }
           end
 
           context 'but \'rspec\' job also has coverage set' do
-            before { config_base[:rspec][:coverage] = '/Code coverage: \d+\.\d+/' }
+            before do
+              config_base[:rspec][:coverage] = '/Code coverage: \d+\.\d+/'
+            end
+
             it { is_expected.to include(coverage_regex: 'Code coverage: \d+\.\d+') }
           end
         end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 7c054dd95f5..7baaef9c85e 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -228,14 +228,19 @@ describe Ci::Build, :models do
     let(:build_regex) { 'Code coverage: \d+\.\d+' }
 
     context 'when project has build_coverage_regex set' do
-      before { project.build_coverage_regex = project_regex }
+      before do
+        project.build_coverage_regex = project_regex
+      end
 
       context 'and coverage_regex attribute is not set' do
         it { is_expected.to eq(project_regex) }
       end
 
       context 'but coverage_regex attribute is also set' do
-        before { build.coverage_regex = build_regex }
+        before do
+          build.coverage_regex = build_regex
+        end
+
         it { is_expected.to eq(build_regex) }
       end
     end
@@ -250,7 +255,7 @@ describe Ci::Build, :models do
       build.coverage_regex = '\(\d+.\d+\%\) covered'
       allow(build).to receive(:trace) { 'Coverage 1033 / 1051 LOC (98.29%) covered' }
       allow(build).to receive(:coverage_regex).and_call_original
-      allow(build).to receive(:update_attributes).with(coverage: 98.29) { true }
+      expect(build).to receive(:update_attributes).with(coverage: 98.29) { true }
       expect(build.update_coverage).to be true
     end
   end

From f8bec0d1fb05d2c3e87a0470579ee7a650ade23c Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Tue, 6 Dec 2016 02:39:59 -0200
Subject: [PATCH 10/15] Improve specs styles/organization and add some more
 specs

---
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  |  2 +-
 .../gitlab/ci/config/entry/coverage_spec.rb   | 25 ++++++++++++++++---
 .../lib/gitlab/ci/config/entry/global_spec.rb | 18 ++++++-------
 spec/models/ci/build_spec.rb                  | 13 +++++-----
 4 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 3ffcfaa1f29..b1e09350847 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -20,7 +20,7 @@ module Ci
             it { is_expected.to include(coverage_regex: '\(\d+\.\d+\) covered') }
           end
 
-          context 'but \'rspec\' job also has coverage set' do
+          context "but 'rspec' job also has coverage set" do
             before do
               config_base[:rspec][:coverage] = '/Code coverage: \d+\.\d+/'
             end
diff --git a/spec/lib/gitlab/ci/config/entry/coverage_spec.rb b/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
index 0549dbc732b..8f989ebd732 100644
--- a/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
@@ -4,12 +4,31 @@ describe Gitlab::Ci::Config::Entry::Coverage do
   let(:entry) { described_class.new(config) }
 
   describe 'validations' do
-    context 'when entry config value is correct' do
+    context "when entry config value is correct without surrounding '/'" do
       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 }
+      end
+
+      describe '#valid?' do
+        subject { entry }
+        it { is_expected.to be_valid }
+      end
+    end
+
+    context "when entry config value is correct with surrounding '/'" do
+      let(:config) { '/Code coverage: \d+\.\d+/' }
+
+      describe '#value' do
+        subject { entry.value }
+        it { is_expected.to eq(config[1...-1]) }
       end
 
       describe '#errors' do
@@ -28,7 +47,7 @@ describe Gitlab::Ci::Config::Entry::Coverage do
 
       describe '#errors' do
         subject { entry.errors }
-        it { is_expected.to include /coverage config must be a regular expression/ }
+        it { is_expected.to include(/coverage config must be a regular expression/) }
       end
 
       describe '#valid?' do
diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb
index 66a1380bc61..7b7f5761ebd 100644
--- a/spec/lib/gitlab/ci/config/entry/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb
@@ -4,19 +4,17 @@ describe Gitlab::Ci::Config::Entry::Global do
   let(:global) { described_class.new(hash) }
 
   describe '.nodes' do
-    subject { described_class.nodes }
-
-    it { is_expected.to be_a Hash }
+    it 'returns a hash' do
+      expect(described_class.nodes).to be_a(Hash)
+    end
 
     context 'when filtering all the entry/node names' do
-      subject { described_class.nodes.keys }
-
-      let(:result) do
-        %i[before_script image services after_script variables stages types
-           cache coverage]
+      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])
       end
-
-      it { is_expected.to match_array result }
     end
   end
 
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 7baaef9c85e..52cc45f07b2 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -251,12 +251,13 @@ describe Ci::Build, :models do
   end
 
   describe '#update_coverage' do
-    it 'grants coverage_regex method is called inside of it' do
-      build.coverage_regex = '\(\d+.\d+\%\) covered'
-      allow(build).to receive(:trace) { 'Coverage 1033 / 1051 LOC (98.29%) covered' }
-      allow(build).to receive(:coverage_regex).and_call_original
-      expect(build).to receive(:update_attributes).with(coverage: 98.29) { true }
-      expect(build.update_coverage).to be true
+    context "regarding coverage_regex's value," do
+      it "saves the correct extracted coverage value" do
+        build.coverage_regex = '\(\d+.\d+\%\) covered'
+        allow(build).to receive(:trace) { 'Coverage 1033 / 1051 LOC (98.29%) covered' }
+        expect(build).to receive(:update_attributes).with(coverage: 98.29) { true }
+        expect(build.update_coverage).to be true
+      end
     end
   end
 

From be7106a145b1e3d4c6e06503e0f7f3032ace3764 Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Wed, 7 Dec 2016 03:01:34 -0200
Subject: [PATCH 11/15] Force coverage value to always be surrounded by '/'

---
 app/models/ci/build.rb                        | 10 +++--
 doc/ci/yaml/README.md                         |  7 ++--
 lib/gitlab/ci/config/entry/coverage.rb        |  8 ----
 lib/gitlab/ci/config/entry/trigger.rb         | 10 +----
 lib/gitlab/ci/config/entry/validators.rb      | 39 +++++++++++++++++++
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  |  6 +--
 .../gitlab/ci/config/entry/coverage_spec.rb   | 37 ++++++++----------
 spec/models/ci/build_spec.rb                  |  9 +++--
 8 files changed, 75 insertions(+), 51 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 951818ad561..e3753869b67 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -275,8 +275,10 @@ module Ci
     end
 
     def update_coverage
-      return unless project
-      return unless regex = self.coverage_regex
+      regex = coverage_regex.to_s[1...-1]
+
+      return if regex.blank?
+
       coverage = extract_coverage(trace, regex)
 
       if coverage.is_a? Numeric
@@ -522,7 +524,9 @@ module Ci
     end
 
     def coverage_regex
-      super || project.build_coverage_regex
+      super ||
+        project.try(:build_coverage_regex).presence &&
+        "/#{project.try(:build_coverage_regex)}/"
     end
 
     def when
diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 5e2d9788f33..85b2c75cee8 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -286,9 +286,10 @@ 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 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 literally.
+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
diff --git a/lib/gitlab/ci/config/entry/coverage.rb b/lib/gitlab/ci/config/entry/coverage.rb
index 706bfc882de..25546f363fb 100644
--- a/lib/gitlab/ci/config/entry/coverage.rb
+++ b/lib/gitlab/ci/config/entry/coverage.rb
@@ -11,14 +11,6 @@ module Gitlab
           validations do
             validates :config, regexp: true
           end
-
-          def value
-            if @config.first == '/' && @config.last == '/'
-              @config[1...-1]
-            else
-              @config
-            end
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/entry/trigger.rb b/lib/gitlab/ci/config/entry/trigger.rb
index 28b0a9ffe01..16b234e6c59 100644
--- a/lib/gitlab/ci/config/entry/trigger.rb
+++ b/lib/gitlab/ci/config/entry/trigger.rb
@@ -9,15 +9,7 @@ module Gitlab
           include Validatable
 
           validations do
-            include LegacyValidationHelpers
-
-            validate :array_of_strings_or_regexps
-
-            def array_of_strings_or_regexps
-              unless validate_array_of_strings_or_regexps(config)
-                errors.add(:config, 'should be an array of strings or regexps')
-              end
-            end
+            validates :config, array_of_strings_or_regexps: true
           end
         end
       end
diff --git a/lib/gitlab/ci/config/entry/validators.rb b/lib/gitlab/ci/config/entry/validators.rb
index 03a8205b081..5f50b80af6c 100644
--- a/lib/gitlab/ci/config/entry/validators.rb
+++ b/lib/gitlab/ci/config/entry/validators.rb
@@ -62,6 +62,45 @@ module Gitlab
                 record.errors.add(attribute, 'must be a regular expression')
               end
             end
+
+            private
+
+            def look_like_regexp?(value)
+              value =~ %r{\A/.*/\z}
+            end
+
+            def validate_regexp(value)
+              look_like_regexp?(value) &&
+                Regexp.new(value.to_s[1...-1]) &&
+                true
+            rescue RegexpError
+              false
+            end
+          end
+
+          class ArrayOfStringsOrRegexps < RegexpValidator
+            def validate_each(record, attribute, value)
+              unless validate_array_of_strings_or_regexps(value)
+                record.errors.add(attribute, 'should be an array of strings or regexps')
+              end
+            end
+
+            private
+
+            def validate_array_of_strings_or_regexps(values)
+              values.is_a?(Array) && values.all?(&method(:validate_string_or_regexp))
+            end
+
+            def validate_string_or_regexp(value)
+              return true if value.is_a?(Symbol)
+              return false unless value.is_a?(String)
+
+              if look_like_regexp?(value)
+                validate_regexp(value)
+              else
+                true
+              end
+            end
           end
 
           class TypeValidator < ActiveModel::EachValidator
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index b1e09350847..e2302f5968a 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -13,11 +13,11 @@ module Ci
 
         context 'when config has coverage set at the global scope' do
           before do
-            config_base.update(coverage: '\(\d+\.\d+\) covered')
+            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') }
+            it { is_expected.to include(coverage_regex: '/\(\d+\.\d+\) covered/') }
           end
 
           context "but 'rspec' job also has coverage set" do
@@ -25,7 +25,7 @@ module Ci
               config_base[:rspec][:coverage] = '/Code coverage: \d+\.\d+/'
             end
 
-            it { is_expected.to include(coverage_regex: 'Code coverage: \d+\.\d+') }
+            it { is_expected.to include(coverage_regex: '/Code coverage: \d+\.\d+/') }
           end
         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 8f989ebd732..eb04075f1be 100644
--- a/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
@@ -4,9 +4,23 @@ describe Gitlab::Ci::Config::Entry::Coverage do
   let(:entry) { described_class.new(config) }
 
   describe 'validations' do
-    context "when entry config value is correct without surrounding '/'" do
+    context "when entry config value doesn't have the surrounding '/'" do
       let(:config) { 'Code coverage: \d+\.\d+' }
 
+      describe '#errors' do
+        subject { entry.errors }
+        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 }
+      end
+    end
+
+    context "when entry config value has the surrounding '/'" do
+      let(:config) { '/Code coverage: \d+\.\d+/' }
+
       describe '#value' do
         subject { entry.value }
         it { is_expected.to eq(config) }
@@ -23,26 +37,7 @@ describe Gitlab::Ci::Config::Entry::Coverage do
       end
     end
 
-    context "when entry config value is correct with surrounding '/'" do
-      let(:config) { '/Code coverage: \d+\.\d+/' }
-
-      describe '#value' do
-        subject { entry.value }
-        it { is_expected.to eq(config[1...-1]) }
-      end
-
-      describe '#errors' do
-        subject { entry.errors }
-        it { is_expected.to be_empty }
-      end
-
-      describe '#valid?' do
-        subject { entry }
-        it { is_expected.to be_valid }
-      end
-    end
-
-    context 'when entry value is not correct' do
+    context 'when entry value is not valid' do
       let(:config) { '(malformed regexp' }
 
       describe '#errors' do
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 52cc45f07b2..f23155a5d13 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -224,19 +224,20 @@ describe Ci::Build, :models do
   describe '#coverage_regex' do
     subject { build.coverage_regex }
 
-    let(:project_regex) { '\(\d+\.\d+\) covered' }
-    let(:build_regex) { 'Code coverage: \d+\.\d+' }
-
     context 'when project has build_coverage_regex set' do
+      let(:project_regex) { '\(\d+\.\d+\) covered' }
+
       before do
         project.build_coverage_regex = project_regex
       end
 
       context 'and coverage_regex attribute is not set' do
-        it { is_expected.to eq(project_regex) }
+        it { is_expected.to eq("/#{project_regex}/") }
       end
 
       context 'but coverage_regex attribute is also set' do
+        let(:build_regex) { '/Code coverage: \d+\.\d+/' }
+
         before do
           build.coverage_regex = build_regex
         end

From 518fd2eb93711e1e9c3d597a6bdf13366d9abdb5 Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Fri, 9 Dec 2016 00:20:39 -0200
Subject: [PATCH 12/15] Improve/polish code logic for some Ci::Build methods

---
 app/models/ci/build.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index e3753869b67..2a613d12913 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -275,11 +275,11 @@ module Ci
     end
 
     def update_coverage
-      regex = coverage_regex.to_s[1...-1]
+      regex = coverage_regex
 
-      return if regex.blank?
+      return unless regex
 
-      coverage = extract_coverage(trace, regex)
+      coverage = extract_coverage(trace, regex[1...-1])
 
       if coverage.is_a? Numeric
         update_attributes(coverage: coverage)
@@ -526,7 +526,7 @@ module Ci
     def coverage_regex
       super ||
         project.try(:build_coverage_regex).presence &&
-        "/#{project.try(:build_coverage_regex)}/"
+        "/#{project.build_coverage_regex}/"
     end
 
     def when

From 8fe708f4a2850d71c11234b234e039b2a9422299 Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Tue, 13 Dec 2016 02:53:12 -0200
Subject: [PATCH 13/15] Make more code improvements around the '/' stripping
 logic

---
 app/models/ci/build.rb                        | 35 +++++++------------
 lib/gitlab/ci/config/entry/coverage.rb        |  4 +++
 lib/gitlab/ci/config/entry/validators.rb      | 12 +++----
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  | 11 ++++--
 .../gitlab/ci/config/entry/coverage_spec.rb   |  2 +-
 .../lib/gitlab/ci/config/entry/global_spec.rb |  4 +--
 spec/models/ci/build_spec.rb                  |  4 +--
 7 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 2a613d12913..62fec28d2d5 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -275,30 +275,23 @@ module Ci
     end
 
     def update_coverage
-      regex = coverage_regex
-
-      return unless regex
-
-      coverage = extract_coverage(trace, regex[1...-1])
-
-      if coverage.is_a? Numeric
-        update_attributes(coverage: coverage)
-      end
+      coverage = extract_coverage(trace, coverage_regex)
+      update_attributes(coverage: coverage) if coverage.is_a?(Numeric)
     end
 
     def extract_coverage(text, regex)
-      begin
-        matches = text.scan(Regexp.new(regex)).last
-        matches = matches.last if matches.kind_of?(Array)
-        coverage = matches.gsub(/\d+(\.\d+)?/).first
+      return unless regex
 
-        if coverage.present?
-          coverage.to_f
-        end
-      rescue
-        # if bad regex or something goes wrong we dont want to interrupt transition
-        # so we just silentrly ignore error for now
+      matches = text.scan(Regexp.new(regex)).last
+      matches = matches.last if matches.kind_of?(Array)
+      coverage = matches.gsub(/\d+(\.\d+)?/).first
+
+      if coverage.present?
+        coverage.to_f
       end
+    rescue
+      # if bad regex or something goes wrong we dont want to interrupt transition
+      # so we just silentrly ignore error for now
     end
 
     def has_trace_file?
@@ -524,9 +517,7 @@ module Ci
     end
 
     def coverage_regex
-      super ||
-        project.try(:build_coverage_regex).presence &&
-        "/#{project.build_coverage_regex}/"
+      super || project.try(:build_coverage_regex)
     end
 
     def when
diff --git a/lib/gitlab/ci/config/entry/coverage.rb b/lib/gitlab/ci/config/entry/coverage.rb
index 25546f363fb..12a063059cb 100644
--- a/lib/gitlab/ci/config/entry/coverage.rb
+++ b/lib/gitlab/ci/config/entry/coverage.rb
@@ -11,6 +11,10 @@ module Gitlab
           validations do
             validates :config, regexp: true
           end
+
+          def value
+            @config[1...-1]
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/entry/validators.rb b/lib/gitlab/ci/config/entry/validators.rb
index 5f50b80af6c..30c52dd65e8 100644
--- a/lib/gitlab/ci/config/entry/validators.rb
+++ b/lib/gitlab/ci/config/entry/validators.rb
@@ -66,7 +66,7 @@ module Gitlab
             private
 
             def look_like_regexp?(value)
-              value =~ %r{\A/.*/\z}
+              value.start_with?('/') && value.end_with?('/')
             end
 
             def validate_regexp(value)
@@ -78,7 +78,7 @@ module Gitlab
             end
           end
 
-          class ArrayOfStringsOrRegexps < RegexpValidator
+          class ArrayOfStringsOrRegexpsValidator < RegexpValidator
             def validate_each(record, attribute, value)
               unless validate_array_of_strings_or_regexps(value)
                 record.errors.add(attribute, 'should be an array of strings or regexps')
@@ -94,12 +94,8 @@ module Gitlab
             def validate_string_or_regexp(value)
               return true if value.is_a?(Symbol)
               return false unless value.is_a?(String)
-
-              if look_like_regexp?(value)
-                validate_regexp(value)
-              else
-                true
-              end
+              return validate_regexp(value) if look_like_regexp?(value)
+              true
             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 e2302f5968a..49349035b3b 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -17,7 +17,7 @@ module Ci
           end
 
           context "and 'rspec' job doesn't have coverage set" do
-            it { is_expected.to include(coverage_regex: '/\(\d+\.\d+\) covered/') }
+            it { is_expected.to include(coverage_regex: '\(\d+\.\d+\) covered') }
           end
 
           context "but 'rspec' job also has coverage set" do
@@ -25,7 +25,7 @@ module Ci
               config_base[:rspec][:coverage] = '/Code coverage: \d+\.\d+/'
             end
 
-            it { is_expected.to include(coverage_regex: '/Code coverage: \d+\.\d+/') }
+            it { is_expected.to include(coverage_regex: 'Code coverage: \d+\.\d+') }
           end
         end
       end
@@ -48,6 +48,7 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
+          coverage_regex: nil,
           tag_list: [],
           options: {},
           allow_failure: false,
@@ -462,6 +463,7 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
+          coverage_regex: nil,
           tag_list: [],
           options: {
             image: "ruby:2.1",
@@ -490,6 +492,7 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
+          coverage_regex: nil,
           tag_list: [],
           options: {
             image: "ruby:2.5",
@@ -729,6 +732,7 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
+          coverage_regex: nil,
           tag_list: [],
           options: {
             image: "ruby:2.1",
@@ -940,6 +944,7 @@ module Ci
             stage_idx: 1,
             name: "normal_job",
             commands: "test",
+            coverage_regex: nil,
             tag_list: [],
             options: {},
             when: "on_success",
@@ -985,6 +990,7 @@ module Ci
             stage_idx: 0,
             name: "job1",
             commands: "execute-script-for-job",
+            coverage_regex: nil,
             tag_list: [],
             options: {},
             when: "on_success",
@@ -997,6 +1003,7 @@ module Ci
             stage_idx: 0,
             name: "job2",
             commands: "execute-script-for-job",
+            coverage_regex: nil,
             tag_list: [],
             options: {},
             when: "on_success",
diff --git a/spec/lib/gitlab/ci/config/entry/coverage_spec.rb b/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
index eb04075f1be..4c6bd859552 100644
--- a/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
@@ -23,7 +23,7 @@ describe Gitlab::Ci::Config::Entry::Coverage do
 
       describe '#value' do
         subject { entry.value }
-        it { is_expected.to eq(config) }
+        it { is_expected.to eq(config[1...-1]) }
       end
 
       describe '#errors' do
diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb
index 7b7f5761ebd..d4f1780b174 100644
--- a/spec/lib/gitlab/ci/config/entry/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb
@@ -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 8
+          expect(global.descendants.count).to eq 9
         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 8
+          expect(global.descendants.count).to eq 9
         end
 
         it 'contains unspecified nodes' do
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index f23155a5d13..fe0a9707b2a 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -232,11 +232,11 @@ describe Ci::Build, :models do
       end
 
       context 'and coverage_regex attribute is not set' do
-        it { is_expected.to eq("/#{project_regex}/") }
+        it { is_expected.to eq(project_regex) }
       end
 
       context 'but coverage_regex attribute is also set' do
-        let(:build_regex) { '/Code coverage: \d+\.\d+/' }
+        let(:build_regex) { 'Code coverage: \d+\.\d+' }
 
         before do
           build.coverage_regex = build_regex

From 441a9beec3e6834d3fe5e047e65c4d8b32ff86d5 Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Wed, 18 Jan 2017 01:42:38 -0200
Subject: [PATCH 14/15] Make some other refinements to validation logic

---
 lib/gitlab/ci/config/entry/validators.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/ci/config/entry/validators.rb b/lib/gitlab/ci/config/entry/validators.rb
index 30c52dd65e8..bd7428b1272 100644
--- a/lib/gitlab/ci/config/entry/validators.rb
+++ b/lib/gitlab/ci/config/entry/validators.rb
@@ -66,7 +66,8 @@ module Gitlab
             private
 
             def look_like_regexp?(value)
-              value.start_with?('/') && value.end_with?('/')
+              value.is_a?(String) && value.start_with?('/') &&
+                value.end_with?('/')
             end
 
             def validate_regexp(value)
@@ -92,7 +93,6 @@ module Gitlab
             end
 
             def validate_string_or_regexp(value)
-              return true if value.is_a?(Symbol)
               return false unless value.is_a?(String)
               return validate_regexp(value) if look_like_regexp?(value)
               true

From 1c24c79a83bff0d1535d813eb8146fc799d5d8ac Mon Sep 17 00:00:00 2001
From: Leandro Camargo <leandroico@gmail.com>
Date: Wed, 25 Jan 2017 01:48:03 -0200
Subject: [PATCH 15/15] Be more lenient on build coverage value updates and fix
 specs

---
 app/models/ci/build.rb                                  | 2 +-
 spec/lib/gitlab/import_export/safe_model_attributes.yml | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 62fec28d2d5..b1f77bf242c 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -276,7 +276,7 @@ module Ci
 
     def update_coverage
       coverage = extract_coverage(trace, coverage_regex)
-      update_attributes(coverage: coverage) if coverage.is_a?(Numeric)
+      update_attributes(coverage: coverage) if coverage.present?
     end
 
     def extract_coverage(text, regex)
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 493bc2db21a..95b230e4f5c 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -222,6 +222,7 @@ CommitStatus:
 - queued_at
 - token
 - lock_version
+- coverage_regex
 Ci::Variable:
 - id
 - project_id