Add latest changes from gitlab-org/gitlab@master

3 files changed, 68 insertions, 50 deletions

diff --git a/spec/frontend/diffs/components/diff_row_utils_spec.js b/spec/frontend/diffs/components/diff_row_utils_spec.js
index c001857fa49..f397c3dc012 100644
--- a/spec/frontend/diffs/components/diff_row_utils_spec.js
+++ b/spec/frontend/diffs/components/diff_row_utils_spec.js
@@ -128,10 +128,10 @@ describe('classNameMapCell', () => {
   it.each`
     line               | hll      | loggedIn | hovered  | expectation
     ${undefined}       | ${true}  | ${true}  | ${true}  | ${[]}
-    ${{ type: 'new' }} | ${false} | ${false} | ${false} | ${['new', { hll: false, 'is-over': false }]}
-    ${{ type: 'new' }} | ${true}  | ${true}  | ${false} | ${['new', { hll: true, 'is-over': false }]}
-    ${{ type: 'new' }} | ${true}  | ${false} | ${true}  | ${['new', { hll: true, 'is-over': false }]}
-    ${{ type: 'new' }} | ${true}  | ${true}  | ${true}  | ${['new', { hll: true, 'is-over': true }]}
+    ${{ type: 'new' }} | ${false} | ${false} | ${false} | ${['new', { hll: false, 'is-over': false, new_line: true, old_line: false }]}
+    ${{ type: 'new' }} | ${true}  | ${true}  | ${false} | ${['new', { hll: true, 'is-over': false, new_line: true, old_line: false }]}
+    ${{ type: 'new' }} | ${true}  | ${false} | ${true}  | ${['new', { hll: true, 'is-over': false, new_line: true, old_line: false }]}
+    ${{ type: 'new' }} | ${true}  | ${true}  | ${true}  | ${['new', { hll: true, 'is-over': true, new_line: true, old_line: false }]}
   `('should return $expectation', ({ line, hll, loggedIn, hovered, expectation }) => {
     const classes = utils.classNameMapCell(line, hll, loggedIn, hovered);
     expect(classes).toEqual(expectation);
diff --git a/spec/lib/gitlab/utils/usage_data_spec.rb b/spec/lib/gitlab/utils/usage_data_spec.rb
index 1396f664389..dfc381d0ef2 100644
--- a/spec/lib/gitlab/utils/usage_data_spec.rb
+++ b/spec/lib/gitlab/utils/usage_data_spec.rb
@@ -58,76 +58,69 @@ RSpec.describe Gitlab::Utils::UsageData do
       expect(described_class.estimate_batch_distinct_count(relation, 'column')).to eq(5)
     end
 
-    context 'quasi integration test for different counting parameters', quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/296169' } do
-      let_it_be(:user) { create(:user, email: 'email1@domain.com') }
-      let_it_be(:another_user) { create(:user, email: 'email2@domain.com') }
-
-      let(:model) { Issue }
-      let(:column) { :author_id }
-
-      context 'different distribution of relation records' do
-        [10, 100, 100_000].each do |spread|
-          context "records are spread within #{spread}" do
-            before do
-              ids = (1..spread).to_a.sample(10)
-              create_list(:issue, 10).each_with_index do |issue, i|
-                issue.id = ids[i]
-              end
-            end
-
-            it 'counts table' do
-              expect(described_class.estimate_batch_distinct_count(model)).to be_within(error_rate).percent_of(10)
-            end
-          end
-        end
-      end
+    context 'quasi integration test for different counting parameters' do
+      # HyperLogLog http://algo.inria.fr/flajolet/Publications/FlFuGaMe07.pdf algorithm
+      # used in estimate_batch_distinct_count produce probabilistic
+      # estimations of unique values present in dataset, because of that its results
+      # are always off by some small factor from real value. However for given
+      # dataset it provide consistent and deterministic result. In the following context
+      # analyzed sets consist of values:
+      # build_needs set: ['1', '2', '3', '4', '5']
+      # ci_build set ['a', 'b']
+      # with them, current implementation is expected to consistently report
+      # 5.217656147118495 and 2.0809220082170614 values
+      # This test suite is expected to assure, that HyperLogLog implementation
+      # behaves consistently between changes made to other parts of codebase.
+      # In case of fine tuning or changes to HyperLogLog algorithm implementation
+      # one should run in depth analysis of accuracy with supplementary rake tasks
+      # currently under implementation at https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51118
+      # and adjust used values in this context accordingly.
+      let_it_be(:build) { create(:ci_build, name: 'a') }
+      let_it_be(:another_build) { create(:ci_build, name: 'b') }
+
+      let(:model) { Ci::BuildNeed }
+      let(:column) { :name }
+      let(:build_needs_estimated_cardinality) { 5.217656147118495 }
+      let(:ci_builds_estimated_cardinality) { 2.0809220082170614 }
 
       context 'different counting parameters' do
         before_all do
-          create_list(:issue, 3, author: user)
-          create_list(:issue, 2, author: another_user)
-        end
-
-        it 'counts table' do
-          expect(described_class.estimate_batch_distinct_count(model)).to be_within(error_rate).percent_of(5)
-        end
-
-        it 'counts with column field' do
-          expect(described_class.estimate_batch_distinct_count(model, column)).to be_within(error_rate).percent_of(2)
+          1.upto(3) { |i| create(:ci_build_need, name: i, build: build) }
+          4.upto(5) { |i| create(:ci_build_need, name: i, build: another_build) }
         end
 
-        it 'counts with :id field' do
-          expect(described_class.estimate_batch_distinct_count(model, :id)).to be_within(error_rate).percent_of(5)
+        it 'counts with symbol passed in column argument' do
+          expect(described_class.estimate_batch_distinct_count(model, column)).to eq(build_needs_estimated_cardinality)
         end
 
-        it 'counts with "id" field' do
-          expect(described_class.estimate_batch_distinct_count(model, "id")).to be_within(error_rate).percent_of(5)
+        it 'counts with string passed in column argument' do
+          expect(described_class.estimate_batch_distinct_count(model, column.to_s)).to eq(build_needs_estimated_cardinality)
         end
 
-        it 'counts with table.column field' do
-          expect(described_class.estimate_batch_distinct_count(model, "#{model.table_name}.#{column}")).to be_within(error_rate).percent_of(2)
+        it 'counts with table.column passed in column argument' do
+          expect(described_class.estimate_batch_distinct_count(model, "#{model.table_name}.#{column}")).to eq(build_needs_estimated_cardinality)
         end
 
-        it 'counts with Arel column' do
-          expect(described_class.estimate_batch_distinct_count(model, model.arel_table[column])).to be_within(error_rate).percent_of(2)
+        it 'counts with Arel passed in column argument' do
+          expect(described_class.estimate_batch_distinct_count(model, model.arel_table[column])).to eq(build_needs_estimated_cardinality)
         end
 
         it 'counts over joined relations' do
-          expect(described_class.estimate_batch_distinct_count(model.joins(:author), "users.email")).to be_within(error_rate).percent_of(2)
+          expect(described_class.estimate_batch_distinct_count(model.joins(:build), "ci_builds.name")).to eq(ci_builds_estimated_cardinality)
         end
 
         it 'counts with :column field with batch_size of 50K' do
-          expect(described_class.estimate_batch_distinct_count(model, column, batch_size: 50_000)).to be_within(error_rate).percent_of(2)
+          expect(described_class.estimate_batch_distinct_count(model, column, batch_size: 50_000)).to eq(build_needs_estimated_cardinality)
         end
 
         it 'counts with different number of batches and aggregates total result' do
           stub_const('Gitlab::Database::PostgresHll::BatchDistinctCounter::MIN_REQUIRED_BATCH_SIZE', 0)
 
-          [1, 2, 4, 5, 6].each { |i| expect(described_class.estimate_batch_distinct_count(model, batch_size: i)).to be_within(error_rate).percent_of(5) }
+          [1, 2, 4, 5, 6].each { |i| expect(described_class.estimate_batch_distinct_count(model, column, batch_size: i)).to eq(build_needs_estimated_cardinality) }
         end
 
         it 'counts with a start and finish' do
-          expect(described_class.estimate_batch_distinct_count(model, column, start: model.minimum(:id), finish: model.maximum(:id))).to be_within(error_rate).percent_of(2)
+          expect(described_class.estimate_batch_distinct_count(model, column, start: model.minimum(:id), finish: model.maximum(:id))).to eq(build_needs_estimated_cardinality)
         end
       end
     end
diff --git a/spec/rubocop/cop/lint/last_keyword_argument_spec.rb b/spec/rubocop/cop/lint/last_keyword_argument_spec.rb
index 33f80f7b799..826c681a880 100644
--- a/spec/rubocop/cop/lint/last_keyword_argument_spec.rb
+++ b/spec/rubocop/cop/lint/last_keyword_argument_spec.rb
@@ -38,6 +38,8 @@ RSpec.describe RuboCop::Cop::Lint::LastKeywordArgument do
       - |
         DEPRECATION WARNING: /Users/tkuah/code/ee-gdk/gitlab/create_service.rb:1: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
         /Users/tkuah/code/ee-gdk/gitlab/user.rb:17: warning: The called method `call' is defined here
+      - |
+        DEPRECATION WARNING: /Users/tkuah/code/ee-gdk/gitlab/other_warning_type.rb:1: warning: Some other warning type
       YAML
     end
 
@@ -62,7 +64,7 @@ RSpec.describe RuboCop::Cop::Lint::LastKeywordArgument do
       allow(File).to receive(:read).and_return(create_spec_yaml, projects_spec_yaml)
     end
 
-    it 'registers an offense' do
+    it 'registers an offense for last keyword warning' do
       expect_offense(<<~SOURCE, 'create_service.rb')
         users.call(params)
                    ^^^^^^ Using the last argument as keyword parameters is deprecated
@@ -73,6 +75,12 @@ RSpec.describe RuboCop::Cop::Lint::LastKeywordArgument do
       SOURCE
     end
 
+    it 'does not register an offense for other warning types' do
+      expect_no_offenses(<<~SOURCE, 'other_warning_type.rb')
+        users.call(params)
+      SOURCE
+    end
+
     it 'registers an offense for the new method call' do
       expect_offense(<<~SOURCE, 'projects_spec.rb')
         Project.new(params)
@@ -95,6 +103,23 @@ RSpec.describe RuboCop::Cop::Lint::LastKeywordArgument do
       SOURCE
     end
 
+    it 'registers an offense on the last non-block argument' do
+      expect_offense(<<~SOURCE, 'create_service.rb')
+        users.call(id, params, &block)
+                       ^^^^^^ Using the last argument as keyword parameters is deprecated
+      SOURCE
+
+      expect_correction(<<~SOURCE)
+        users.call(id, **params, &block)
+      SOURCE
+    end
+
+    it 'does not register an offense if the only argument is a block argument' do
+      expect_no_offenses(<<~SOURCE, 'create_service.rb')
+        users.call(&block)
+      SOURCE
+    end
+
     it 'registers an offense and corrects by converting splat to double splat' do
       expect_offense(<<~SOURCE, 'create_service.rb')
         users.call(id, *params)