Commit acb56a7f authored by Victor Zagorodny's avatar Victor Zagorodny

Make PipelineVulnerabilitiesFinder sort stable

Security::PipelineVulnerabilitiesFinder sort is
now stable contrary to the bare Ruby sort_by
method. Using just sort_by leads to instability
across different platforms (e.g., x86_64-linux and
x86_64-darwin18) which in turn leads to different
sorting results for the equal elements across
these platforms. This is important because it
breaks test suite results consistency between
local and CI environment.
parent 1d7bcbf3
......@@ -41,11 +41,27 @@ module Security
occurrences.concat(filtered_occurrences)
end
occurrences.sort_by { |x| [-x.severity_value, -x.confidence_value] }
sort_occurrences(occurrences)
end
private
def sort_occurrences(occurrences)
# This sort is stable (see https://en.wikipedia.org/wiki/Sorting_algorithm#Stability) contrary to the bare
# Ruby sort_by method. Using just sort_by leads to instability across different platforms (e.g., x86_64-linux and
# x86_64-darwin18) which in turn leads to different sorting results for the equal elements across these platforms.
# This is important because it breaks test suite results consistency between local and CI
# environment.
# This is easier to address from within the class rather than from tests because this leads to bad class design
# and exposing too much of its implementation details to the test suite.
# See also https://stackoverflow.com/questions/15442298/is-sort-in-ruby-stable.
stable_sort_by(occurrences) { |x| [-x.severity_value, -x.confidence_value] }
end
def stable_sort_by(occurrences)
occurrences.sort_by.with_index { |x, idx| [yield(x), idx] }
end
def pipeline_reports
pipeline&.security_reports&.reports
end
......
......@@ -275,6 +275,21 @@ describe Security::PipelineVulnerabilitiesFinder do
end
end
context 'when being tested for sort stability' do
let(:params) { { report_type: %w[sast] } }
it 'maintains the order of the occurrences having the same severity and confidence' do
select_proc = proc { |o| o.severity == 'medium' && o.confidence == 'high' }
report_occurrences = pipeline.security_reports.reports['sast'].occurrences.select(&select_proc)
found_occurrences = subject.select(&select_proc)
found_occurrences.each_with_index do |found, i|
expect(found.metadata['cve']).to eq(report_occurrences[i].compare_key)
end
end
end
def read_fixture(fixture)
JSON.parse(File.read(fixture.file.path))
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