Merge branch 'leaky-constant-fix-37' into 'master'

Fix leaky constant in migration helpers and ignored cols spec

See merge request gitlab-org/gitlab!32170

.rubocop.yml

@@ -351,9 +351,6 @@ RSpec/LeakyConstantDeclaration:
     - 'spec/db/schema_spec.rb'
     - 'spec/lib/feature_spec.rb'
     - 'spec/lib/gitlab/config/entry/simplifiable_spec.rb'
-    - 'spec/lib/gitlab/database/migration_helpers_spec.rb'
-    - 'spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb'
-    - 'spec/lib/gitlab/database/with_lock_retries_spec.rb'
     - 'spec/lib/gitlab/git/diff_collection_spec.rb'
     - 'spec/lib/gitlab/import_export/import_test_coverage_spec.rb'
     - 'spec/lib/gitlab/import_export/project/relation_factory_spec.rb'

changelogs/unreleased/leaky-constant-fix-37.yml

+---
+title: Fix leaky constant issue in migration helpers, with lock retries and ignored cols spec
+merge_request: 32170
+author: Rajendra Kadam
+type: fixed

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -1539,13 +1539,18 @@ describe Gitlab::Database::MigrationHelpers do
   end
 
   describe '#create_or_update_plan_limit' do
-    class self::Plan < ActiveRecord::Base
+    before do
+      stub_const('Plan', Class.new(ActiveRecord::Base))
+      stub_const('PlanLimits', Class.new(ActiveRecord::Base))
+
+      Plan.class_eval do
         self.table_name = 'plans'
       end
 
-    class self::PlanLimits < ActiveRecord::Base
+      PlanLimits.class_eval do
         self.table_name = 'plan_limits'
       end
+    end
 
     it 'properly escapes names' do
       expect(model).to receive(:execute).with <<~SQL
@@ -1560,28 +1565,28 @@ describe Gitlab::Database::MigrationHelpers do
     context 'when plan does not exist' do
       it 'does not create any plan limits' do
         expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 10) }
-          .not_to change { self.class::PlanLimits.count }
+          .not_to change { PlanLimits.count }
       end
     end
 
     context 'when plan does exist' do
-      let!(:plan) { self.class::Plan.create!(name: 'plan_name') }
+      let!(:plan) { Plan.create!(name: 'plan_name') }
 
       context 'when limit does not exist' do
         it 'inserts a new plan limits' do
           expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 10) }
-            .to change { self.class::PlanLimits.count }.by(1)
+            .to change { PlanLimits.count }.by(1)
 
-          expect(self.class::PlanLimits.pluck(:project_hooks)).to contain_exactly(10)
+          expect(PlanLimits.pluck(:project_hooks)).to contain_exactly(10)
         end
       end
 
       context 'when limit does exist' do
-        let!(:plan_limit) { self.class::PlanLimits.create!(plan_id: plan.id) }
+        let!(:plan_limit) { PlanLimits.create!(plan_id: plan.id) }
 
         it 'updates an existing plan limits' do
           expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 999) }
-            .not_to change { self.class::PlanLimits.count }
+            .not_to change { PlanLimits.count }
 
           expect(plan_limit.reload.project_hooks).to eq(999)
         end
@@ -1605,7 +1610,10 @@ describe Gitlab::Database::MigrationHelpers do
   describe '#backfill_iids' do
     include MigrationsHelpers
 
-    class self::Issue < ActiveRecord::Base
+    before do
+      stub_const('Issue', Class.new(ActiveRecord::Base))
+
+      Issue.class_eval do
         include AtomicInternalId
 
         self.table_name = 'issues'
@@ -1619,6 +1627,7 @@ describe Gitlab::Database::MigrationHelpers do
           backfill: true,
           presence: false
       end
+    end
 
     let(:namespaces)     { table(:namespaces) }
     let(:projects)       { table(:projects) }
@@ -1636,7 +1645,7 @@ describe Gitlab::Database::MigrationHelpers do
 
       model.backfill_iids('issues')
 
-      issue = self.class::Issue.create!(project_id: project.id)
+      issue = Issue.create!(project_id: project.id)
 
       expect(issue.iid).to eq(1)
     end
@@ -1647,7 +1656,7 @@ describe Gitlab::Database::MigrationHelpers do
 
       model.backfill_iids('issues')
 
