Fix specs after rebase

Later migrations added fields to the EE DB which were used by factories which were used in these specs.

And in CE on MySQL, a single appearance row is enforced.

The migration and migration specs should not depend on the codebase staying the same.
This commit is contained in:
Michael Kozono 2017-12-05 23:08:45 -08:00
parent 869d08b581
commit 03cba8c0f1
4 changed files with 20 additions and 89 deletions

View File

@ -1,13 +1,12 @@
require 'spec_helper' require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20171103140253_track_untracked_uploads')
describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sidekiq, schema: 20171103140253 do describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq do
include TrackUntrackedUploadsHelpers include TrackUntrackedUploadsHelpers
subject { described_class.new } subject { described_class.new }
let!(:untracked_files_for_uploads) { table(:untracked_files_for_uploads) } let!(:untracked_files_for_uploads) { described_class::UntrackedFile }
let!(:uploads) { table(:uploads) } let!(:uploads) { described_class::Upload }
before do before do
ensure_temporary_tracking_table_exists ensure_temporary_tracking_table_exists
@ -18,7 +17,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid
end end
context 'with untracked files and tracked files in untracked_files_for_uploads' do context 'with untracked files and tracked files in untracked_files_for_uploads' do
let!(:appearance) { create(:appearance, logo: uploaded_file, header_logo: uploaded_file) } let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) }
let!(:user1) { create(:user, :with_avatar) } let!(:user1) { create(:user, :with_avatar) }
let!(:user2) { create(:user, :with_avatar) } let!(:user2) { create(:user, :with_avatar) }
let!(:project1) { create(:project, :with_avatar) } let!(:project1) { create(:project, :with_avatar) }
@ -111,13 +110,13 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid
it 'does not drop the temporary tracking table after processing the batch, if there are still untracked rows' do it 'does not drop the temporary tracking table after processing the batch, if there are still untracked rows' do
subject.perform(1, untracked_files_for_uploads.last.id - 1) subject.perform(1, untracked_files_for_uploads.last.id - 1)
expect(table_exists?(:untracked_files_for_uploads)).to be_truthy expect(ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)).to be_truthy
end end
it 'drops the temporary tracking table after processing the batch, if there are no untracked rows left' do 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) subject.perform(1, untracked_files_for_uploads.last.id)
expect(table_exists?(:untracked_files_for_uploads)).to be_falsey expect(ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey
end end
it 'does not block a whole batch because of one bad path' do it 'does not block a whole batch because of one bad path' do
@ -168,13 +167,13 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid
end end
context 'for an appearance logo file path' do context 'for an appearance logo file path' do
let(:model) { create(:appearance, logo: uploaded_file) } let(:model) { create_or_update_appearance(logo: uploaded_file) }
it_behaves_like 'non_markdown_file' it_behaves_like 'non_markdown_file'
end end
context 'for an appearance header_logo file path' do context 'for an appearance header_logo file path' do
let(:model) { create(:appearance, header_logo: uploaded_file) } let(:model) { create_or_update_appearance(header_logo: uploaded_file) }
it_behaves_like 'non_markdown_file' it_behaves_like 'non_markdown_file'
end end
@ -459,7 +458,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do
describe '#file_size' do describe '#file_size' do
context 'for an appearance logo file path' do context 'for an appearance logo file path' do
let(:appearance) { create(:appearance, logo: uploaded_file) } let(:appearance) { create_or_update_appearance(logo: uploaded_file) }
let(:untracked_file) { described_class.create!(path: appearance.uploads.first.path) } let(:untracked_file) { described_class.create!(path: appearance.uploads.first.path) }
it 'returns the file size' do it 'returns the file size' do

View File

@ -1,9 +1,9 @@
require 'spec_helper' require 'spec_helper'
describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :sidekiq, schema: 20171103140253 do describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
include TrackUntrackedUploadsHelpers include TrackUntrackedUploadsHelpers
let!(:untracked_files_for_uploads) { table(:untracked_files_for_uploads) } let!(:untracked_files_for_uploads) { described_class::UntrackedFile }
matcher :be_scheduled_migration do |*expected| matcher :be_scheduled_migration do |*expected|
match do |migration| match do |migration|
@ -35,7 +35,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side
it 'ensures the untracked_files_for_uploads table exists' do it 'ensures the untracked_files_for_uploads table exists' do
expect do expect do
described_class.new.perform described_class.new.perform
end.to change { table_exists?(:untracked_files_for_uploads) }.from(false).to(true) end.to change { ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads) }.from(false).to(true)
end end
it 'has a path field long enough for really long paths' do it 'has a path field long enough for really long paths' do
@ -63,7 +63,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side
end end
context 'when files were uploaded before and after hashed storage was enabled' do context 'when files were uploaded before and after hashed storage was enabled' do
let!(:appearance) { create(:appearance, logo: uploaded_file, header_logo: uploaded_file) } let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) }
let!(:user) { create(:user, :with_avatar) } let!(:user) { create(:user, :with_avatar) }
let!(:project1) { create(:project, :with_avatar) } let!(:project1) { create(:project, :with_avatar) }
let(:project2) { create(:project) } # instantiate after enabling hashed_storage let(:project2) { create(:project) } # instantiate after enabling hashed_storage
@ -151,7 +151,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side
end end
context 'when files were uploaded before and after hashed storage was enabled' do context 'when files were uploaded before and after hashed storage was enabled' do
let!(:appearance) { create(:appearance, logo: uploaded_file, header_logo: uploaded_file) } let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) }
let!(:user) { create(:user, :with_avatar) } let!(:user) { create(:user, :with_avatar) }
let!(:project1) { create(:project, :with_avatar) } let!(:project1) { create(:project, :with_avatar) }
let(:project2) { create(:project) } # instantiate after enabling hashed_storage let(:project2) { create(:project) } # instantiate after enabling hashed_storage

