Commit 5de9508b authored by Stan Hu's avatar Stan Hu

Gracefully handle unexpected severities in code quality report

A number of projects generate code quality reports that have severity
levels with typos (e.g. `marjor`) or with capitalized letters
(e.g. `MAJOR`). This would cause an error in our code quality parser
and cause a 204 No Content response for the controller endpoint.

We should categorize the former as `unknown` and make the severity
name case-insensitive. The frontend already handles unknown values.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/351276

Changelog: fixed
parent 9df0ef28
......@@ -2,7 +2,9 @@
class CodequalityDegradationEntity < Grape::Entity
expose :description
expose :severity
expose :severity do |degradation|
degradation.dig(:severity)&.downcase
end
expose :file_path do |degradation|
degradation.dig(:location, :path)
......
......@@ -9218,7 +9218,7 @@ Represents a code quality degradation on the pipeline.
| <a id="codequalitydegradationfingerprint"></a>`fingerprint` | [`String!`](#string) | Unique fingerprint to identify the code quality degradation. For example, an MD5 hash. |
| <a id="codequalitydegradationline"></a>`line` | [`Int!`](#int) | Line on which the code quality degradation occurred. |
| <a id="codequalitydegradationpath"></a>`path` | [`String!`](#string) | Relative path to the file containing the code quality degradation. |
| <a id="codequalitydegradationseverity"></a>`severity` | [`CodeQualityDegradationSeverity!`](#codequalitydegradationseverity) | Status of the degradation (BLOCKER, CRITICAL, MAJOR, MINOR, INFO). |
| <a id="codequalitydegradationseverity"></a>`severity` | [`CodeQualityDegradationSeverity!`](#codequalitydegradationseverity) | Status of the degradation (BLOCKER, CRITICAL, MAJOR, MINOR, INFO, UNKNOWN). |
### `Commit`
......@@ -16573,6 +16573,7 @@ Values for sorting runners.
| <a id="codequalitydegradationseverityinfo"></a>`INFO` | Code Quality degradation has a status of info. |
| <a id="codequalitydegradationseveritymajor"></a>`MAJOR` | Code Quality degradation has a status of major. |
| <a id="codequalitydegradationseverityminor"></a>`MINOR` | Code Quality degradation has a status of minor. |
| <a id="codequalitydegradationseverityunknown"></a>`UNKNOWN` | Code Quality degradation has a status of unknown. |
### `CommitActionMode`
......@@ -6,7 +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 ... }
SEVERITY_PRIORITIES = %w(blocker critical major minor info unknown).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
......@@ -32,7 +32,8 @@ module Gitlab
def sort_degradations!
@degradations = @degradations.sort_by do |_fingerprint, degradation|
SEVERITY_PRIORITIES[degradation.dig(:severity)]
severity = degradation.dig(:severity)&.downcase
SEVERITY_PRIORITIES[severity] || SEVERITY_PRIORITIES['unknown']
end.to_h
end
......
......@@ -85,6 +85,9 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReports do
let(:info) { build(:codequality_degradation, :info) }
let(:major_2) { build(:codequality_degradation, :major) }
let(:critical) { build(:codequality_degradation, :critical) }
let(:uppercase_major) { build(:codequality_degradation, severity: 'MAJOR') }
let(:unknown) { build(:codequality_degradation, severity: 'unknown') }
let(:codequality_report) { described_class.new }
before do
......@@ -94,6 +97,7 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReports do
codequality_report.add_degradation(major_2)
codequality_report.add_degradation(info)
codequality_report.add_degradation(critical)
codequality_report.add_degradation(unknown)
codequality_report.sort_degradations!
end
......@@ -105,8 +109,30 @@ RSpec.describe Gitlab::Ci::Reports::CodequalityReports do
major,
major_2,
minor,
info
info,
unknown
])
end
context 'with non-existence and uppercase severities' do
let(:other_report) { described_class.new }
let(:non_existent) { build(:codequality_degradation, severity: 'non-existent') }
before do
other_report.add_degradation(blocker)
other_report.add_degradation(uppercase_major)
other_report.add_degradation(minor)
other_report.add_degradation(non_existent)
end
it 'sorts unknown last' do
expect(other_report.degradations.values).to eq([
blocker,
uppercase_major,
minor,
non_existent
])
end
end
end
end
......@@ -30,6 +30,21 @@ RSpec.describe CodequalityDegradationEntity do
expect(subject[:line]).to eq(10)
end
end
context 'when severity is capitalized' do
let(:codequality_degradation) { build(:codequality_degradation_3) }
before do
codequality_degradation[:severity] = 'MINOR'
end
it 'lowercases severity', :aggregate_failures do
expect(subject[:description]).to eq("Avoid parameter lists longer than 5 parameters. [12/5]")
expect(subject[:severity]).to eq("minor")
expect(subject[:file_path]).to eq("file_b.rb")
expect(subject[:line]).to eq(10)
end
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment