Commit 8fe708f4 authored by Leandro Camargo's avatar Leandro Camargo

Make more code improvements around the '/' stripping logic

parent 518fd2eb
...@@ -275,30 +275,23 @@ module Ci ...@@ -275,30 +275,23 @@ module Ci
end end
def update_coverage def update_coverage
regex = coverage_regex coverage = extract_coverage(trace, coverage_regex)
update_attributes(coverage: coverage) if coverage.is_a?(Numeric)
return unless regex
coverage = extract_coverage(trace, regex[1...-1])
if coverage.is_a? Numeric
update_attributes(coverage: coverage)
end
end end
def extract_coverage(text, regex) def extract_coverage(text, regex)
begin 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? matches = text.scan(Regexp.new(regex)).last
coverage.to_f matches = matches.last if matches.kind_of?(Array)
end coverage = matches.gsub(/\d+(\.\d+)?/).first
rescue
# if bad regex or something goes wrong we dont want to interrupt transition if coverage.present?
# so we just silentrly ignore error for now coverage.to_f
end end
rescue
# if bad regex or something goes wrong we dont want to interrupt transition
# so we just silentrly ignore error for now
end end
def has_trace_file? def has_trace_file?
...@@ -524,9 +517,7 @@ module Ci ...@@ -524,9 +517,7 @@ module Ci
end end
def coverage_regex def coverage_regex
super || super || project.try(:build_coverage_regex)
project.try(:build_coverage_regex).presence &&
"/#{project.build_coverage_regex}/"
end end
def when def when
......
...@@ -11,6 +11,10 @@ module Gitlab ...@@ -11,6 +11,10 @@ module Gitlab
validations do validations do
validates :config, regexp: true validates :config, regexp: true
end end
def value
@config[1...-1]
end
end end
end end
end end
......
...@@ -66,7 +66,7 @@ module Gitlab ...@@ -66,7 +66,7 @@ module Gitlab
private private
def look_like_regexp?(value) def look_like_regexp?(value)
value =~ %r{\A/.*/\z} value.start_with?('/') && value.end_with?('/')
end end
def validate_regexp(value) def validate_regexp(value)
...@@ -78,7 +78,7 @@ module Gitlab ...@@ -78,7 +78,7 @@ module Gitlab
end end
end end
class ArrayOfStringsOrRegexps < RegexpValidator class ArrayOfStringsOrRegexpsValidator < RegexpValidator
def validate_each(record, attribute, value) def validate_each(record, attribute, value)
unless validate_array_of_strings_or_regexps(value) unless validate_array_of_strings_or_regexps(value)
record.errors.add(attribute, 'should be an array of strings or regexps') record.errors.add(attribute, 'should be an array of strings or regexps')
...@@ -94,12 +94,8 @@ module Gitlab ...@@ -94,12 +94,8 @@ module Gitlab
def validate_string_or_regexp(value) def validate_string_or_regexp(value)
return true if value.is_a?(Symbol) return true if value.is_a?(Symbol)
return false unless value.is_a?(String) return false unless value.is_a?(String)
return validate_regexp(value) if look_like_regexp?(value)
if look_like_regexp?(value) true
validate_regexp(value)
else
true
end
end end
end end
......
...@@ -17,7 +17,7 @@ module Ci ...@@ -17,7 +17,7 @@ module Ci
end end
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/') } it { is_expected.to include(coverage_regex: '\(\d+\.\d+\) covered') }
end end
context "but 'rspec' job also has coverage set" do context "but 'rspec' job also has coverage set" do
...@@ -25,7 +25,7 @@ module Ci ...@@ -25,7 +25,7 @@ module Ci
config_base[:rspec][:coverage] = '/Code coverage: \d+\.\d+/' config_base[:rspec][:coverage] = '/Code coverage: \d+\.\d+/'
end 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 end
end end
...@@ -48,6 +48,7 @@ module Ci ...@@ -48,6 +48,7 @@ module Ci
stage_idx: 1, stage_idx: 1,
name: "rspec", name: "rspec",
commands: "pwd\nrspec", commands: "pwd\nrspec",
coverage_regex: nil,
tag_list: [], tag_list: [],
options: {}, options: {},
allow_failure: false, allow_failure: false,
...@@ -462,6 +463,7 @@ module Ci ...@@ -462,6 +463,7 @@ module Ci
stage_idx: 1, stage_idx: 1,
name: "rspec", name: "rspec",
commands: "pwd\nrspec", commands: "pwd\nrspec",
coverage_regex: nil,
tag_list: [], tag_list: [],
options: { options: {
image: "ruby:2.1", image: "ruby:2.1",
...@@ -490,6 +492,7 @@ module Ci ...@@ -490,6 +492,7 @@ module Ci
stage_idx: 1, stage_idx: 1,
name: "rspec", name: "rspec",
commands: "pwd\nrspec", commands: "pwd\nrspec",
coverage_regex: nil,
tag_list: [], tag_list: [],
options: { options: {
image: "ruby:2.5", image: "ruby:2.5",
...@@ -729,6 +732,7 @@ module Ci ...@@ -729,6 +732,7 @@ module Ci
stage_idx: 1, stage_idx: 1,
name: "rspec", name: "rspec",
commands: "pwd\nrspec", commands: "pwd\nrspec",
coverage_regex: nil,
tag_list: [], tag_list: [],
options: { options: {
image: "ruby:2.1", image: "ruby:2.1",
...@@ -940,6 +944,7 @@ module Ci ...@@ -940,6 +944,7 @@ module Ci
stage_idx: 1, stage_idx: 1,
name: "normal_job", name: "normal_job",
commands: "test", commands: "test",
coverage_regex: nil,
tag_list: [], tag_list: [],
options: {}, options: {},
when: "on_success", when: "on_success",
...@@ -985,6 +990,7 @@ module Ci ...@@ -985,6 +990,7 @@ module Ci
stage_idx: 0, stage_idx: 0,
name: "job1", name: "job1",
commands: "execute-script-for-job", commands: "execute-script-for-job",
coverage_regex: nil,
tag_list: [], tag_list: [],
options: {}, options: {},
when: "on_success", when: "on_success",
...@@ -997,6 +1003,7 @@ module Ci ...@@ -997,6 +1003,7 @@ module Ci
stage_idx: 0, stage_idx: 0,
name: "job2", name: "job2",
commands: "execute-script-for-job", commands: "execute-script-for-job",
coverage_regex: nil,
tag_list: [], tag_list: [],
options: {}, options: {},
when: "on_success", when: "on_success",
......
...@@ -23,7 +23,7 @@ describe Gitlab::Ci::Config::Entry::Coverage do ...@@ -23,7 +23,7 @@ describe Gitlab::Ci::Config::Entry::Coverage do
describe '#value' do describe '#value' do
subject { entry.value } subject { entry.value }
it { is_expected.to eq(config) } it { is_expected.to eq(config[1...-1]) }
end end
describe '#errors' do describe '#errors' do
......
...@@ -40,7 +40,7 @@ describe Gitlab::Ci::Config::Entry::Global do ...@@ -40,7 +40,7 @@ describe Gitlab::Ci::Config::Entry::Global do
end end
it 'creates node object for each entry' do it 'creates node object for each entry' do
expect(global.descendants.count).to eq 8 expect(global.descendants.count).to eq 9
end end
it 'creates node object using valid class' do it 'creates node object using valid class' do
...@@ -181,7 +181,7 @@ describe Gitlab::Ci::Config::Entry::Global do ...@@ -181,7 +181,7 @@ describe Gitlab::Ci::Config::Entry::Global do
describe '#nodes' do describe '#nodes' do
it 'instantizes all nodes' do it 'instantizes all nodes' do
expect(global.descendants.count).to eq 8 expect(global.descendants.count).to eq 9
end end
it 'contains unspecified nodes' do it 'contains unspecified nodes' do
......
...@@ -232,11 +232,11 @@ describe Ci::Build, :models do ...@@ -232,11 +232,11 @@ describe Ci::Build, :models do
end end
context 'and coverage_regex attribute is not set' do context 'and coverage_regex attribute is not set' do
it { is_expected.to eq("/#{project_regex}/") } it { is_expected.to eq(project_regex) }
end end
context 'but coverage_regex attribute is also set' do 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 before do
build.coverage_regex = build_regex build.coverage_regex = build_regex
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment