Merge branch 'mo-refactor-pipeline-artifact' into 'master'

Refactor PipelineArtifact for codequality

See merge request gitlab-org/gitlab!51762

app/models/ci/pipeline.rb

@@ -995,13 +995,21 @@ module Ci
     end
 
     def has_coverage_reports?
-      pipeline_artifacts&.has_code_coverage?
+      pipeline_artifacts&.has_report?(:code_coverage)
     end
 
     def can_generate_coverage_reports?
       has_reports?(Ci::JobArtifact.coverage_reports)
     end
 
+    def has_codequality_reports?
+      pipeline_artifacts&.has_report?(:code_quality)
+    end
+
+    def can_generate_codequality_reports?
+      has_reports?(Ci::JobArtifact.codequality_reports)
+    end
+
     def test_report_summary
       strong_memoize(:test_report_summary) do
         Gitlab::Ci::Reports::TestReportSummary.new(latest_builds_report_results)

app/models/ci/pipeline_artifact.rb

@@ -30,15 +30,18 @@ module Ci
     update_project_statistics project_statistics_name: :pipeline_artifacts_size
 
     enum file_type: {
-      code_coverage: 1
+      code_coverage: 1,
+      code_quality: 2
     }
 
-    def self.has_code_coverage?
-      where(file_type: :code_coverage).exists?
-    end
+    class << self
+      def has_report?(file_type)
+        where(file_type: file_type).exists?
+      end
 
-    def self.find_with_code_coverage
-      find_by(file_type: :code_coverage)
+      def find_by_file_type(file_type)
+        find_by(file_type: file_type)
+      end
     end
 
     def present

app/services/ci/generate_coverage_reports_service.rb

@@ -12,7 +12,7 @@ module Ci
       {
         status: :parsed,
         key: key(base_pipeline, head_pipeline),
-        data: head_pipeline.pipeline_artifacts.find_with_code_coverage.present.for_files(merge_request.new_paths)
+        data: head_pipeline.pipeline_artifacts.find_by_file_type(:code_coverage).present.for_files(merge_request.new_paths)
       }
     rescue => e
       Gitlab::ErrorTracking.track_exception(e, project_id: project.id)

spec/factories/ci/pipeline_artifacts.rb

@@ -34,5 +34,15 @@ FactoryBot.define do
 
       size { file.size }
     end
+
+    trait :codequality_report do
+      file_type { :code_quality }
+      size { 2.megabytes }
+
+      after(:build) do |artifact, _evaluator|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/pipeline_artifacts/code_quality.json'), 'application/json')
+      end
+    end
   end
 end

spec/factories/ci/pipelines.rb

@@ -163,6 +163,12 @@ FactoryBot.define do
         end
       end
 
+      trait :with_codequality_report_artifact do
+        after(:build) do |pipeline, evaluator|
+          pipeline.pipeline_artifacts << build(:ci_pipeline_artifact, :codequality_report, pipeline: pipeline, project: pipeline.project)
+        end
+      end
+
       trait :with_terraform_reports do
         status { :success }
 

spec/fixtures/pipeline_artifacts/code_quality.json

+{
+  "files": {
+    "demo.rb": [
+      {
+        "line": 5,
+        "description": "Method `new_array` has 8 arguments (exceeds 4 allowed). Consider refactoring.",
+        "severity": "major"
+      },
+      {
+        "line": 5,
+        "description": "Avoid parameter lists longer than 5 parameters. [8/5]",
+        "severity": "minor"
+      }
+    ]
+  }
+}

spec/models/ci/pipeline_artifact_spec.rb

@@ -76,38 +76,98 @@ RSpec.describe Ci::PipelineArtifact, type: :model do
     end
   end
 
-  describe '.has_code_coverage?' do
-    subject { Ci::PipelineArtifact.has_code_coverage? }
+  describe '.has_report?' do
+    subject(:pipeline_artifact) { Ci::PipelineArtifact.has_report?(file_type) }
 
-    context 'when pipeline artifact has a code coverage' do
-      let!(:pipeline_artifact) { create(:ci_pipeline_artifact) }
+    context 'when file_type is code_coverage' do
+      let(:file_type) { :code_coverage }
+
+      context 'when pipeline artifact has a coverage report' do
+        let!(:pipeline_artifact) { create(:ci_pipeline_artifact) }
+
+        it 'returns true' do
+          expect(pipeline_artifact).to be_truthy
+        end
+      end
+
+      context 'when pipeline artifact does not have a coverage report' do
+        it 'returns false' do
+          expect(pipeline_artifact).to be_falsey
+        end
+      end
+    end
+
+    context 'when file_type is code_quality' do
+      let(:file_type) { :code_quality }
+
+      context 'when pipeline artifact has a quality report' do
+        let!(:pipeline_artifact) { create(:ci_pipeline_artifact, :codequality_report) }
 
