Merge branch 'mo-persist-junit-data-build-finished' into 'master'

Persist JUnit data when a build is finished

See merge request gitlab-org/gitlab!33093

app/models/ci/build.rb

@@ -688,6 +688,10 @@ module Ci
       job_artifacts.any?
     end
 
+    def has_test_reports?
+      job_artifacts.test_reports.exists?
+    end
+
     def has_old_trace?
       old_trace.present?
     end

app/models/ci/build_report_result.rb

@@ -11,5 +11,35 @@ module Ci
 
     validates :build, :project, presence: true
     validates :data, json_schema: { filename: "build_report_result_data" }
+
+    store_accessor :data, :tests
+
+    def tests_name
+      tests.dig("name")
+    end
+
+    def tests_duration
+      tests.dig("duration")
+    end
+
+    def tests_success
+      tests.dig("success").to_i
+    end
+
+    def tests_failed
+      tests.dig("failed").to_i
+    end
+
+    def tests_errored
+      tests.dig("errored").to_i
+    end
+
+    def tests_skipped
+      tests.dig("skipped").to_i
+    end
+
+    def tests_total
+      [tests_success, tests_failed, tests_errored, tests_skipped].sum
+    end
   end
 end

app/services/ci/build_report_result_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class BuildReportResultService
+    def execute(build)
+      return unless Feature.enabled?(:build_report_summary, build.project)
+      return unless build.has_test_reports?
+
+      build.report_results.create!(
+        project_id: build.project_id,
+        data: tests_params(build)
+      )
+    end
+
+    private
+
+    def generate_test_suite_report(build)
+      build.collect_test_reports!(Gitlab::Ci::Reports::TestReports.new)
+    end
+
+    def tests_params(build)
+      test_suite = generate_test_suite_report(build)
+
+      {
+        tests: {
+          name: test_suite.name,
+          duration: test_suite.total_time,
+          failed: test_suite.failed_count,
+          errored: test_suite.error_count,
+          skipped: test_suite.skipped_count,
+          success: test_suite.success_count
+        }
+      }
+    end
+  end
+end

app/validators/json_schemas/build_report_result_data.json

