Merge branch 'eb-clean-up-build-finished-worker' into 'master'

Simplify BuildFinishedWorker

See merge request gitlab-org/gitlab!53229

app/workers/all_queues.yml

@@ -1084,15 +1084,6 @@
   :idempotent:
   :tags:
   - :requires_disk_io
-- :name: pipeline_background:ci_build_report_result
-  :feature_category: :continuous_integration
-  :has_external_dependencies:
-  :urgency: :low
-  :resource_boundary: :unknown
-  :weight: 1
-  :idempotent: true
-  :tags:
-  - :requires_disk_io
 - :name: pipeline_background:ci_build_trace_chunk_flush
   :feature_category: :continuous_integration
   :has_external_dependencies:
@@ -1181,24 +1172,6 @@
   :weight: 4
   :idempotent:
   :tags: []
-- :name: pipeline_default:build_coverage
-  :feature_category: :continuous_integration
-  :has_external_dependencies:
-  :urgency: :low
-  :resource_boundary: :unknown
-  :weight: 3
-  :idempotent:
-  :tags:
-  - :requires_disk_io
-- :name: pipeline_default:build_trace_sections
-  :feature_category: :continuous_integration
-  :has_external_dependencies:
-  :urgency: :low
-  :resource_boundary: :unknown
-  :weight: 3
-  :idempotent:
-  :tags:
-  - :requires_disk_io
 - :name: pipeline_default:ci_create_cross_project_pipeline
   :feature_category: :continuous_integration
   :has_external_dependencies:

app/workers/build_coverage_worker.rb

-# frozen_string_literal: true
-
-class BuildCoverageWorker # rubocop:disable Scalability/IdempotentWorker
-  include ApplicationWorker
-  include PipelineQueue
-
-  tags :requires_disk_io
-
-  # rubocop: disable CodeReuse/ActiveRecord
-  def perform(build_id)
-    Ci::Build.find_by(id: build_id)&.update_coverage
-  end
-  # rubocop: enable CodeReuse/ActiveRecord
-end

app/workers/build_finished_worker.rb

@@ -29,9 +29,9 @@ class BuildFinishedWorker # rubocop:disable Scalability/IdempotentWorker
   # @param [Ci::Build] build The build to process.
   def process_build(build)
     # We execute these in sync to reduce IO.
-    BuildTraceSectionsWorker.new.perform(build.id)
-    BuildCoverageWorker.new.perform(build.id)
-    Ci::BuildReportResultWorker.new.perform(build.id)
+    build.parse_trace_sections!
+    build.update_coverage
+    Ci::BuildReportResultService.new.execute(build)
 
     # We execute these async as these are independent operations.
     BuildHooksWorker.perform_async(build.id)

app/workers/build_trace_sections_worker.rb

-# frozen_string_literal: true
-
-class BuildTraceSectionsWorker # rubocop:disable Scalability/IdempotentWorker
-  include ApplicationWorker
-  include PipelineQueue
-
-  tags :requires_disk_io
-
-  # rubocop: disable CodeReuse/ActiveRecord
-  def perform(build_id)
-    Ci::Build.find_by(id: build_id)&.parse_trace_sections!
-  end
-  # rubocop: enable CodeReuse/ActiveRecord
-end

app/workers/ci/build_report_result_worker.rb

-# frozen_string_literal: true
-
-module Ci
-  class BuildReportResultWorker
-    include ApplicationWorker
-    include PipelineBackgroundQueue
-
-    tags :requires_disk_io
-
-    idempotent!
-
-    def perform(build_id)
-      Ci::Build.find_by_id(build_id).try do |build|
-        Ci::BuildReportResultService.new.execute(build)
-      end
-    end
-  end
-end

spec/workers/build_coverage_worker_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe BuildCoverageWorker do
-  describe '#perform' do
-    context 'when build exists' do
-      let!(:build) { create(:ci_build) }
-
-      it 'updates code coverage' do
-        expect_any_instance_of(Ci::Build)
-          .to receive(:update_coverage)
-
-        described_class.new.perform(build.id)
-      end
-    end
-
-    context 'when build does not exist' do
-      it 'does not raise exception' do
-        expect { described_class.new.perform(123) }
-          .not_to raise_error
-      end
-    end
-  end
-end

spec/workers/build_finished_worker_spec.rb

@@ -11,20 +11,16 @@ RSpec.describe BuildFinishedWorker do
     context 'when build exists' do
       let!(:build) { create(:ci_build) }
 
-      it 'calculates coverage and calls hooks', :aggregate_failures do
-        trace_worker = double('trace worker')
-        coverage_worker = double('coverage worker')
-
-        allow(BuildTraceSectionsWorker).to receive(:new).and_return(trace_worker)
-        allow(BuildCoverageWorker).to receive(:new).and_return(coverage_worker)
+      before do
+        expect(Ci::Build).to receive(:find_by).with(id: build.id).and_return(build)
+      end
 
-        # Unfortunately, `ordered` does not seem to work when called within `allow_next_instance_of`
-        # so we're doing this the long and dirty way
-        expect(trace_worker).to receive(:perform).ordered
-        expect(coverage_worker).to receive(:perform).ordered
+      it 'calculates coverage and calls hooks', :aggregate_failures do
+        expect(build).to receive(:parse_trace_sections!).ordered
+        expect(build).to receive(:update_coverage).ordered
 
-        expect_next_instance_of(Ci::BuildReportResultWorker) do |instance|
-          expect(instance).to receive(:perform)
+        expect_next_instance_of(Ci::BuildReportResultService) do |build_report_result_service|
+          expect(build_report_result_service).to receive(:execute).with(build)
         end
 
         expect(BuildHooksWorker).to receive(:perform_async)

spec/workers/build_trace_sections_worker_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe BuildTraceSectionsWorker do
-  describe '#perform' do
-    context 'when build exists' do
-      let!(:build) { create(:ci_build) }
-
-      it 'updates trace sections' do
-        expect_any_instance_of(Ci::Build)
-          .to receive(:parse_trace_sections!)
-
-        described_class.new.perform(build.id)
-      end
-    end
-
-    context 'when build does not exist' do
-      it 'does not raise exception' do
-        expect { described_class.new.perform(123) }
-          .not_to raise_error
-      end
-    end
-  end
-end

spec/workers/ci/build_report_result_worker_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Ci::BuildReportResultWorker do
-  subject { described_class.new.perform(build_id) }
-
-  context 'when build exists' do
-    let(:build) { create(:ci_build) }
-    let(:build_id) { build.id }
-
-    it 'calls build report result service' do
-      expect_next_instance_of(Ci::BuildReportResultService) do |build_report_result_service|
-        expect(build_report_result_service).to receive(:execute)
-      end
-
-      subject
-    end
-  end
-
-  context 'when build does not exist' do
-    let(:build_id) { -1 }
-
-    it 'does not call build report result service' do
-      expect(Ci::BuildReportResultService).not_to receive(:execute)
-
-      subject
-    end
-  end
-end