-      it 'returns true' do
-        expect(subject).to be_truthy
+        it 'returns true' do
+          expect(pipeline_artifact).to be_truthy
+        end
+      end
+
+      context 'when pipeline artifact does not have a quality report' do
+        it 'returns false' do
+          expect(pipeline_artifact).to be_falsey
+        end
       end
     end
 
-    context 'when pipeline artifact does not have a code coverage' do
+    context 'when file_type is nil' do
+      let(:file_type) { nil }
+
       it 'returns false' do
-        expect(subject).to be_falsey
+        expect(pipeline_artifact).to be_falsey
       end
     end
   end
 
-  describe '.find_with_code_coverage' do
-    subject { Ci::PipelineArtifact.find_with_code_coverage }
+  describe '.find_by_file_type' do
+    subject(:pipeline_artifact) { Ci::PipelineArtifact.find_by_file_type(file_type) }
 
-    context 'when pipeline artifact has a coverage report' do
-      let!(:coverage_report) { create(:ci_pipeline_artifact) }
+    context 'when file_type is code_coverage' do
+      let(:file_type) { :code_coverage }
+
+      context 'when pipeline artifact has a coverage report' do
+        let!(:coverage_report) { create(:ci_pipeline_artifact) }
+
+        it 'returns a pipeline artifact with a coverage report' do
+          expect(pipeline_artifact.file_type).to eq('code_coverage')
+        end
+      end
+
+      context 'when pipeline artifact does not have a coverage report' do
+        it 'returns nil' do
+          expect(pipeline_artifact).to be_nil
+        end
+      end
+    end
+
+    context 'when file_type is code_quality' do
+      let(:file_type) { :code_quality }
+
+      context 'when pipeline artifact has a quality report' do
+        let!(:coverage_report) { create(:ci_pipeline_artifact, :codequality_report) }
 
-      it 'returns a pipeline artifact with a code coverage' do
-        expect(subject.file_type).to eq('code_coverage')
+        it 'returns a pipeline artifact with a quality report' do
+          expect(pipeline_artifact.file_type).to eq('code_quality')
+        end
+      end
+
+      context 'when pipeline artifact does not have a quality report' do
+        it 'returns nil' do
+          expect(pipeline_artifact).to be_nil
+        end
       end
     end
 
-    context 'when pipeline artifact does not have a coverage report' do
+    context 'when file_type is nil' do
+      let(:file_type) { nil }
+
       it 'returns nil' do
-        expect(subject).to be_nil
+        expect(pipeline_artifact).to be_nil
       end
     end
   end

spec/models/ci/pipeline_spec.rb

@@ -3489,6 +3489,54 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
     end
   end
 
+  describe '#has_codequality_reports?' do
+    subject { pipeline.has_codequality_reports? }
+
+    context 'when pipeline has a codequality artifact' do
+      let(:pipeline) { create(:ci_pipeline, :with_codequality_report_artifact, :running, project: project) }
+
+      it { expect(subject).to be_truthy }
+    end
+
+    context 'when pipeline does not have a codequality artifact' do
+      let(:pipeline) { create(:ci_pipeline, :success, project: project) }
+
+      it { expect(subject).to be_falsey }
+    end
+  end
+
+  describe '#can_generate_codequality_reports?' do
+    subject { pipeline.can_generate_codequality_reports? }
+
+    context 'when pipeline has builds with codequality reports' do
+      before do
+        create(:ci_build, :codequality_reports, pipeline: pipeline, project: project)
+      end
+
+      context 'when pipeline status is running' do
+        let(:pipeline) { create(:ci_pipeline, :running, project: project) }
+
+        it { expect(subject).to be_falsey }
+      end
+
+      context 'when pipeline status is success' do
+        let(:pipeline) { create(:ci_pipeline, :success, project: project) }
+
+        it { expect(subject).to be_truthy }
+      end
+    end
+
+    context 'when pipeline does not have builds with codequality reports' do
+      before do
+        create(:ci_build, :artifacts, pipeline: pipeline, project: project)
+      end
+
+      let(:pipeline) { create(:ci_pipeline, :success, project: project) }
+
+      it { expect(subject).to be_falsey }
+    end
+  end
+
   describe '#test_report_summary' do
     subject { pipeline.test_report_summary }