Merge branch 'eb-sort-code-quality-mr-widget' into 'master'

Sort code quality degradations in comparison reports

See merge request gitlab-org/gitlab!57258

changelogs/unreleased/eb-sort-code-quality-mr-widget.yml

+---
+title: Sort code quality degradations in MR Widget comparison reports
+merge_request: 57258
+author:
+type: added

lib/gitlab/ci/reports/codequality_reports.rb

@@ -6,6 +6,7 @@ module Gitlab
       class CodequalityReports
         attr_reader :degradations, :error_message
 
+        SEVERITY_PRIORITIES = %w(blocker critical major minor info).map.with_index.to_h.freeze # { "blocker" => 0, "critical" => 1 ... }
         CODECLIMATE_SCHEMA_PATH = Rails.root.join('app', 'validators', 'json_schemas', 'codeclimate.json').to_s
 
         def initialize
@@ -29,6 +30,12 @@ module Gitlab
           @degradations.values
         end
 
+        def sort_degradations!
+          @degradations = @degradations.sort_by do |_fingerprint, degradation|
+            SEVERITY_PRIORITIES[degradation.dig(:severity)]
+          end.to_h
+        end
+
         private
 
         def valid_degradation?(degradation)

lib/gitlab/ci/reports/codequality_reports_comparer.rb

@@ -7,6 +7,11 @@ module Gitlab
         def initialize(base_report, head_report)
           @base_report = base_report
           @head_report = head_report
+
+          unless not_found?
+            @base_report.sort_degradations!
+            @head_report.sort_degradations!
+          end
         end
 
         def success?

spec/factories/ci/reports/codequality_degradations.rb

@@ -95,4 +95,47 @@ FactoryBot.define do
       }.with_indifferent_access
     end
   end
+
+  # TODO: Use this in all other specs and remove the previous numbered factories
+  # https://gitlab.com/gitlab-org/gitlab/-/issues/325886
+  factory :codequality_degradation, class: Hash do
+    skip_create
+
+    # Feel free to add in more configurable properties here
+    # as the need arises
+    fingerprint { SecureRandom.hex }
+    severity { "major" }
+
+    Gitlab::Ci::Reports::CodequalityReports::SEVERITY_PRIORITIES.keys.each do |s|
+      trait s.to_sym do
+        severity { s }
+      end
+    end
+
+    initialize_with do
+      {
+        "categories": [
+          "Complexity"
+        ],
+        "check_name": "argument_count",
+        "content": {
+          "body": ""
+        },
+        "description": "Avoid parameter lists longer than 5 parameters. [12/5]",
+        "fingerprint": fingerprint,
+        "location": {
+          "path": "file_a.rb",
+          "lines": {
+            "begin": 10,
+            "end": 10
+          }
+        },
+        "other_locations": [],
+        "remediation_points": 900000,
+        "severity": severity,
+        "type": "issue",
+        "engine_name": "structure"
+      }.with_indifferent_access
+    end
+  end
 end

spec/lib/gitlab/ci/reports/codequality_reports_comparer_spec.rb

@@ -6,15 +6,17 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
   let(:comparer) { described_class.new(base_report, head_report) }
   let(:base_report) { Gitlab::Ci::Reports::CodequalityReports.new }
   let(:head_report) { Gitlab::Ci::Reports::CodequalityReports.new }
-  let(:degradation_1) { build(:codequality_degradation_1) }
-  let(:degradation_2) { build(:codequality_degradation_2) }
+  let(:major_degradation) { build(:codequality_degradation, :major) }
+  let(:minor_degradation) { build(:codequality_degradation, :major) }
+  let(:critical_degradation) { build(:codequality_degradation, :critical) }
+  let(:blocker_degradation) { build(:codequality_degradation, :blocker) }
 
   describe '#status' do
     subject(:report_status) { comparer.status }
 
     context 'when head report has an error' do
       before do
-        head_report.add_degradation(degradation_1)
+        head_report.add_degradation(major_degradation)
       end
 
       it 'returns status failed' do
@@ -50,7 +52,7 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when head report has an error' do
       before do
-        head_report.add_degradation(degradation_1)
+        head_report.add_degradation(major_degradation)
       end
 
       it 'returns the number of new errors' do
@@ -70,8 +72,8 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report has an error and head has a different error' do
       before do
-        base_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_2)
+        base_report.add_degradation(major_degradation)
+        head_report.add_degradation(minor_degradation)
       end
 
       it 'counts the base report error as resolved' do
@@ -81,7 +83,7 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report has errors head has no errors' do
       before do
-        base_report.add_degradation(degradation_1)
+        base_report.add_degradation(major_degradation)
       end
 
       it 'counts the base report errors as resolved' do
@@ -91,8 +93,8 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report has errors and head has the same error' do
       before do
-        base_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_1)
+        base_report.add_degradation(major_degradation)
+        head_report.add_degradation(major_degradation)
       end
 
       it 'returns zero' do
@@ -102,7 +104,7 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report does not have errors and head has errors' do
       before do
-        head_report.add_degradation(degradation_1)
+        head_report.add_degradation(major_degradation)
       end
 
       it 'returns zero' do
@@ -124,7 +126,7 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report has an error' do
       before do
-        base_report.add_degradation(degradation_1)
+        base_report.add_degradation(major_degradation)
       end
 
       it 'returns zero' do
@@ -134,7 +136,7 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when head report has an error' do
       before do
-        head_report.add_degradation(degradation_1)
+        head_report.add_degradation(major_degradation)
       end
 
       it 'includes the head report error in the count' do