@@ -3,9 +3,9 @@
   "type": "object",
   "properties": {
     "coverage": { "type": "float" },
-    "junit": {
+    "tests": {
       "type": "object",
-      "items": { "$ref": "./build_report_result_data_junit.json" }
+      "items": { "$ref": "./build_report_result_data_tests.json" }
     }
   },
   "additionalProperties": false

app/validators/json_schemas/build_report_result_data_junit.json → app/validators/json_schemas/build_report_result_data_tests.json

 {
-  "description": "Build report result data junit",
+  "description": "Build report result data tests",
   "type": "object",
   "properties": {
     "name": { "type": "string" },

app/workers/all_queues.yml

@@ -803,6 +803,14 @@
   :weight: 1
   :idempotent: 
   :tags: []
+- :name: pipeline_background:ci_build_report_result
+  :feature_category: :continuous_integration
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: pipeline_background:ci_build_trace_chunk_flush
   :feature_category: :continuous_integration
   :has_external_dependencies: 

app/workers/build_finished_worker.rb

@@ -28,6 +28,7 @@ class BuildFinishedWorker # rubocop:disable Scalability/IdempotentWorker
     # We execute these in sync to reduce IO.
     BuildTraceSectionsWorker.new.perform(build.id)
     BuildCoverageWorker.new.perform(build.id)
+    Ci::BuildReportResultWorker.new.perform(build.id)
 
     # We execute these async as these are independent operations.
     BuildHooksWorker.perform_async(build.id)

app/workers/ci/build_report_result_worker.rb

+# frozen_string_literal: true
+
+module Ci
+  class BuildReportResultWorker
+    include ApplicationWorker
+    include PipelineBackgroundQueue
+
+    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/factories/ci/build_report_results.rb

@@ -6,7 +6,7 @@ FactoryBot.define do
     project factory: :project
     data do
       {
-        junit: {
+        tests: {
           name: "rspec",
           duration: 0.42,
           failed: 0,
@@ -20,7 +20,7 @@ FactoryBot.define do
     trait :with_junit_success do
       data do
         {
-          junit: {
+          tests: {
             name: "rspec",
             duration: 0.42,
             failed: 0,

spec/models/ci/build_report_result_spec.rb

@@ -29,4 +29,46 @@ describe Ci::BuildReportResult do
       end
     end
   end
+
+  describe '#tests_name' do
+    it 'returns the suite name' do
+      expect(build_report_result.tests_name).to eq("rspec")
+    end
+  end
+
+  describe '#tests_duration' do
+    it 'returns the suite duration' do
+      expect(build_report_result.tests_duration).to eq(0.42)
+    end
+  end
+
+  describe '#tests_success' do
+    it 'returns the success count' do
+      expect(build_report_result.tests_success).to eq(2)
+    end
+  end
+
+  describe '#tests_failed' do
+    it 'returns the failed count' do
+      expect(build_report_result.tests_failed).to eq(0)
+    end
+  end
+
+  describe '#tests_errored' do
+    it 'returns the errored count' do
+      expect(build_report_result.tests_errored).to eq(0)
+    end
+  end
+
+  describe '#tests_skipped' do
+    it 'returns the skipped count' do
+      expect(build_report_result.tests_skipped).to eq(0)
+    end
+  end
+
+  describe '#tests_total' do
+    it 'returns the total count' do
+      expect(build_report_result.tests_total).to eq(2)
+    end
+  end
 end

spec/models/ci/build_spec.rb

@@ -875,6 +875,22 @@ describe Ci::Build do
     end
   end
 
+  describe '#has_test_reports?' do
+    subject { build.has_test_reports? }
+
+    context 'when build has a test report' do
+      let(:build) { create(:ci_build, :test_reports) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when build does not have a test report' do
+      let(:build) { create(:ci_build) }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe '#has_old_trace?' do
     subject { build.has_old_trace? }
 

spec/services/ci/build_report_result_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::BuildReportResultService do
+  describe "#execute" do
+    subject(:build_report_result) { described_class.new.execute(build) }
+
+    context 'when build is finished' do
+      let(:build) { create(:ci_build, :success, :test_reports) }
+
+      it 'creates a build report result entry', :aggregate_failures do
+        expect(build_report_result.tests_name).to eq("test")
+        expect(build_report_result.tests_success).to eq(2)
+        expect(build_report_result.tests_failed).to eq(2)
+        expect(build_report_result.tests_errored).to eq(0)
+        expect(build_report_result.tests_skipped).to eq(0)
+        expect(build_report_result.tests_duration).to eq(0.010284)
+        expect(Ci::BuildReportResult.count).to eq(1)
+      end
+
+      context 'when feature is disable' do
+        it 'does not persist the data' do
+          stub_feature_flags(build_report_summary: false)
+
+          subject
+
+          expect(Ci::BuildReportResult.count).to eq(0)
+        end
+      end
+
+      context 'when data has already been persisted' do
+        it 'raises an error and do not persist the same data twice' do
+          expect { 2.times { described_class.new.execute(build) } }.to raise_error(ActiveRecord::RecordNotUnique)
+
+          expect(Ci::BuildReportResult.count).to eq(1)
+        end
+      end
+    end
+
+    context 'when build is running and test report does not exist' do
+      let(:build) { create(:ci_build, :running) }
+
+      it 'does not persist data' do
+        subject
+
+        expect(Ci::BuildReportResult.count).to eq(0)
+      end
+    end
+  end
+end

spec/workers/build_finished_worker_spec.rb

@@ -3,7 +3,11 @@
 require 'spec_helper'
 
 describe BuildFinishedWorker do
+  subject { described_class.new.perform(build.id) }
+
   describe '#perform' do
+    let(:build) { create(:ci_build, :success, pipeline: create(:ci_pipeline)) }
+
     context 'when build exists' do
       let!(:build) { create(:ci_build) }
 
@@ -18,8 +22,10 @@ describe BuildFinishedWorker do
         expect(BuildHooksWorker).to receive(:perform_async)
         expect(ArchiveTraceWorker).to receive(:perform_async)
         expect(ExpirePipelineCacheWorker).to receive(:perform_async)
+        expect(ChatNotificationWorker).not_to receive(:perform_async)
+        expect(Ci::BuildReportResultWorker).not_to receive(:perform)
 
-        described_class.new.perform(build.id)
+        subject
       end
     end
 
@@ -30,23 +36,26 @@ describe BuildFinishedWorker do
       end
     end
 
-    it 'schedules a ChatNotification job for a chat build' do
-      build = create(:ci_build, :success, pipeline: create(:ci_pipeline, source: :chat))
+    context 'when build has a chat' do
+      let(:build) { create(:ci_build, :success, pipeline: create(:ci_pipeline, source: :chat)) }
 
-      expect(ChatNotificationWorker)
-        .to receive(:perform_async)
-        .with(build.id)
+      it 'schedules a ChatNotification job' do
+        expect(ChatNotificationWorker).to receive(:perform_async).with(build.id)
 
-      described_class.new.perform(build.id)
+        subject
+      end
     end
 
-    it 'does not schedule a ChatNotification job for a regular build' do
-      build = create(:ci_build, :success, pipeline: create(:ci_pipeline))
+    context 'when build has a test report' do
+      let(:build) { create(:ci_build, :test_reports) }
 
-      expect(ChatNotificationWorker)
-        .not_to receive(:perform_async)
+      it 'schedules a BuildReportResult job' do
+        expect_next_instance_of(Ci::BuildReportResultWorker) do |worker|
+          expect(worker).to receive(:perform).with(build.id)
+        end
 
-      described_class.new.perform(build.id)
+        subject
+      end
     end
   end
 end

spec/workers/ci/build_report_result_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+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