Merge branch '20248-add-coverage-regex-in-job-yaml' into 'master'

Add ability to define a coverage regex in the .gitlab-ci.yml

Closes #20428

See merge request !7447

app/models/ci/build.rb

@@ -275,29 +275,23 @@ module Ci
     end
     def update_coverage
-      return unless project
-      coverage_regex = project.build_coverage_regex
-      return unless coverage_regex
       coverage = extract_coverage(trace, coverage_regex)
-
-      if coverage.is_a? Numeric
-        update_attributes(coverage: coverage)
-      end
+      update_attributes(coverage: coverage) if coverage.present?
     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
-        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
+      return unless regex
+      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?
@@ -522,6 +516,10 @@ module Ci
       self.update(artifacts_expire_at: nil)
     end
+    def coverage_regex
+      super || project.try(:build_coverage_regex)
+    end
+
     def when
       read_attribute(:when) || build_attributes_from_config[:when] || 'on_success'
     end

changelogs/unreleased/issue-20428.yml

+---
+title: Add ability to define a coverage regex in the .gitlab-ci.yml
+merge_request: 7447
+author: Leandro Camargo

db/migrate/20161114024742_add_coverage_regex_to_builds.rb

+# 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

db/schema.rb

@@ -215,6 +215,7 @@ ActiveRecord::Schema.define(version: 20170130204620) 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

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,23 @@ 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
@@ -319,6 +337,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 |
 ### script
@@ -993,6 +1012,25 @@ 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 overriding the
+other would be:
+
+```yaml
+coverage: /\(\d+\.\d+\) covered\./
+
+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

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],
         yaml_variables: yaml_variables(name),
         options: {
           image: job[:image],

lib/gitlab/ci/config/entry/coverage.rb

+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        ##
+        # Entry that represents Coverage settings.
+        #
+        class Coverage < Node
+          include Validatable
+
+          validations do
+            validates :config, regexp: true
+          end
+
+          def value
+            @config[1...-1]
+          end
+        end
+      end
+    end
+  end
+end

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

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]
           validations do
             validates :config, allowed_keys: ALLOWED_KEYS
@@ -71,9 +71,12 @@ module Gitlab
           entry :environment, Entry::Environment,
                description: 'Environment configuration for this job.'
+          entry :coverage, Entry::Coverage,
+               description: 'Coverage 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
           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: coverage_defined? ? coverage_value : nil,
               artifacts: artifacts_value,
               after_script: after_script_value }
           end

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)
+            !value.nil? && Regexp.new(value.to_s) && true
+          rescue RegexpError, TypeError
+            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)

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

lib/gitlab/ci/config/entry/validators.rb

@@ -54,6 +54,51 @@ 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
+
+            private
+
+            def look_like_regexp?(value)
+              value.is_a?(String) && value.start_with?('/') &&
+                value.end_with?('/')
+            end
+
+            def validate_regexp(value)
+              look_like_regexp?(value) &&
+                Regexp.new(value.to_s[1...-1]) &&
+                true
+            rescue RegexpError
+              false
+            end
+          end
+
+          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')
+              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 false unless value.is_a?(String)
+              return validate_regexp(value) if look_like_regexp?(value)
+              true
+            end
+          end
+
           class TypeValidator < ActiveModel::EachValidator
             def validate_each(record, attribute, value)
               type = options[:with]

spec/lib/ci/gitlab_ci_yaml_processor_spec.rb

@@ -4,6 +4,33 @@ 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: '/\(\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][:coverage] = '/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' }
@@ -21,6 +48,7 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
+          coverage_regex: nil,
           tag_list: [],
           options: {},
           allow_failure: false,
@@ -435,6 +463,7 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
+          coverage_regex: nil,
           tag_list: [],
           options: {
             image: "ruby:2.1",
@@ -463,6 +492,7 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
+          coverage_regex: nil,
           tag_list: [],
           options: {
             image: "ruby:2.5",
@@ -702,6 +732,7 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
+          coverage_regex: nil,
           tag_list: [],
           options: {
             image: "ruby:2.1",
@@ -913,6 +944,7 @@ module Ci
             stage_idx: 1,
             name: "normal_job",
             commands: "test",
+            coverage_regex: nil,
             tag_list: [],
             options: {},
             when: "on_success",
@@ -958,6 +990,7 @@ module Ci
             stage_idx: 0,
             name: "job1",
             commands: "execute-script-for-job",
+            coverage_regex: nil,
             tag_list: [],
             options: {},
             when: "on_success",
@@ -970,6 +1003,7 @@ module Ci
             stage_idx: 0,
             name: "job2",
             commands: "execute-script-for-job",
+            coverage_regex: nil,
             tag_list: [],
             options: {},
             when: "on_success",

spec/lib/gitlab/ci/config/entry/coverage_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Entry::Coverage do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' 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[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 valid' do
+      let(:config) { '(malformed regexp' }
+
+      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
+  end
+end

spec/lib/gitlab/ci/config/entry/global_spec.rb

@@ -4,12 +4,17 @@ 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
+    it 'returns a hash' do
+      expect(described_class.nodes).to be_a(Hash)
     end
-    it 'returns a hash' do
-      expect(described_class.nodes).to be_a Hash
+    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])
+      end
     end
   end
@@ -35,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
@@ -176,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

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! }

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

spec/models/ci/build_spec.rb

@@ -221,6 +221,47 @@ describe Ci::Build, :models do
     end
   end
+  describe '#coverage_regex' do
+    subject { build.coverage_regex }
+
+    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) }
+      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
+
+        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
+    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
+
   describe 'deployment' do
     describe '#last_deployment' do
       subject { build.last_deployment }