Skip to content

G
gitlab-ce
Commit 19f942a9 authored by Rajendra Kadam's avatar Rajendra Kadam Committed by Peter Leitzen
Browse files
Options

Fix leaky constant in migration helpers and ignored cols spec

parent 11271ac4
Hide whitespace changes
Inline Side-by-side
Showing with 105 additions and 86 deletions
.rubocop.yml
View file @ 19f942a9
......@@ -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 0 → 100644
View file @ 19f942a9
---
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
View file @ 19f942a9
......@@ -1539,12 +1539,17 @@ describe Gitlab::Database::MigrationHelpers do
end
describe '#create_or_update_plan_limit' do
class self::Plan < ActiveRecord::Base
self.table_name = 'plans'
end
before do
stub_const('Plan', Class.new(ActiveRecord::Base))
stub_const('PlanLimits', Class.new(ActiveRecord::Base))
class self::PlanLimits < ActiveRecord::Base
self.table_name = 'plan_limits'
Plan.class_eval do
self.table_name = 'plans'
end
PlanLimits.class_eval do
self.table_name = 'plan_limits'
end
end
it 'properly escapes names' do
......@@ -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,19 +1610,23 @@ describe Gitlab::Database::MigrationHelpers do
describe '#backfill_iids' do
include MigrationsHelpers
class self::Issue < ActiveRecord::Base
include AtomicInternalId
before do
stub_const('Issue', Class.new(ActiveRecord::Base))
Issue.class_eval do
include AtomicInternalId
self.table_name = 'issues'
self.inheritance_column = :_type_disabled
self.table_name = 'issues'
self.inheritance_column = :_type_disabled
belongs_to :project, class_name: "::Project"
belongs_to :project, class_name: "::Project"
has_internal_id :iid,
scope: :project,
init: ->(s) { s&.project&.issues&.maximum(:iid) },
backfill: true,
presence: false
has_internal_id :iid,
scope: :project,
init: ->(s) { s&.project&.issues&.maximum(:iid) },
backfill: true,
presence: false
end
end
let(:namespaces) { table(:namespaces) }
......@@ -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
View file @ 19f942a9
......@@ -3,39 +3,48 @@
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
end
Testing.module_eval do
Testing::MyBase.class_eval do
end
class SomeAbstract < MyBase
include IgnorableColumns
SomeAbstract.class_eval do
include IgnorableColumns
self.abstract_class = true
self.abstract_class = true
self.table_name = 'projects'
self.table_name = 'projects'
ignore_column :unused, remove_after: '2019-01-01', remove_with: '12.0'
end
ignore_column :unused, remove_after: '2019-01-01', remove_with: '12.0'
end
class B < MyBase
include IgnorableColumns
Testing::B.class_eval do
include IgnorableColumns
self.table_name = 'issues'
self.table_name = 'issues'
ignore_column :id, :other, remove_after: '2019-01-01', remove_with: '12.0'
ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1'
end
ignore_column :id, :other, remove_after: '2019-01-01', remove_with: '12.0'
ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1'
end
class A < SomeAbstract
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
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
self.table_name = 'users'
Testing::C.class_eval do
self.table_name = 'users'
end
end
end
......@@ -43,7 +52,7 @@ describe Gitlab::Database::ObsoleteIgnoredColumns do
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
View file @ 19f942a9
......@@ -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
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7