-      issue_b = self.class::Issue.create!(project_id: project.id)
+      issue_b = Issue.create!(project_id: project.id)
 
       expect(issue_a.reload.iid).to eq(1)
       expect(issue_b.iid).to eq(2)
@@ -1662,8 +1671,8 @@ describe Gitlab::Database::MigrationHelpers do
 
       model.backfill_iids('issues')
 
-      issue_a = self.class::Issue.create!(project_id: project_a.id)
-      issue_b = self.class::Issue.create!(project_id: project_b.id)
+      issue_a = Issue.create!(project_id: project_a.id)
+      issue_b = Issue.create!(project_id: project_b.id)
 
       expect(issue_a.iid).to eq(2)
       expect(issue_b.iid).to eq(3)
@@ -1672,7 +1681,7 @@ describe Gitlab::Database::MigrationHelpers do
     context 'when the new code creates a row post deploy but before the migration runs' do
       it 'does not change the row iid' do
         project = setup
-        issue = self.class::Issue.create!(project_id: project.id)
+        issue = Issue.create!(project_id: project.id)
 
         model.backfill_iids('issues')
 
@@ -1683,7 +1692,7 @@ describe Gitlab::Database::MigrationHelpers do
         project = setup
         issue_a = issues.create!(project_id: project.id)
         issue_b = issues.create!(project_id: project.id)
-        issue_c = self.class::Issue.create!(project_id: project.id)
+        issue_c = Issue.create!(project_id: project.id)
 
         model.backfill_iids('issues')
 
@@ -1697,8 +1706,8 @@ describe Gitlab::Database::MigrationHelpers do
         project_b = setup
         issue_a = issues.create!(project_id: project_a.id)
         issue_b = issues.create!(project_id: project_b.id)
-        issue_c = self.class::Issue.create!(project_id: project_a.id)
-        issue_d = self.class::Issue.create!(project_id: project_b.id)
+        issue_c = Issue.create!(project_id: project_a.id)
+        issue_d = Issue.create!(project_id: project_b.id)
 
         model.backfill_iids('issues')
 
@@ -1712,12 +1721,12 @@ describe Gitlab::Database::MigrationHelpers do
         project = setup
         issue_a = issues.create!(project_id: project.id)
         issue_b = issues.create!(project_id: project.id)
-        issue_c = self.class::Issue.create!(project_id: project.id)
+        issue_c = Issue.create!(project_id: project.id)
 
         model.backfill_iids('issues')
 
-        issue_d = self.class::Issue.create!(project_id: project.id)
-        issue_e = self.class::Issue.create!(project_id: project.id)
+        issue_d = Issue.create!(project_id: project.id)
+        issue_e = Issue.create!(project_id: project.id)
 
         expect(issue_a.reload.iid).to eq(1)
         expect(issue_b.reload.iid).to eq(2)
@@ -1731,14 +1740,14 @@ describe Gitlab::Database::MigrationHelpers do
         project_b = setup
         issue_a = issues.create!(project_id: project_a.id)
         issue_b = issues.create!(project_id: project_b.id)
-        issue_c = self.class::Issue.create!(project_id: project_a.id)
-        issue_d = self.class::Issue.create!(project_id: project_b.id)
+        issue_c = Issue.create!(project_id: project_a.id)
+        issue_d = Issue.create!(project_id: project_b.id)
 
         model.backfill_iids('issues')
 
-        issue_e = self.class::Issue.create!(project_id: project_a.id)
-        issue_f = self.class::Issue.create!(project_id: project_b.id)
-        issue_g = self.class::Issue.create!(project_id: project_a.id)
+        issue_e = Issue.create!(project_id: project_a.id)
+        issue_f = Issue.create!(project_id: project_b.id)
+        issue_g = Issue.create!(project_id: project_a.id)
 
         expect(issue_a.reload.iid).to eq(1)
         expect(issue_b.reload.iid).to eq(1)
@@ -1754,7 +1763,7 @@ describe Gitlab::Database::MigrationHelpers do
       it 'backfills iids' do
         project = setup
         issue_a = issues.create!(project_id: project.id)
-        issue_b = self.class::Issue.create!(project_id: project.id)
+        issue_b = Issue.create!(project_id: project.id)
         issue_c = issues.create!(project_id: project.id)
 
         model.backfill_iids('issues')
