Make more code improvements around the '/' stripping logic

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