summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZ.J. van de Weg <git@zjvandeweg.nl>2017-04-11 21:10:12 +0200
committerZ.J. van de Weg <git@zjvandeweg.nl>2017-04-12 12:03:39 +0200
commit23fbbe0c921ab3c878164d49316c4ce82b2035fb (patch)
treee2222ccadba40a2a351556668e808e2c8a5c640d
parent069c54a7d7a1d1d6ec1dc48c4212139eff6735df (diff)
downloadgitlab-ce-zj-fix-coverage-bug.tar.gz
Return nil as coverage instead of a File objectzj-fix-coverage-bug
Given a valid pipeline job, and a regex which wouldn't match to a jobs trace, the stream of the trace would return the File object. This was not the case when it matched a value, as that would have been return from the block. Now the `extract_coverage` method returns `nil` if no match was found.
-rw-r--r--app/workers/build_coverage_worker.rb3
-rw-r--r--lib/gitlab/ci/trace/stream.rb5
-rw-r--r--spec/lib/gitlab/ci/trace/stream_spec.rb6
-rw-r--r--spec/lib/gitlab/ci/trace_spec.rb20
4 files changed, 24 insertions, 10 deletions
diff --git a/app/workers/build_coverage_worker.rb b/app/workers/build_coverage_worker.rb
index def0ab1dde1..f7ae996bb17 100644
--- a/app/workers/build_coverage_worker.rb
+++ b/app/workers/build_coverage_worker.rb
@@ -3,7 +3,6 @@ class BuildCoverageWorker
include BuildQueue
def perform(build_id)
- Ci::Build.find_by(id: build_id)
- .try(:update_coverage)
+ Ci::Build.find_by(id: build_id)&.update_coverage
end
end
diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb
index 2af94e2c60e..41dcf846fed 100644
--- a/lib/gitlab/ci/trace/stream.rb
+++ b/lib/gitlab/ci/trace/stream.rb
@@ -76,11 +76,14 @@ module Gitlab
stream.each_line do |line|
matches = line.scan(regex)
next unless matches.is_a?(Array)
+ next if matches.empty?
match = matches.flatten.last
coverage = match.gsub(/\d+(\.\d+)?/).first
- return coverage.to_f if coverage.present?
+ return coverage if coverage.present?
end
+
+ nil
rescue
# if bad regex or something goes wrong we dont want to interrupt transition
# so we just silentrly ignore error for now
diff --git a/spec/lib/gitlab/ci/trace/stream_spec.rb b/spec/lib/gitlab/ci/trace/stream_spec.rb
index f1a1a71c528..2e57ccef182 100644
--- a/spec/lib/gitlab/ci/trace/stream_spec.rb
+++ b/spec/lib/gitlab/ci/trace/stream_spec.rb
@@ -167,7 +167,7 @@ describe Gitlab::Ci::Trace::Stream do
let(:data) { 'Coverage 1033 / 1051 LOC (98.29%) covered' }
let(:regex) { '\(\d+.\d+\%\) covered' }
- it { is_expected.to eq(98.29) }
+ it { is_expected.to eq("98.29") }
end
context 'valid content & bad regex' do
@@ -188,14 +188,14 @@ describe Gitlab::Ci::Trace::Stream do
let(:data) { ' (98.39%) covered. (98.29%) covered' }
let(:regex) { '\(\d+.\d+\%\) covered' }
- it { is_expected.to eq(98.29) }
+ it { is_expected.to eq("98.29") }
end
context 'using a regex capture' do
let(:data) { 'TOTAL 9926 3489 65%' }
let(:regex) { 'TOTAL\s+\d+\s+\d+\s+(\d{1,3}\%)' }
- it { is_expected.to eq(65) }
+ it { is_expected.to eq("65") }
end
end
end
diff --git a/spec/lib/gitlab/ci/trace_spec.rb b/spec/lib/gitlab/ci/trace_spec.rb
index 69e8dc9220d..9cb0b62590a 100644
--- a/spec/lib/gitlab/ci/trace_spec.rb
+++ b/spec/lib/gitlab/ci/trace_spec.rb
@@ -40,12 +40,24 @@ describe Gitlab::Ci::Trace do
describe '#extract_coverage' do
let(:regex) { '\(\d+.\d+\%\) covered' }
- before do
- trace.set('Coverage 1033 / 1051 LOC (98.29%) covered')
+ context 'matching coverage' do
+ before do
+ trace.set('Coverage 1033 / 1051 LOC (98.29%) covered')
+ end
+
+ it "returns valid coverage" do
+ expect(trace.extract_coverage(regex)).to eq("98.29")
+ end
end
- it "returns valid coverage" do
- expect(trace.extract_coverage(regex)).to eq(98.29)
+ context 'no coverage' do
+ before do
+ trace.set('No coverage')
+ end
+
+ it 'returs nil' do
+ expect(trace.extract_coverage(regex)).to be_nil
+ end
end
end