@@ -1768,12 +1777,12 @@ describe Gitlab::Database::MigrationHelpers do
         project = setup
         issue_a = issues.create!(project_id: project.id)
         issue_b = issues.create!(project_id: project.id)
-        issue_c = self.class::Issue.create!(project_id: project.id)
+        issue_c = Issue.create!(project_id: project.id)
         issue_d = issues.create!(project_id: project.id)
 
         model.backfill_iids('issues')
 
-        issue_e = self.class::Issue.create!(project_id: project.id)
+        issue_e = Issue.create!(project_id: project.id)
 
         expect(issue_a.reload.iid).to eq(1)
         expect(issue_b.reload.iid).to eq(2)
@@ -1787,9 +1796,9 @@ describe Gitlab::Database::MigrationHelpers do
       it 'backfills iids' do
         project = setup
         issue_a = issues.create!(project_id: project.id)
-        issue_b = self.class::Issue.create!(project_id: project.id)
+        issue_b = Issue.create!(project_id: project.id)
         issue_c = issues.create!(project_id: project.id)
-        issue_d = self.class::Issue.create!(project_id: project.id)
+        issue_d = Issue.create!(project_id: project.id)
 
         model.backfill_iids('issues')
 
@@ -1803,13 +1812,13 @@ describe Gitlab::Database::MigrationHelpers do
         project = setup
         issue_a = issues.create!(project_id: project.id)
         issue_b = issues.create!(project_id: project.id)
-        issue_c = self.class::Issue.create!(project_id: project.id)
+        issue_c = Issue.create!(project_id: project.id)
         issue_d = issues.create!(project_id: project.id)
-        issue_e = self.class::Issue.create!(project_id: project.id)
+        issue_e = Issue.create!(project_id: project.id)
 
         model.backfill_iids('issues')
 
-        issue_f = self.class::Issue.create!(project_id: project.id)
+        issue_f = Issue.create!(project_id: project.id)
 
         expect(issue_a.reload.iid).to eq(1)
         expect(issue_b.reload.iid).to eq(2)
@@ -1825,7 +1834,7 @@ describe Gitlab::Database::MigrationHelpers do
         project = setup
         issue_a = issues.create!(project_id: project.id)
         issue_b = issues.create!(project_id: project.id)
-        issue_c = self.class::Issue.create!(project_id: project.id)
+        issue_c = Issue.create!(project_id: project.id)
         issue_c.delete
 
         model.backfill_iids('issues')
@@ -1838,12 +1847,12 @@ describe Gitlab::Database::MigrationHelpers do
         project = setup
         issue_a = issues.create!(project_id: project.id)
         issue_b = issues.create!(project_id: project.id)
-        issue_c = self.class::Issue.create!(project_id: project.id)
+        issue_c = Issue.create!(project_id: project.id)
         issue_c.delete
 
         model.backfill_iids('issues')
 
-        issue_d = self.class::Issue.create!(project_id: project.id)
+        issue_d = Issue.create!(project_id: project.id)
 
         expect(issue_a.reload.iid).to eq(1)
         expect(issue_b.reload.iid).to eq(2)
@@ -1856,7 +1865,7 @@ describe Gitlab::Database::MigrationHelpers do
         project = setup
         issue_a = issues.create!(project_id: project.id)
         issue_b = issues.create!(project_id: project.id)
-        issue_c = self.class::Issue.create!(project_id: project.id)
+        issue_c = Issue.create!(project_id: project.id)
         issue_c.delete
         issue_d = issues.create!(project_id: project.id)
 
@@ -1871,13 +1880,13 @@ describe Gitlab::Database::MigrationHelpers do
         project = setup
         issue_a = issues.create!(project_id: project.id)
         issue_b = issues.create!(project_id: project.id)
-        issue_c = self.class::Issue.create!(project_id: project.id)
+        issue_c = Issue.create!(project_id: project.id)
         issue_c.delete
         issue_d = issues.create!(project_id: project.id)
 
         model.backfill_iids('issues')
 
-        issue_e = self.class::Issue.create!(project_id: project.id)
+        issue_e = Issue.create!(project_id: project.id)
 
         expect(issue_a.reload.iid).to eq(1)
         expect(issue_b.reload.iid).to eq(2)
@@ -1891,9 +1900,9 @@ describe Gitlab::Database::MigrationHelpers do
         project = setup
         issue_a = issues.create!(project_id: project.id)
         issue_b = issues.create!(project_id: project.id)