@@ -144,8 +146,8 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report has errors and head report has errors' do
       before do
-        base_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_2)
+        base_report.add_degradation(major_degradation)
+        head_report.add_degradation(minor_degradation)
       end
 
       it 'includes errors in the count' do
@@ -155,9 +157,9 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report has errors and head report has the same error' do
       before do
-        base_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_2)
+        base_report.add_degradation(major_degradation)
+        head_report.add_degradation(major_degradation)
+        head_report.add_degradation(minor_degradation)
       end
 
       it 'includes errors in the count' do
@@ -179,20 +181,28 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report has errors and head has the same error' do
       before do
-        base_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_2)
-      end
-
-      it 'includes the base report errors' do
-        expect(existing_errors).to contain_exactly(degradation_1)
+        base_report.add_degradation(major_degradation)
+        base_report.add_degradation(critical_degradation)
+        base_report.add_degradation(blocker_degradation)
+        head_report.add_degradation(critical_degradation)
+        head_report.add_degradation(blocker_degradation)
+        head_report.add_degradation(major_degradation)
+        head_report.add_degradation(minor_degradation)
+      end
+
+      it 'includes the base report errors sorted by severity' do
+        expect(existing_errors).to eq([
+          blocker_degradation,
+          critical_degradation,
+          major_degradation
+        ])
       end
     end
 
     context 'when base report has errors and head has a different error' do
       before do
-        base_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_2)
+        base_report.add_degradation(major_degradation)
+        head_report.add_degradation(minor_degradation)
       end
 
       it 'returns an empty array' do
@@ -202,7 +212,7 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report does not have errors and head has errors' do
       before do
-        head_report.add_degradation(degradation_1)
+        head_report.add_degradation(major_degradation)
       end
 
       it 'returns an empty array' do
@@ -224,19 +234,25 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report has errors and head has more errors' do
       before do
-        base_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_2)
+        base_report.add_degradation(major_degradation)
+        head_report.add_degradation(critical_degradation)
+        head_report.add_degradation(minor_degradation)
+        head_report.add_degradation(blocker_degradation)
+        head_report.add_degradation(major_degradation)
       end
 
-      it 'includes errors not found in the base report' do
-        expect(new_errors).to eq([degradation_2])
+      it 'includes errors not found in the base report sorted by severity' do
+        expect(new_errors).to eq([
+          blocker_degradation,
+          critical_degradation,
+          minor_degradation
+        ])
       end
     end
 
     context 'when base report has an error and head has no errors' do
       before do
-        base_report.add_degradation(degradation_1)
+        base_report.add_degradation(major_degradation)
       end
 
       it 'returns an empty array' do
@@ -246,11 +262,11 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report does not have errors and head has errors' do
       before do
-        head_report.add_degradation(degradation_1)
+        head_report.add_degradation(major_degradation)
       end
 
       it 'returns the head report error' do
-        expect(new_errors).to eq([degradation_1])
+        expect(new_errors).to eq([major_degradation])
       end
     end
 
@@ -268,9 +284,9 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report errors are still found in the head report' do
       before do
-        base_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_2)
+        base_report.add_degradation(major_degradation)
+        head_report.add_degradation(major_degradation)
+        head_report.add_degradation(minor_degradation)
       end
 
       it 'returns an empty array' do
@@ -280,18 +296,25 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReportsComparer do
 
     context 'when base report has errors and head has a different error' do
       before do
-        base_report.add_degradation(degradation_1)
-        head_report.add_degradation(degradation_2)
+        base_report.add_degradation(major_degradation)
+        base_report.add_degradation(minor_degradation)
+        base_report.add_degradation(critical_degradation)
+        base_report.add_degradation(blocker_degradation)
+        head_report.add_degradation(major_degradation)
       end
 
-      it 'returns the base report error' do
-        expect(resolved_errors).to eq([degradation_1])
+      it 'returns the base report errors not found in the head report, sorted by severity' do
+        expect(resolved_errors).to eq([
+          blocker_degradation,
+          critical_degradation,
+          minor_degradation
+        ])
       end
     end
 
     context 'when base report does not have errors and head has errors' do
       before do
-        head_report.add_degradation(degradation_1)
+        head_report.add_degradation(major_degradation)
       end
 
       it 'returns an empty array' do

spec/lib/gitlab/ci/reports/codequality_reports_spec.rb

@@ -77,4 +77,36 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReports do
       end
     end
   end
+
+  describe '#sort_degradations!' do
+    let(:major) { build(:codequality_degradation, :major) }
+    let(:minor) { build(:codequality_degradation, :minor) }
+    let(:blocker) { build(:codequality_degradation, :blocker) }
+    let(:info) { build(:codequality_degradation, :info) }
+    let(:major_2) { build(:codequality_degradation, :major) }
+    let(:critical) { build(:codequality_degradation, :critical) }
+    let(:codequality_report) { described_class.new }
+
+    before do
+      codequality_report.add_degradation(major)
+      codequality_report.add_degradation(minor)
+      codequality_report.add_degradation(blocker)
+      codequality_report.add_degradation(major_2)
+      codequality_report.add_degradation(info)
+      codequality_report.add_degradation(critical)
+
+      codequality_report.sort_degradations!
+    end
+
+    it 'sorts degradations based on severity' do
+      expect(codequality_report.degradations.values).to eq([
+        blocker,
+        critical,
+        major,
+        major_2,
+        minor,
+        info
+      ])
+    end
+  end
 end