Merge branch 'rp-use-stable-sort-in-sorter' into 'master'

Use stable sort in metrics dashboard sorter

See merge request gitlab-org/gitlab!36278

changelogs/unreleased/rp-use-stable-sort-in-sorter.yml

+---
+title: Sort metrics dashboard panels and groups using a stable sort
+merge_request: 36278
+author:
+type: fixed

ee/app/finders/security/pipeline_vulnerabilities_finder.rb

@@ -51,11 +51,7 @@ module Security
       # 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] }
+      Gitlab::Utils.stable_sort_by(occurrences) { |x| [-x.severity_value, -x.confidence_value] }
     end
 
     def pipeline_reports

lib/gitlab/metrics/dashboard/stages/sorter.rb

@@ -16,7 +16,7 @@ module Gitlab
 
           # Sorts the groups in the dashboard by the :priority key
           def sort_groups!
-            dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| -group[:priority].to_i }
+            dashboard[:panel_groups] = Gitlab::Utils.stable_sort_by(dashboard[:panel_groups]) { |group| -group[:priority].to_i }
           end
 
           # Sorts the panels in the dashboard by the :weight key
@@ -24,7 +24,7 @@ module Gitlab
             dashboard[:panel_groups].each do |group|
               missing_panels! unless group[:panels].is_a? Array
 
-              group[:panels] = group[:panels].sort_by { |panel| -panel[:weight].to_i }
+              group[:panels] = Gitlab::Utils.stable_sort_by(group[:panels]) { |panel| -panel[:weight].to_i }
             end
           end
         end

lib/gitlab/utils.rb

@@ -178,5 +178,15 @@ module Gitlab
         .group_by(&:first)
         .transform_values { |kvs| kvs.map(&:last) }
     end
+
+    # 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 method uses a list item's original index position to break ties.
+    def stable_sort_by(list)
+      list.sort_by.with_index { |x, idx| [yield(x), idx] }
+    end
   end
 end

spec/lib/gitlab/utils_spec.rb

@@ -358,4 +358,40 @@ RSpec.describe Gitlab::Utils do
       })
     end
   end
+
+  describe '.stable_sort_by' do
+    subject(:sorted_list) { described_class.stable_sort_by(list) { |obj| obj[:priority] } }
+
+    context 'when items have the same priority' do
+      let(:list) do
+        [
+          { name: 'obj 1', priority: 1 },
+          { name: 'obj 2', priority: 1 },
+          { name: 'obj 3', priority: 1 }
+        ]
+      end
+
+      it 'does not change order in cases of ties' do
+        expect(sorted_list).to eq(list)
+      end
+    end
+
+    context 'when items have different priorities' do
+      let(:list) do
+        [
+          { name: 'obj 1', priority: 2 },
+          { name: 'obj 2', priority: 1 },
+          { name: 'obj 3', priority: 3 }
+        ]
+      end
+
+      it 'sorts items like the regular sort_by' do
+        expect(sorted_list).to eq([
+          { name: 'obj 2', priority: 1 },
+          { name: 'obj 1', priority: 2 },
+          { name: 'obj 3', priority: 3 }
+        ])
+      end
+    end
+  end
 end