-        issue_c = self.class::Issue.create!(project_id: project.id)
+        issue_c = Issue.create!(project_id: project.id)
         issue_c.delete
-        issue_d = self.class::Issue.create!(project_id: project.id)
+        issue_d = Issue.create!(project_id: project.id)
 
         model.backfill_iids('issues')
 
@@ -1906,13 +1915,13 @@ describe Gitlab::Database::MigrationHelpers do
         project = setup
         issue_a = issues.create!(project_id: project.id)
         issue_b = issues.create!(project_id: project.id)
-        issue_c = self.class::Issue.create!(project_id: project.id)
+        issue_c = Issue.create!(project_id: project.id)
         issue_c.delete
-        issue_d = self.class::Issue.create!(project_id: project.id)
+        issue_d = Issue.create!(project_id: project.id)
 
         model.backfill_iids('issues')
 
-        issue_e = self.class::Issue.create!(project_id: project.id)
+        issue_e = Issue.create!(project_id: project.id)
 
         expect(issue_a.reload.iid).to eq(1)
         expect(issue_b.reload.iid).to eq(2)
@@ -1929,7 +1938,7 @@ describe Gitlab::Database::MigrationHelpers do
 
         model.backfill_iids('issues')
 
-        issue_b = self.class::Issue.create!(project_id: project_b.id)
+        issue_b = Issue.create!(project_id: project_b.id)
 
         expect(issue_a.reload.iid).to eq(1)
         expect(issue_b.reload.iid).to eq(1)

spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb

@@ -3,14 +3,22 @@
 require 'spec_helper'
 
 describe Gitlab::Database::ObsoleteIgnoredColumns do
-  module Testing
+  before do
+    stub_const('Testing', Module.new)
+    stub_const('Testing::MyBase', Class.new(ActiveRecord::Base))
+    stub_const('SomeAbstract', Class.new(Testing::MyBase))
+    stub_const('Testing::B', Class.new(Testing::MyBase))
+    stub_const('Testing::A', Class.new(SomeAbstract))
+    stub_const('Testing::C', Class.new(Testing::MyBase))
+
     # Used a fixed date to prevent tests failing across date boundaries
-    REMOVE_DATE = Date.new(2019, 12, 16)
+    stub_const('REMOVE_DATE', Date.new(2019, 12, 16))
 
-    class MyBase < ApplicationRecord
+    Testing.module_eval do
+      Testing::MyBase.class_eval do
       end
 
-    class SomeAbstract < MyBase
+      SomeAbstract.class_eval do
         include IgnorableColumns
 
         self.abstract_class = true
@@ -20,7 +28,7 @@ describe Gitlab::Database::ObsoleteIgnoredColumns do
         ignore_column :unused, remove_after: '2019-01-01', remove_with: '12.0'
       end
 
-    class B < MyBase
+      Testing::B.class_eval do
         include IgnorableColumns
 
         self.table_name = 'issues'
@@ -29,21 +37,22 @@ describe Gitlab::Database::ObsoleteIgnoredColumns do
         ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1'
       end
 
-    class A < SomeAbstract
+      Testing::A.class_eval do
         ignore_column :also_unused, remove_after: '2019-02-01', remove_with: '12.1'
         ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1'
       end
 
-    class C < MyBase
+      Testing::C.class_eval do
         self.table_name = 'users'
       end
     end
+  end
 
   subject { described_class.new(Testing::MyBase) }
 
   describe '#execute' do
     it 'returns a list of class names and columns pairs' do
-      Timecop.freeze(Testing::REMOVE_DATE) do
+      Timecop.freeze(REMOVE_DATE) do
         expect(subject.execute).to eq([
           ['Testing::A', {
             'unused' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-01-01'), '12.0'),

spec/lib/gitlab/database/with_lock_retries_spec.rb

@@ -35,9 +35,6 @@ describe Gitlab::Database::WithLockRetries do
     end
 
     context 'when lock retry is enabled' do
-      class ActiveRecordSecond < ActiveRecord::Base
-      end
-
       let(:lock_fiber) do
         Fiber.new do
           # Initiating a second DB connection for the lock
@@ -52,6 +49,8 @@ describe Gitlab::Database::WithLockRetries do
       end
 
       before do
+        stub_const('ActiveRecordSecond', Class.new(ActiveRecord::Base))
+
         lock_fiber.resume # start the transaction and lock the table
       end