diff --git a/app/workers/pipeline_metrics_worker.rb b/app/workers/pipeline_metrics_worker.rb index 3d1cd770515..7bb92df3bbd 100644 --- a/app/workers/pipeline_metrics_worker.rb +++ b/app/workers/pipeline_metrics_worker.rb @@ -5,11 +5,26 @@ class PipelineMetricsWorker def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline| - merge_requests = pipeline.merge_requests.map(&:id) - - metrics = MergeRequest::Metrics.where(merge_request_id: merge_requests) - metrics.update_all(latest_build_started_at: pipeline.started_at) if pipeline.active? - metrics.update_all(latest_build_finished_at: pipeline.finished_at) if pipeline.success? + update_metrics_for_active_pipeline(pipeline) if pipeline.active? + update_metrics_for_succeeded_pipeline(pipeline) if pipeline.success? end end + + private + + def update_metrics_for_active_pipeline(pipeline) + metrics(pipeline).update_all(latest_build_started_at: pipeline.started_at, latest_build_finished_at: nil) + end + + def update_metrics_for_succeeded_pipeline(pipeline) + metrics(pipeline).update_all(latest_build_started_at: pipeline.started_at, latest_build_finished_at: pipeline.finished_at) + end + + def metrics(pipeline) + MergeRequest::Metrics.where(merge_request_id: merge_requests(pipeline)) + end + + def merge_requests(pipeline) + pipeline.merge_requests.map(&:id) + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 550a890797e..163c0b5c516 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -187,33 +187,24 @@ describe Ci::Pipeline, models: true do end end - describe "merge request metrics" do + describe 'merge request metrics' do let(:project) { FactoryGirl.create :project } let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) } let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) } + before do + expect(PipelineMetricsWorker).to receive(:perform_async).with(pipeline.id) + end + context 'when transitioning to running' do - it 'records the build start time' do - time = Time.now - Timecop.freeze(time) { build.run } - - expect(merge_request.reload.metrics.latest_build_started_at).to be_within(1.second).of(time) - end - - it 'clears the build end time' do - build.run - - expect(merge_request.reload.metrics.latest_build_finished_at).to be_nil + it 'schedules metrics workers' do + pipeline.run end end context 'when transitioning to success' do - it 'records the build end time' do - build.run - time = Time.now - Timecop.freeze(time) { build.success } - - expect(merge_request.reload.metrics.latest_build_finished_at).to be_within(1.second).of(time) + it 'schedules metrics workers' do + pipeline.succeed end end end diff --git a/spec/workers/pipeline_metrics_worker_spec.rb b/spec/workers/pipeline_metrics_worker_spec.rb new file mode 100644 index 00000000000..f58df3f0d6e --- /dev/null +++ b/spec/workers/pipeline_metrics_worker_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe PipelineMetricsWorker do + let(:project) { create(:project) } + let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) } + + let(:pipeline) do + create(:ci_empty_pipeline, + status: status, + project: project, + ref: 'master', + sha: project.repository.commit('master').id, + started_at: 1.hour.ago, + finished_at: Time.now) + end + + describe '#perform' do + subject { described_class.new.perform(pipeline.id) } + + context 'when pipeline is running' do + let(:status) { 'running' } + + it 'records the build start time' do + subject + + expect(merge_request.reload.metrics.latest_build_started_at).to eq(pipeline.started_at) + end + + it 'clears the build end time' do + subject + + expect(merge_request.reload.metrics.latest_build_finished_at).to be_nil + end + end + + context 'when pipeline succeeded' do + let(:status) { 'success' } + + it 'records the build end time' do + subject + + expect(merge_request.reload.metrics.latest_build_finished_at).to eq(pipeline.finished_at) + end + end + end +end