Revert "Merge branch '20248-add-coverage-regex-in-job-yaml' into 'master'"revert-93e98058

This reverts merge request !7447

18 files changed, 38 insertions, 312 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index b1f77bf242c..5fe8ddf69d7 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -275,23 +275,29 @@ 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)
-      update_attributes(coverage: coverage) if coverage.present?
+
+      if coverage.is_a? Numeric
+        update_attributes(coverage: coverage)
+      end
     end
 
     def extract_coverage(text, regex)
-      return unless regex
-
-      matches = text.scan(Regexp.new(regex)).last
-      matches = matches.last if matches.kind_of?(Array)
-      coverage = matches.gsub(/\d+(\.\d+)?/).first
+      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
+        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
-    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?
@@ -516,10 +522,6 @@ 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
diff --git a/changelogs/unreleased/issue-20428.yml b/changelogs/unreleased/issue-20428.yml
deleted file mode 100644
index 60da1c14702..00000000000
--- a/changelogs/unreleased/issue-20428.yml
+++ /dev/null
@@ -1,4 +0,0 @@
----
-title: Add ability to define a coverage regex in the .gitlab-ci.yml
-merge_request: 7447
-author: Leandro Camargo
diff --git a/db/migrate/20161114024742_add_coverage_regex_to_builds.rb b/db/migrate/20161114024742_add_coverage_regex_to_builds.rb
deleted file mode 100644
index 88aa5d52b39..00000000000
--- a/db/migrate/20161114024742_add_coverage_regex_to_builds.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-# 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 c73c311ccb2..5efb4f6595c 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -215,7 +215,6 @@ 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
diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 06810898cfe..f11257be5c3 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -76,7 +76,6 @@ There are a few reserved `keywords` that **cannot** be used as job names:
 | after_script  | no | Define commands that run after each job's script |
 | variables     | no | Define build variables |
 | cache         | no | Define list of files that should be cached between subsequent runs |
-| coverage      | no | Define coverage settings for all jobs |
 
 ### image and services
 