View File

@ -4,9 +4,6 @@ require Rails.root.join('db', 'post_migrate', '20171103140253_track_untracked_up
describe TrackUntrackedUploads, :migration, :sidekiq do describe TrackUntrackedUploads, :migration, :sidekiq do
include TrackUntrackedUploadsHelpers include TrackUntrackedUploadsHelpers
let(:untracked_files_for_uploads) { table(:untracked_files_for_uploads) }
let(:uploads) { table(:uploads) }
matcher :be_scheduled_migration do matcher :be_scheduled_migration do
match do |migration| match do |migration|
BackgroundMigrationWorker.jobs.any? do |job| BackgroundMigrationWorker.jobs.any? do |job|
@ -27,75 +24,4 @@ describe TrackUntrackedUploads, :migration, :sidekiq do
expect(BackgroundMigrationWorker.jobs.size).to eq(1) expect(BackgroundMigrationWorker.jobs.size).to eq(1)
end end
end end
context 'with tracked and untracked uploads' do
let!(:appearance) { create(:appearance, logo: uploaded_file, header_logo: uploaded_file) }
let!(:user1) { create(:user, :with_avatar) }
let!(:user2) { create(:user, :with_avatar) }
let!(:project1) { create(:project, :with_avatar) }
let!(:project2) { create(:project, :with_avatar) }
before do
UploadService.new(project1, uploaded_file, FileUploader).execute # Markdown upload
UploadService.new(project2, uploaded_file, FileUploader).execute # Markdown upload
# Save expected Upload attributes
@appearance_logo_attributes = appearance.uploads.where("path like '%/logo/%'").first.attributes.slice('path', 'uploader', 'size', 'checksum')
@appearance_header_logo_attributes = appearance.uploads.where("path like '%/header_logo/%'").first.attributes.slice('path', 'uploader', 'size', 'checksum')
@user1_avatar_attributes = user1.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum')
@user2_avatar_attributes = user2.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum')
@project1_avatar_attributes = project1.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum')
@project2_avatar_attributes = project2.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum')
@project1_markdown_attributes = project1.uploads.last.attributes.slice('path', 'uploader', 'size', 'checksum')
@project2_markdown_attributes = project2.uploads.last.attributes.slice('path', 'uploader', 'size', 'checksum')
# Untrack 4 files
user2.uploads.delete_all
project2.uploads.delete_all # 2 files: avatar and a Markdown upload
appearance.uploads.where("path like '%header_logo%'").delete_all
end
it 'tracks untracked uploads' do
expect do
migrate!
end.to change { uploads.count }.from(4).to(8)
expect(appearance.reload.uploads.where("path like '%/header_logo/%'").first.attributes).to include(@appearance_header_logo_attributes)
expect(user2.reload.uploads.first.attributes).to include(@user2_avatar_attributes)
expect(project2.reload.uploads.where(uploader: 'AvatarUploader').first.attributes).to include(@project2_avatar_attributes)
expect(project2.uploads.where(uploader: 'FileUploader').first.attributes).to include(@project2_markdown_attributes)
end
it 'ignores already-tracked uploads' do
migrate!
expect(appearance.reload.uploads.where("path like '%/logo/%'").first.attributes).to include(@appearance_logo_attributes)
expect(user1.reload.uploads.first.attributes).to include(@user1_avatar_attributes)
expect(project1.reload.uploads.where(uploader: 'AvatarUploader').first.attributes).to include(@project1_avatar_attributes)
expect(project1.uploads.where(uploader: 'FileUploader').first.attributes).to include(@project1_markdown_attributes)
end
it 'ignores uploads for deleted models' do
user2.destroy
project2.destroy
expect do
migrate!
end.to change { uploads.count }.from(4).to(5)
end
it 'the temporary table untracked_files_for_uploads no longer exists' do
migrate!
expect(table_exists?(:untracked_files_for_uploads)).to be_falsey
end
end
context 'without any uploads ever' do
it 'does not add any upload records' do
expect do
migrate!
end.not_to change { uploads.count }.from(0)
end
end
end end

View File

@ -11,4 +11,10 @@ module TrackUntrackedUploadsHelpers
def drop_temp_table_if_exists 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) ActiveRecord::Base.connection.drop_table(:untracked_files_for_uploads) if ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)
end end
def create_or_update_appearance(attrs)
a = Appearance.first_or_initialize(title: 'foo', description: 'bar')
a.update!(attrs)
a
end
end end