Commit 0f27bc5b authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'mk-fix-pg-undefined-table-ci-errors' into 'master'

Resolve "Build Is Red: Admin > Users > Impersonation Tokens token creation allows creation of a token"

Closes gitlab-ee#4914

See merge request gitlab-org/gitlab-ce!17102
parents 2e8d366d 080dba4a
......@@ -249,7 +249,7 @@ module Gitlab
end
def drop_temp_table_if_finished
if UntrackedFile.all.empty?
if UntrackedFile.all.empty? && !Rails.env.test? # Dropping a table intermittently breaks test cleanup
UntrackedFile.connection.drop_table(:untracked_files_for_uploads,
if_exists: true)
end
......
......@@ -171,8 +171,10 @@ module Gitlab
end
def drop_temp_table
UntrackedFile.connection.drop_table(:untracked_files_for_uploads,
if_exists: true)
unless Rails.env.test? # Dropping a table intermittently breaks test cleanup
UntrackedFile.connection.drop_table(:untracked_files_for_uploads,
if_exists: true)
end
end
end
end
......
......@@ -14,16 +14,10 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :migra
let!(:uploads) { described_class::Upload }
before do
DatabaseCleaner.clean
drop_temp_table_if_exists
ensure_temporary_tracking_table_exists
uploads.delete_all
end
after(:all) do
drop_temp_table_if_exists
end
context 'with untracked files and tracked files in untracked_files_for_uploads' do
let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) }
let!(:user1) { create(:user, :with_avatar) }
......@@ -122,9 +116,9 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :migra
end
it 'drops the temporary tracking table after processing the batch, if there are no untracked rows left' do
subject.perform(1, untracked_files_for_uploads.last.id)
expect(subject).to receive(:drop_temp_table_if_finished)
expect(ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey
subject.perform(1, untracked_files_for_uploads.last.id)
end
it 'does not block a whole batch because of one bad path' do
......@@ -260,10 +254,6 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do
ensure_temporary_tracking_table_exists
end
after(:all) do
drop_temp_table_if_exists
end
describe '#upload_path' do
def assert_upload_path(file_path, expected_upload_path)
untracked_file = create_untracked_file(file_path)
......
require 'spec_helper'
describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq, :migration, schema: 20180129193323 do
include TrackUntrackedUploadsHelpers
include MigrationsHelpers
let!(:untracked_files_for_uploads) { described_class::UntrackedFile }
before do
DatabaseCleaner.clean
end
after do
drop_temp_table_if_exists
end
around do |example|
# Especially important so the follow-up migration does not get run
Sidekiq::Testing.fake! do
......@@ -76,7 +68,8 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
it 'correctly schedules the follow-up background migration jobs' do
described_class.new.perform
expect(described_class::FOLLOW_UP_MIGRATION).to be_scheduled_migration(1, 5)
ids = described_class::UntrackedFile.all.order(:id).pluck(:id)
expect(described_class::FOLLOW_UP_MIGRATION).to be_scheduled_migration(ids.first, ids.last)
expect(BackgroundMigrationWorker.jobs.size).to eq(1)
end
......@@ -150,9 +143,11 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
# may not have an upload directory because they have no uploads.
context 'when no files were ever uploaded' do
it 'deletes the `untracked_files_for_uploads` table (and does not raise error)' do
described_class.new.perform
background_migration = described_class.new
expect(background_migration).to receive(:drop_temp_table)
expect(untracked_files_for_uploads.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey
background_migration.perform
end
end
end
......@@ -89,5 +89,5 @@ end
## Best practices
1. Note that this type of tests do not run within the transaction, we use
a truncation database cleanup strategy. Do not depend on transaction being
a deletion database cleanup strategy. Do not depend on transaction being
present.
......@@ -8,10 +8,6 @@ module TrackUntrackedUploadsHelpers
Gitlab::BackgroundMigration::PrepareUntrackedUploads.new.send(:ensure_temporary_tracking_table_exists)
end
def drop_temp_table_if_exists
ActiveRecord::Base.connection.drop_table(:untracked_files_for_uploads) if ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)
end
def create_or_update_appearance(attrs)
a = Appearance.first_or_initialize(title: 'foo', description: 'bar')
a.update!(attrs)
......
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