@@ -279,23 +278,6 @@ cache:
   untracked: true
 ```
 
-### coverage
-
-`coverage` allows you to configure how coverage will be filtered out from the
-build outputs. Setting this up globally will make all the jobs to use this
-setting for output filtering and extracting the coverage information from your
-builds.
-
-Regular expressions are the only valid kind of value expected here. So, using
-surrounding `/` is mandatory in order to consistently and explicitly represent
-a regular expression string. You must escape special characters if you want to
-match them literally.
-
-A simple example:
-```yaml
-coverage: /\(\d+\.\d+\) covered\./
-```
-
 ## Jobs
 
 `.gitlab-ci.yml` allows you to specify an unlimited number of jobs. Each job
@@ -337,7 +319,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 |
-| coverage      | no | Define coverage settings for a given job |
 
 ### script
 
@@ -1012,25 +993,6 @@ 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
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 649ee4d018b..7463bd719d5 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -61,7 +61,6 @@ 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],
diff --git a/lib/gitlab/ci/config/entry/coverage.rb b/lib/gitlab/ci/config/entry/coverage.rb
deleted file mode 100644
index 12a063059cb..00000000000
--- a/lib/gitlab/ci/config/entry/coverage.rb
+++ /dev/null
@@ -1,22 +0,0 @@
-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
diff --git a/lib/gitlab/ci/config/entry/global.rb b/lib/gitlab/ci/config/entry/global.rb
index ede97cc0504..a4ec8f0ff2f 100644
--- a/lib/gitlab/ci/config/entry/global.rb
+++ b/lib/gitlab/ci/config/entry/global.rb
@@ -33,11 +33,8 @@ module Gitlab
           entry :cache, Entry::Cache,
             description: 'Configure caching between build jobs.'
 
-          entry :coverage, Entry::Coverage,
-               description: 'Coverage configuration for this pipeline.'
-
           helpers :before_script, :image, :services, :after_script,
-                  :variables, :stages, :types, :cache, :coverage, :jobs
+                  :variables, :stages, :types, :cache, :jobs
 
           def compose!(_deps = nil)
             super(self) do
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index 69a5e6f433d..a55362f0b6b 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]
+                            after_script variables environment]
 
           validations do
             validates :config, allowed_keys: ALLOWED_KEYS
@@ -71,12 +71,9 @@ 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, :coverage
+                  :artifacts, :commands, :environment
 
           attributes :script, :tags, :allow_failure, :when, :dependencies
 
@@ -133,7 +130,6 @@ 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
diff --git a/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb b/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
index 9b9a0a8125a..f01975aab5c 100644
--- a/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
+++ b/lib/gitlab/ci/config/entry/legacy_validation_helpers.rb
@@ -28,21 +28,17 @@ 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 == '/'
-              validate_regexp(value[1...-1])
+              Regexp.new(value[1...-1])
             else
               true
             end
+          rescue RegexpError
+            false
           end
 
           def validate_boolean(value)
diff --git a/lib/gitlab/ci/config/entry/trigger.rb b/lib/gitlab/ci/config/entry/trigger.rb
index 16b234e6c59..28b0a9ffe01 100644
--- a/lib/gitlab/ci/config/entry/trigger.rb
+++ b/lib/gitlab/ci/config/entry/trigger.rb
@@ -9,7 +9,15 @@ module Gitlab
           include Validatable
 
           validations do
-            validates :config, array_of_strings_or_regexps: true
+            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
           end
         end
       end
diff --git a/lib/gitlab/ci/config/entry/validators.rb b/lib/gitlab/ci/config/entry/validators.rb
index bd7428b1272..8632dd0e233 100644
--- a/lib/gitlab/ci/config/entry/validators.rb
+++ b/lib/gitlab/ci/config/entry/validators.rb
@@ -54,51 +54,6 @@ 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]
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 49349035b3b..f824e2e1efe 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -4,33 +4,6 @@ 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' }
 
@@ -48,7 +21,6 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
-          coverage_regex: nil,
           tag_list: [],
           options: {},
           allow_failure: false,
@@ -463,7 +435,6 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
-          coverage_regex: nil,
           tag_list: [],
           options: {
             image: "ruby:2.1",
@@ -492,7 +463,6 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
-          coverage_regex: nil,
           tag_list: [],
           options: {
             image: "ruby:2.5",
@@ -732,7 +702,6 @@ module Ci
           stage_idx: 1,
           name: "rspec",
           commands: "pwd\nrspec",
-          coverage_regex: nil,
           tag_list: [],
           options: {
             image: "ruby:2.1",
@@ -944,7 +913,6 @@ module Ci
             stage_idx: 1,
             name: "normal_job",
             commands: "test",
-            coverage_regex: nil,
             tag_list: [],
             options: {},
             when: "on_success",
@@ -990,7 +958,6 @@ module Ci
             stage_idx: 0,
             name: "job1",
             commands: "execute-script-for-job",
-            coverage_regex: nil,
             tag_list: [],
             options: {},
             when: "on_success",
@@ -1003,7 +970,6 @@ 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
deleted file mode 100644
index 4c6bd859552..00000000000
--- a/spec/lib/gitlab/ci/config/entry/coverage_spec.rb
+++ /dev/null
@@ -1,54 +0,0 @@
-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
diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb
index d4f1780b174..e64c8d46bd8 100644
--- a/spec/lib/gitlab/ci/config/entry/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb
@@ -4,17 +4,12 @@ describe Gitlab::Ci::Config::Entry::Global do
   let(:global) { described_class.new(hash) }
 
   describe '.nodes' do
-    it 'returns a hash' do
-      expect(described_class.nodes).to be_a(Hash)
+    it 'can contain global config keys' do
+      expect(described_class.nodes).to include :before_script
     end
 
-    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
+    it 'returns a hash' do
+      expect(described_class.nodes).to be_a Hash
     end
   end
 
@@ -40,7 +35,7 @@ describe Gitlab::Ci::Config::Entry::Global do
         end
 
         it 'creates node object for each entry' do
-          expect(global.descendants.count).to eq 9
+          expect(global.descendants.count).to eq 8
         end
 
         it 'creates node object using valid class' do
@@ -181,7 +176,7 @@ describe Gitlab::Ci::Config::Entry::Global do
 
       describe '#nodes' do
         it 'instantizes all nodes' do
-          expect(global.descendants.count).to eq 9
+          expect(global.descendants.count).to eq 8
         end
 
         it 'contains unspecified nodes' do
diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb
index d20f4ec207d..fc9b8b86dc4 100644
--- a/spec/lib/gitlab/ci/config/entry/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb
@@ -3,20 +3,6 @@ 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/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 95b230e4f5c..493bc2db21a 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -222,7 +222,6 @@ CommitStatus:
 - queued_at
 - token
 - lock_version
-- coverage_regex
 Ci::Variable:
 - id
 - project_id
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 4080092405d..3888ae651f6 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -221,47 +221,6 @@ 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 }