diff options
author | Lin Jen-Shin <godfat@godfat.org> | 2017-12-15 17:14:26 +0800 |
---|---|---|
committer | Lin Jen-Shin <godfat@godfat.org> | 2017-12-15 17:14:26 +0800 |
commit | 59ac184fcf64f1812fbfd88a00ea029ca3c1f4e7 (patch) | |
tree | 5db0594a6f568f02b4f54c6bf4eabe01229a9f95 /spec/lib/gitlab | |
parent | 85be6d83be4632c76760e373da131a90afb093b9 (diff) | |
parent | 1baea77438779e74657b49ca26810d6c8f041b41 (diff) | |
download | gitlab-ce-59ac184fcf64f1812fbfd88a00ea029ca3c1f4e7.tar.gz |
Merge remote-tracking branch 'upstream/master' into no-ivar-in-modules
* upstream/master: (671 commits)
Make rubocop happy
Use guard clause
Improve language
Prettify
Use temp branch
Pass info about who started the job and which job triggered it
Docs: add indexes for monitoring and performance monitoring
clearer-documentation-on-inline-diffs
Add docs for commit diff discussion in merge requests
sorting for tags api
Clear BatchLoader after each spec to prevent holding onto records longer than necessary
Include project in BatchLoader key to prevent returning blobs for the wrong project
moved lfs_blob_ids method into ExtractsPath module
Converted JS modules into exported modules
spec fixes
Bump gitlab-shell version to 5.10.3
Clear caches before updating MR diffs
Use new Ruby version 2.4 in GitLab QA images
moved lfs blob fetch from extractspath file
Update GitLab QA dependencies
...
Diffstat (limited to 'spec/lib/gitlab')
53 files changed, 2136 insertions, 698 deletions
diff --git a/spec/lib/gitlab/background_migration/create_fork_network_memberships_range_spec.rb b/spec/lib/gitlab/background_migration/create_fork_network_memberships_range_spec.rb index 79d2c071446..e1c4f9cfea7 100644 --- a/spec/lib/gitlab/background_migration/create_fork_network_memberships_range_spec.rb +++ b/spec/lib/gitlab/background_migration/create_fork_network_memberships_range_spec.rb @@ -2,19 +2,20 @@ require 'spec_helper' describe Gitlab::BackgroundMigration::CreateForkNetworkMembershipsRange, :migration, schema: 20170929131201 do let(:migration) { described_class.new } + let(:projects) { table(:projects) } - let(:base1) { create(:project) } - let(:base1_fork1) { create(:project) } - let(:base1_fork2) { create(:project) } + let(:base1) { projects.create } + let(:base1_fork1) { projects.create } + let(:base1_fork2) { projects.create } - let(:base2) { create(:project) } - let(:base2_fork1) { create(:project) } - let(:base2_fork2) { create(:project) } + let(:base2) { projects.create } + let(:base2_fork1) { projects.create } + let(:base2_fork2) { projects.create } - let(:fork_of_fork) { create(:project) } - let(:fork_of_fork2) { create(:project) } - let(:second_level_fork) { create(:project) } - let(:third_level_fork) { create(:project) } + let(:fork_of_fork) { projects.create } + let(:fork_of_fork2) { projects.create } + let(:second_level_fork) { projects.create } + let(:third_level_fork) { projects.create } let(:fork_network1) { fork_networks.find_by(root_project_id: base1.id) } let(:fork_network2) { fork_networks.find_by(root_project_id: base2.id) } @@ -97,7 +98,7 @@ describe Gitlab::BackgroundMigration::CreateForkNetworkMembershipsRange, :migrat end it 'does not miss members for forks of forks for which the root was deleted' do - forked_project_links.create(id: 9, forked_from_project_id: base1_fork1.id, forked_to_project_id: create(:project).id) + forked_project_links.create(id: 9, forked_from_project_id: base1_fork1.id, forked_to_project_id: projects.create.id) base1.destroy expect(migration.missing_members?(7, 10)).to be_falsy @@ -105,8 +106,8 @@ describe Gitlab::BackgroundMigration::CreateForkNetworkMembershipsRange, :migrat context 'with more forks' do before do - forked_project_links.create(id: 9, forked_from_project_id: fork_of_fork.id, forked_to_project_id: create(:project).id) - forked_project_links.create(id: 10, forked_from_project_id: fork_of_fork.id, forked_to_project_id: create(:project).id) + forked_project_links.create(id: 9, forked_from_project_id: fork_of_fork.id, forked_to_project_id: projects.create.id) + forked_project_links.create(id: 10, forked_from_project_id: fork_of_fork.id, forked_to_project_id: projects.create.id) end it 'only processes a single batch of links at a time' do diff --git a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb index cb52d971047..3998ca940a4 100644 --- a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb @@ -225,7 +225,8 @@ describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads, :migrati let(:user_class) { table(:users) } let(:author) { build(:user).becomes(user_class).tap(&:save!).becomes(User) } let(:namespace) { create(:namespace, owner: author) } - let(:project) { create(:project_empty_repo, namespace: namespace, creator: author) } + let(:projects) { table(:projects) } + let(:project) { projects.create(namespace_id: namespace.id, creator_id: author.id) } # We can not rely on FactoryGirl as the state of Event may change in ways that # the background migration does not expect, hence we use the Event class of diff --git a/spec/lib/gitlab/background_migration/populate_fork_networks_range_spec.rb b/spec/lib/gitlab/background_migration/populate_fork_networks_range_spec.rb index e52baf8dde7..8582af96199 100644 --- a/spec/lib/gitlab/background_migration/populate_fork_networks_range_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_fork_networks_range_spec.rb @@ -2,10 +2,11 @@ require 'spec_helper' describe Gitlab::BackgroundMigration::PopulateForkNetworksRange, :migration, schema: 20170929131201 do let(:migration) { described_class.new } - let(:base1) { create(:project) } + let(:projects) { table(:projects) } + let(:base1) { projects.create } - let(:base2) { create(:project) } - let(:base2_fork1) { create(:project) } + let(:base2) { projects.create } + let(:base2_fork1) { projects.create } let!(:forked_project_links) { table(:forked_project_links) } let!(:fork_networks) { table(:fork_networks) } @@ -18,10 +19,10 @@ describe Gitlab::BackgroundMigration::PopulateForkNetworksRange, :migration, sch # A normal fork link forked_project_links.create(id: 1, forked_from_project_id: base1.id, - forked_to_project_id: create(:project).id) + forked_to_project_id: projects.create.id) forked_project_links.create(id: 2, forked_from_project_id: base1.id, - forked_to_project_id: create(:project).id) + forked_to_project_id: projects.create.id) forked_project_links.create(id: 3, forked_from_project_id: base2.id, forked_to_project_id: base2_fork1.id) @@ -29,10 +30,10 @@ describe Gitlab::BackgroundMigration::PopulateForkNetworksRange, :migration, sch # create a fork of a fork forked_project_links.create(id: 4, forked_from_project_id: base2_fork1.id, - forked_to_project_id: create(:project).id) + forked_to_project_id: projects.create.id) forked_project_links.create(id: 5, - forked_from_project_id: create(:project).id, - forked_to_project_id: create(:project).id) + forked_from_project_id: projects.create.id, + forked_to_project_id: projects.create.id) # Stub out the calls to the other migrations allow(BackgroundMigrationWorker).to receive(:perform_in) @@ -63,7 +64,7 @@ describe Gitlab::BackgroundMigration::PopulateForkNetworksRange, :migration, sch end it 'creates a fork network for the fork of which the source was deleted' do - fork = create(:project) + fork = projects.create forked_project_links.create(id: 6, forked_from_project_id: 99999, forked_to_project_id: fork.id) migration.perform(5, 8) diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb new file mode 100644 index 00000000000..be45c00dfe6 --- /dev/null +++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb @@ -0,0 +1,519 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq do + include TrackUntrackedUploadsHelpers + + subject { described_class.new } + + let!(:untracked_files_for_uploads) { described_class::UntrackedFile } + 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) } + 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 + + # File records created by PrepareUntrackedUploads + untracked_files_for_uploads.create!(path: appearance.uploads.first.path) + untracked_files_for_uploads.create!(path: appearance.uploads.last.path) + untracked_files_for_uploads.create!(path: user1.uploads.first.path) + untracked_files_for_uploads.create!(path: user2.uploads.first.path) + untracked_files_for_uploads.create!(path: project1.uploads.first.path) + untracked_files_for_uploads.create!(path: project2.uploads.first.path) + untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project1.full_path}/#{project1.uploads.last.path}") + untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project2.full_path}/#{project2.uploads.last.path}") + + # 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 'adds untracked files to the uploads table' do + expect do + subject.perform(1, untracked_files_for_uploads.last.id) + end.to change { uploads.count }.from(4).to(8) + + expect(user2.uploads.count).to eq(1) + expect(project2.uploads.count).to eq(2) + expect(appearance.uploads.count).to eq(2) + end + + it 'deletes rows after processing them' do + expect(subject).to receive(:drop_temp_table_if_finished) # Don't drop the table so we can look at it + + expect do + subject.perform(1, untracked_files_for_uploads.last.id) + end.to change { untracked_files_for_uploads.count }.from(8).to(0) + end + + it 'does not create duplicate uploads of already tracked files' do + subject.perform(1, untracked_files_for_uploads.last.id) + + expect(user1.uploads.count).to eq(1) + expect(project1.uploads.count).to eq(2) + expect(appearance.uploads.count).to eq(2) + end + + it 'uses the start and end batch ids [only 1st half]' do + ids = untracked_files_for_uploads.all.order(:id).pluck(:id) + start_id = ids[0] + end_id = ids[3] + + expect do + subject.perform(start_id, end_id) + end.to change { uploads.count }.from(4).to(6) + + expect(user1.uploads.count).to eq(1) + expect(user2.uploads.count).to eq(1) + expect(appearance.uploads.count).to eq(2) + expect(project1.uploads.count).to eq(2) + expect(project2.uploads.count).to eq(0) + + # Only 4 have been either confirmed or added to uploads + expect(untracked_files_for_uploads.count).to eq(4) + end + + it 'uses the start and end batch ids [only 2nd half]' do + ids = untracked_files_for_uploads.all.order(:id).pluck(:id) + start_id = ids[4] + end_id = ids[7] + + expect do + subject.perform(start_id, end_id) + end.to change { uploads.count }.from(4).to(6) + + expect(user1.uploads.count).to eq(1) + expect(user2.uploads.count).to eq(0) + expect(appearance.uploads.count).to eq(1) + expect(project1.uploads.count).to eq(2) + expect(project2.uploads.count).to eq(2) + + # Only 4 have been either confirmed or added to uploads + expect(untracked_files_for_uploads.count).to eq(4) + end + + 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) + + expect(ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)).to be_truthy + 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(ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey + end + + it 'does not block a whole batch because of one bad path' do + untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project2.full_path}/._7d37bf4c747916390e596744117d5d1a") + expect(untracked_files_for_uploads.count).to eq(9) + expect(uploads.count).to eq(4) + + subject.perform(1, untracked_files_for_uploads.last.id) + + expect(untracked_files_for_uploads.count).to eq(1) + expect(uploads.count).to eq(8) + end + + it 'an unparseable path is shown in error output' do + bad_path = "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project2.full_path}/._7d37bf4c747916390e596744117d5d1a" + untracked_files_for_uploads.create!(path: bad_path) + + expect(Rails.logger).to receive(:error).with(/Error parsing path "#{bad_path}":/) + + subject.perform(1, untracked_files_for_uploads.last.id) + end + end + + context 'with no untracked files' do + it 'does not add to the uploads table (and does not raise error)' do + expect do + subject.perform(1, 1000) + end.not_to change { uploads.count }.from(0) + end + end + + describe 'upload outcomes for each path pattern' do + shared_examples_for 'non_markdown_file' do + let!(:expected_upload_attrs) { model.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum') } + let!(:untracked_file) { untracked_files_for_uploads.create!(path: expected_upload_attrs['path']) } + + before do + model.uploads.delete_all + end + + it 'creates an Upload record' do + expect do + subject.perform(1, untracked_files_for_uploads.last.id) + end.to change { model.reload.uploads.count }.from(0).to(1) + + expect(model.uploads.first.attributes).to include(expected_upload_attrs) + end + end + + context 'for an appearance logo file path' do + let(:model) { create_or_update_appearance(logo: uploaded_file) } + + it_behaves_like 'non_markdown_file' + end + + context 'for an appearance header_logo file path' do + let(:model) { create_or_update_appearance(header_logo: uploaded_file) } + + it_behaves_like 'non_markdown_file' + end + + context 'for a pre-Markdown Note attachment file path' do + let(:model) { create(:note, :with_attachment) } + let!(:expected_upload_attrs) { Upload.where(model_type: 'Note', model_id: model.id).first.attributes.slice('path', 'uploader', 'size', 'checksum') } + let!(:untracked_file) { untracked_files_for_uploads.create!(path: expected_upload_attrs['path']) } + + before do + Upload.where(model_type: 'Note', model_id: model.id).delete_all + end + + # Can't use the shared example because Note doesn't have an `uploads` association + it 'creates an Upload record' do + expect do + subject.perform(1, untracked_files_for_uploads.last.id) + end.to change { Upload.where(model_type: 'Note', model_id: model.id).count }.from(0).to(1) + + expect(Upload.where(model_type: 'Note', model_id: model.id).first.attributes).to include(expected_upload_attrs) + end + end + + context 'for a user avatar file path' do + let(:model) { create(:user, :with_avatar) } + + it_behaves_like 'non_markdown_file' + end + + context 'for a group avatar file path' do + let(:model) { create(:group, :with_avatar) } + + it_behaves_like 'non_markdown_file' + end + + context 'for a project avatar file path' do + let(:model) { create(:project, :with_avatar) } + + it_behaves_like 'non_markdown_file' + end + + context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do + let(:model) { create(:project) } + + before do + # Upload the file + UploadService.new(model, uploaded_file, FileUploader).execute + + # Create the untracked_files_for_uploads record + untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{model.full_path}/#{model.uploads.first.path}") + + # Save the expected upload attributes + @expected_upload_attrs = model.reload.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum') + + # Untrack the file + model.reload.uploads.delete_all + end + + it 'creates an Upload record' do + expect do + subject.perform(1, untracked_files_for_uploads.last.id) + end.to change { model.reload.uploads.count }.from(0).to(1) + + expect(model.uploads.first.attributes).to include(@expected_upload_attrs) + end + end + end +end + +describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do + include TrackUntrackedUploadsHelpers + + let(:upload_class) { Gitlab::BackgroundMigration::PopulateUntrackedUploads::Upload } + + before(:all) 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) + + expect(untracked_file.upload_path).to eq(expected_upload_path) + end + + context 'for an appearance logo file path' do + it 'returns the file path relative to the CarrierWave root' do + assert_upload_path('/-/system/appearance/logo/1/some_logo.jpg', 'uploads/-/system/appearance/logo/1/some_logo.jpg') + end + end + + context 'for an appearance header_logo file path' do + it 'returns the file path relative to the CarrierWave root' do + assert_upload_path('/-/system/appearance/header_logo/1/some_logo.jpg', 'uploads/-/system/appearance/header_logo/1/some_logo.jpg') + end + end + + context 'for a pre-Markdown Note attachment file path' do + it 'returns the file path relative to the CarrierWave root' do + assert_upload_path('/-/system/note/attachment/1234/some_attachment.pdf', 'uploads/-/system/note/attachment/1234/some_attachment.pdf') + end + end + + context 'for a user avatar file path' do + it 'returns the file path relative to the CarrierWave root' do + assert_upload_path('/-/system/user/avatar/1234/avatar.jpg', 'uploads/-/system/user/avatar/1234/avatar.jpg') + end + end + + context 'for a group avatar file path' do + it 'returns the file path relative to the CarrierWave root' do + assert_upload_path('/-/system/group/avatar/1234/avatar.jpg', 'uploads/-/system/group/avatar/1234/avatar.jpg') + end + end + + context 'for a project avatar file path' do + it 'returns the file path relative to the CarrierWave root' do + assert_upload_path('/-/system/project/avatar/1234/avatar.jpg', 'uploads/-/system/project/avatar/1234/avatar.jpg') + end + end + + context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do + it 'returns the file path relative to the project directory in uploads' do + project = create(:project) + random_hex = SecureRandom.hex + + assert_upload_path("/#{project.full_path}/#{random_hex}/Some file.jpg", "#{random_hex}/Some file.jpg") + end + end + end + + describe '#uploader' do + def assert_uploader(file_path, expected_uploader) + untracked_file = create_untracked_file(file_path) + + expect(untracked_file.uploader).to eq(expected_uploader) + end + + context 'for an appearance logo file path' do + it 'returns AttachmentUploader as a string' do + assert_uploader('/-/system/appearance/logo/1/some_logo.jpg', 'AttachmentUploader') + end + end + + context 'for an appearance header_logo file path' do + it 'returns AttachmentUploader as a string' do + assert_uploader('/-/system/appearance/header_logo/1/some_logo.jpg', 'AttachmentUploader') + end + end + + context 'for a pre-Markdown Note attachment file path' do + it 'returns AttachmentUploader as a string' do + assert_uploader('/-/system/note/attachment/1234/some_attachment.pdf', 'AttachmentUploader') + end + end + + context 'for a user avatar file path' do + it 'returns AvatarUploader as a string' do + assert_uploader('/-/system/user/avatar/1234/avatar.jpg', 'AvatarUploader') + end + end + + context 'for a group avatar file path' do + it 'returns AvatarUploader as a string' do + assert_uploader('/-/system/group/avatar/1234/avatar.jpg', 'AvatarUploader') + end + end + + context 'for a project avatar file path' do + it 'returns AvatarUploader as a string' do + assert_uploader('/-/system/project/avatar/1234/avatar.jpg', 'AvatarUploader') + end + end + + context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do + it 'returns FileUploader as a string' do + project = create(:project) + + assert_uploader("/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg", 'FileUploader') + end + end + end + + describe '#model_type' do + def assert_model_type(file_path, expected_model_type) + untracked_file = create_untracked_file(file_path) + + expect(untracked_file.model_type).to eq(expected_model_type) + end + + context 'for an appearance logo file path' do + it 'returns Appearance as a string' do + assert_model_type('/-/system/appearance/logo/1/some_logo.jpg', 'Appearance') + end + end + + context 'for an appearance header_logo file path' do + it 'returns Appearance as a string' do + assert_model_type('/-/system/appearance/header_logo/1/some_logo.jpg', 'Appearance') + end + end + + context 'for a pre-Markdown Note attachment file path' do + it 'returns Note as a string' do + assert_model_type('/-/system/note/attachment/1234/some_attachment.pdf', 'Note') + end + end + + context 'for a user avatar file path' do + it 'returns User as a string' do + assert_model_type('/-/system/user/avatar/1234/avatar.jpg', 'User') + end + end + + context 'for a group avatar file path' do + it 'returns Namespace as a string' do + assert_model_type('/-/system/group/avatar/1234/avatar.jpg', 'Namespace') + end + end + + context 'for a project avatar file path' do + it 'returns Project as a string' do + assert_model_type('/-/system/project/avatar/1234/avatar.jpg', 'Project') + end + end + + context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do + it 'returns Project as a string' do + project = create(:project) + + assert_model_type("/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg", 'Project') + end + end + end + + describe '#model_id' do + def assert_model_id(file_path, expected_model_id) + untracked_file = create_untracked_file(file_path) + + expect(untracked_file.model_id).to eq(expected_model_id) + end + + context 'for an appearance logo file path' do + it 'returns the ID as a string' do + assert_model_id('/-/system/appearance/logo/1/some_logo.jpg', 1) + end + end + + context 'for an appearance header_logo file path' do + it 'returns the ID as a string' do + assert_model_id('/-/system/appearance/header_logo/1/some_logo.jpg', 1) + end + end + + context 'for a pre-Markdown Note attachment file path' do + it 'returns the ID as a string' do + assert_model_id('/-/system/note/attachment/1234/some_attachment.pdf', 1234) + end + end + + context 'for a user avatar file path' do + it 'returns the ID as a string' do + assert_model_id('/-/system/user/avatar/1234/avatar.jpg', 1234) + end + end + + context 'for a group avatar file path' do + it 'returns the ID as a string' do + assert_model_id('/-/system/group/avatar/1234/avatar.jpg', 1234) + end + end + + context 'for a project avatar file path' do + it 'returns the ID as a string' do + assert_model_id('/-/system/project/avatar/1234/avatar.jpg', 1234) + end + end + + context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do + it 'returns the ID as a string' do + project = create(:project) + + assert_model_id("/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg", project.id) + end + end + end + + describe '#file_size' do + context 'for an appearance logo file path' do + let(:appearance) { create_or_update_appearance(logo: uploaded_file) } + let(:untracked_file) { described_class.create!(path: appearance.uploads.first.path) } + + it 'returns the file size' do + expect(untracked_file.file_size).to eq(35255) + end + + it 'returns the same thing that CarrierWave would return' do + expect(untracked_file.file_size).to eq(appearance.logo.size) + end + end + + context 'for a project avatar file path' do + let(:project) { create(:project, avatar: uploaded_file) } + let(:untracked_file) { described_class.create!(path: project.uploads.first.path) } + + it 'returns the file size' do + expect(untracked_file.file_size).to eq(35255) + end + + it 'returns the same thing that CarrierWave would return' do + expect(untracked_file.file_size).to eq(project.avatar.size) + end + end + + context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do + let(:project) { create(:project) } + let(:untracked_file) { create_untracked_file("/#{project.full_path}/#{project.uploads.first.path}") } + + before do + UploadService.new(project, uploaded_file, FileUploader).execute + end + + it 'returns the file size' do + expect(untracked_file.file_size).to eq(35255) + end + + it 'returns the same thing that CarrierWave would return' do + expect(untracked_file.file_size).to eq(project.uploads.first.size) + end + end + end + + def create_untracked_file(path_relative_to_upload_dir) + described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}#{path_relative_to_upload_dir}") + end +end diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb new file mode 100644 index 00000000000..cd3f1a45270 --- /dev/null +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -0,0 +1,242 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do + include TrackUntrackedUploadsHelpers + + let!(:untracked_files_for_uploads) { described_class::UntrackedFile } + + matcher :be_scheduled_migration do |*expected| + match do |migration| + BackgroundMigrationWorker.jobs.any? do |job| + job['args'] == [migration, expected] + end + end + + failure_message do |migration| + "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!" + end + end + + before do + DatabaseCleaner.clean + + drop_temp_table_if_exists + 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 + example.run + end + end + + it 'ensures the untracked_files_for_uploads table exists' do + expect do + described_class.new.perform + end.to change { ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads) }.from(false).to(true) + end + + it 'has a path field long enough for really long paths' do + described_class.new.perform + + component = 'a' * 255 + + long_path = [ + 'uploads', + component, # project.full_path + component # filename + ].flatten.join('/') + + record = untracked_files_for_uploads.create!(path: long_path) + expect(record.reload.path.size).to eq(519) + end + + context "test bulk insert with ON CONFLICT DO NOTHING or IGNORE" do + around do |example| + # If this is CI, we use Postgres 9.2 so this whole context should be + # skipped since we're unable to use ON CONFLICT DO NOTHING or IGNORE. + if described_class.new.send(:can_bulk_insert_and_ignore_duplicates?) + example.run + end + end + + context 'when files were uploaded before and after hashed storage was enabled' do + let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) } + let!(:user) { create(:user, :with_avatar) } + let!(:project1) { create(:project, :with_avatar) } + let(:project2) { create(:project) } # instantiate after enabling hashed_storage + + before do + # Markdown upload before enabling hashed_storage + UploadService.new(project1, uploaded_file, FileUploader).execute + + stub_application_setting(hashed_storage_enabled: true) + + # Markdown upload after enabling hashed_storage + UploadService.new(project2, uploaded_file, FileUploader).execute + end + + it 'adds unhashed files to the untracked_files_for_uploads table' do + described_class.new.perform + + expect(untracked_files_for_uploads.count).to eq(5) + end + + it 'adds files with paths relative to CarrierWave.root' do + described_class.new.perform + untracked_files_for_uploads.all.each do |file| + expect(file.path.start_with?('uploads/')).to be_truthy + end + end + + it 'does not add hashed files to the untracked_files_for_uploads table' do + described_class.new.perform + + hashed_file_path = project2.uploads.where(uploader: 'FileUploader').first.path + expect(untracked_files_for_uploads.where("path like '%#{hashed_file_path}%'").exists?).to be_falsey + end + + 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) + expect(BackgroundMigrationWorker.jobs.size).to eq(1) + end + + # E.g. from a previous failed run of this background migration + context 'when there is existing data in untracked_files_for_uploads' do + before do + described_class.new.perform + end + + it 'does not error or produce duplicates of existing data' do + expect do + described_class.new.perform + end.not_to change { untracked_files_for_uploads.count }.from(5) + end + end + + # E.g. The installation is in use at the time of migration, and someone has + # just uploaded a file + context 'when there are files in /uploads/tmp' do + let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') } + + before do + FileUtils.touch(tmp_file) + end + + after do + FileUtils.rm(tmp_file) + end + + it 'does not add files from /uploads/tmp' do + described_class.new.perform + + expect(untracked_files_for_uploads.count).to eq(5) + end + end + end + end + + context 'test bulk insert without ON CONFLICT DO NOTHING or IGNORE' do + before do + # If this is CI, we use Postgres 9.2 so this stub has no effect. + # + # If this is being run on Postgres 9.5+ or MySQL, then this stub allows us + # to test the bulk insert functionality without ON CONFLICT DO NOTHING or + # IGNORE. + allow_any_instance_of(described_class).to receive(:postgresql_pre_9_5?).and_return(true) + end + + context 'when files were uploaded before and after hashed storage was enabled' do + let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) } + let!(:user) { create(:user, :with_avatar) } + let!(:project1) { create(:project, :with_avatar) } + let(:project2) { create(:project) } # instantiate after enabling hashed_storage + + before do + # Markdown upload before enabling hashed_storage + UploadService.new(project1, uploaded_file, FileUploader).execute + + stub_application_setting(hashed_storage_enabled: true) + + # Markdown upload after enabling hashed_storage + UploadService.new(project2, uploaded_file, FileUploader).execute + end + + it 'adds unhashed files to the untracked_files_for_uploads table' do + described_class.new.perform + + expect(untracked_files_for_uploads.count).to eq(5) + end + + it 'adds files with paths relative to CarrierWave.root' do + described_class.new.perform + untracked_files_for_uploads.all.each do |file| + expect(file.path.start_with?('uploads/')).to be_truthy + end + end + + it 'does not add hashed files to the untracked_files_for_uploads table' do + described_class.new.perform + + hashed_file_path = project2.uploads.where(uploader: 'FileUploader').first.path + expect(untracked_files_for_uploads.where("path like '%#{hashed_file_path}%'").exists?).to be_falsey + end + + 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) + expect(BackgroundMigrationWorker.jobs.size).to eq(1) + end + + # E.g. from a previous failed run of this background migration + context 'when there is existing data in untracked_files_for_uploads' do + before do + described_class.new.perform + end + + it 'does not error or produce duplicates of existing data' do + expect do + described_class.new.perform + end.not_to change { untracked_files_for_uploads.count }.from(5) + end + end + + # E.g. The installation is in use at the time of migration, and someone has + # just uploaded a file + context 'when there are files in /uploads/tmp' do + let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') } + + before do + FileUtils.touch(tmp_file) + end + + after do + FileUtils.rm(tmp_file) + end + + it 'does not add files from /uploads/tmp' do + described_class.new.perform + + expect(untracked_files_for_uploads.count).to eq(5) + end + end + end + end + + # Very new or lightly-used installations that are running this migration + # may not have an upload directory because they have no uploads. + context 'when no files were ever uploaded' do + it 'does not add to the untracked_files_for_uploads table (and does not raise error)' do + described_class.new.perform + + expect(untracked_files_for_uploads.count).to eq(0) + end + end +end diff --git a/spec/lib/gitlab/backup/manager_spec.rb b/spec/lib/gitlab/backup/manager_spec.rb deleted file mode 100644 index b68301a066a..00000000000 --- a/spec/lib/gitlab/backup/manager_spec.rb +++ /dev/null @@ -1,299 +0,0 @@ -require 'spec_helper' - -describe Backup::Manager do - include StubENV - - let(:progress) { StringIO.new } - - before do - allow(progress).to receive(:puts) - allow(progress).to receive(:print) - - allow_any_instance_of(String).to receive(:color) do |string, _color| - string - end - - @old_progress = $progress # rubocop:disable Style/GlobalVars - $progress = progress # rubocop:disable Style/GlobalVars - end - - after do - $progress = @old_progress # rubocop:disable Style/GlobalVars - end - - describe '#remove_old' do - let(:files) do - [ - '1451606400_2016_01_01_1.2.3_gitlab_backup.tar', - '1451520000_2015_12_31_4.5.6_gitlab_backup.tar', - '1451520000_2015_12_31_4.5.6-pre_gitlab_backup.tar', - '1451520000_2015_12_31_4.5.6-rc1_gitlab_backup.tar', - '1451520000_2015_12_31_4.5.6-pre-ee_gitlab_backup.tar', - '1451510000_2015_12_30_gitlab_backup.tar', - '1450742400_2015_12_22_gitlab_backup.tar', - '1449878400_gitlab_backup.tar', - '1449014400_gitlab_backup.tar', - 'manual_gitlab_backup.tar' - ] - end - - before do - allow(Dir).to receive(:chdir).and_yield - allow(Dir).to receive(:glob).and_return(files) - allow(FileUtils).to receive(:rm) - allow(Time).to receive(:now).and_return(Time.utc(2016)) - end - - context 'when keep_time is zero' do - before do - allow(Gitlab.config.backup).to receive(:keep_time).and_return(0) - - subject.remove_old - end - - it 'removes no files' do - expect(FileUtils).not_to have_received(:rm) - end - - it 'prints a skipped message' do - expect(progress).to have_received(:puts).with('skipping') - end - end - - context 'when no valid file is found' do - let(:files) do - [ - '14516064000_2016_01_01_1.2.3_gitlab_backup.tar', - 'foo_1451520000_2015_12_31_4.5.6_gitlab_backup.tar', - '1451520000_2015_12_31_4.5.6-foo_gitlab_backup.tar' - ] - end - - before do - allow(Gitlab.config.backup).to receive(:keep_time).and_return(1) - - subject.remove_old - end - - it 'removes no files' do - expect(FileUtils).not_to have_received(:rm) - end - - it 'prints a done message' do - expect(progress).to have_received(:puts).with('done. (0 removed)') - end - end - - context 'when there are no files older than keep_time' do - before do - # Set to 30 days - allow(Gitlab.config.backup).to receive(:keep_time).and_return(2592000) - - subject.remove_old - end - - it 'removes no files' do - expect(FileUtils).not_to have_received(:rm) - end - - it 'prints a done message' do - expect(progress).to have_received(:puts).with('done. (0 removed)') - end - end - - context 'when keep_time is set to remove files' do - before do - # Set to 1 second - allow(Gitlab.config.backup).to receive(:keep_time).and_return(1) - - subject.remove_old - end - - it 'removes matching files with a human-readable versioned timestamp' do - expect(FileUtils).to have_received(:rm).with(files[1]) - expect(FileUtils).to have_received(:rm).with(files[2]) - expect(FileUtils).to have_received(:rm).with(files[3]) - end - - it 'removes matching files with a human-readable versioned timestamp with tagged EE' do - expect(FileUtils).to have_received(:rm).with(files[4]) - end - - it 'removes matching files with a human-readable non-versioned timestamp' do - expect(FileUtils).to have_received(:rm).with(files[5]) - expect(FileUtils).to have_received(:rm).with(files[6]) - end - - it 'removes matching files without a human-readable timestamp' do - expect(FileUtils).to have_received(:rm).with(files[7]) - expect(FileUtils).to have_received(:rm).with(files[8]) - end - - it 'does not remove files that are not old enough' do - expect(FileUtils).not_to have_received(:rm).with(files[0]) - end - - it 'does not remove non-matching files' do - expect(FileUtils).not_to have_received(:rm).with(files[9]) - end - - it 'prints a done message' do - expect(progress).to have_received(:puts).with('done. (8 removed)') - end - end - - context 'when removing a file fails' do - let(:file) { files[1] } - let(:message) { "Permission denied @ unlink_internal - #{file}" } - - before do - allow(Gitlab.config.backup).to receive(:keep_time).and_return(1) - allow(FileUtils).to receive(:rm).with(file).and_raise(Errno::EACCES, message) - - subject.remove_old - end - - it 'removes the remaining expected files' do - expect(FileUtils).to have_received(:rm).with(files[4]) - expect(FileUtils).to have_received(:rm).with(files[5]) - expect(FileUtils).to have_received(:rm).with(files[6]) - expect(FileUtils).to have_received(:rm).with(files[7]) - expect(FileUtils).to have_received(:rm).with(files[8]) - end - - it 'sets the correct removed count' do - expect(progress).to have_received(:puts).with('done. (7 removed)') - end - - it 'prints the error from file that could not be removed' do - expect(progress).to have_received(:puts).with(a_string_matching(message)) - end - end - end - - describe '#unpack' do - context 'when there are no backup files in the directory' do - before do - allow(Dir).to receive(:glob).and_return([]) - end - - it 'fails the operation and prints an error' do - expect { subject.unpack }.to raise_error SystemExit - expect(progress).to have_received(:puts) - .with(a_string_matching('No backups found')) - end - end - - context 'when there are two backup files in the directory and BACKUP variable is not set' do - before do - allow(Dir).to receive(:glob).and_return( - [ - '1451606400_2016_01_01_1.2.3_gitlab_backup.tar', - '1451520000_2015_12_31_gitlab_backup.tar' - ] - ) - end - - it 'fails the operation and prints an error' do - expect { subject.unpack }.to raise_error SystemExit - expect(progress).to have_received(:puts) - .with(a_string_matching('Found more than one backup')) - end - end - - context 'when BACKUP variable is set to a non-existing file' do - before do - allow(Dir).to receive(:glob).and_return( - [ - '1451606400_2016_01_01_gitlab_backup.tar' - ] - ) - allow(File).to receive(:exist?).and_return(false) - - stub_env('BACKUP', 'wrong') - end - - it 'fails the operation and prints an error' do - expect { subject.unpack }.to raise_error SystemExit - expect(File).to have_received(:exist?).with('wrong_gitlab_backup.tar') - expect(progress).to have_received(:puts) - .with(a_string_matching('The backup file wrong_gitlab_backup.tar does not exist')) - end - end - - context 'when BACKUP variable is set to a correct file' do - before do - allow(Dir).to receive(:glob).and_return( - [ - '1451606400_2016_01_01_1.2.3_gitlab_backup.tar' - ] - ) - allow(File).to receive(:exist?).and_return(true) - allow(Kernel).to receive(:system).and_return(true) - allow(YAML).to receive(:load_file).and_return(gitlab_version: Gitlab::VERSION) - - stub_env('BACKUP', '1451606400_2016_01_01_1.2.3') - end - - it 'unpacks the file' do - subject.unpack - - expect(Kernel).to have_received(:system) - .with("tar", "-xf", "1451606400_2016_01_01_1.2.3_gitlab_backup.tar") - expect(progress).to have_received(:puts).with(a_string_matching('done')) - end - end - end - - describe '#upload' do - let(:backup_file) { Tempfile.new('backup', Gitlab.config.backup.path) } - let(:backup_filename) { File.basename(backup_file.path) } - - before do - allow(subject).to receive(:tar_file).and_return(backup_filename) - - stub_backup_setting( - upload: { - connection: { - provider: 'AWS', - aws_access_key_id: 'id', - aws_secret_access_key: 'secret' - }, - remote_directory: 'directory', - multipart_chunk_size: 104857600, - encryption: nil, - storage_class: nil - } - ) - - # the Fog mock only knows about directories we create explicitly - Fog.mock! - connection = ::Fog::Storage.new(Gitlab.config.backup.upload.connection.symbolize_keys) - connection.directories.create(key: Gitlab.config.backup.upload.remote_directory) - end - - context 'target path' do - it 'uses the tar filename by default' do - expect_any_instance_of(Fog::Collection).to receive(:create) - .with(hash_including(key: backup_filename)) - .and_return(true) - - Dir.chdir(Gitlab.config.backup.path) do - subject.upload - end - end - - it 'adds the DIRECTORY environment variable if present' do - stub_env('DIRECTORY', 'daily') - - expect_any_instance_of(Fog::Collection).to receive(:create) - .with(hash_including(key: "daily/#{backup_filename}")) - .and_return(true) - - Dir.chdir(Gitlab.config.backup.path) do - subject.upload - end - end - end - end -end diff --git a/spec/lib/gitlab/backup/repository_spec.rb b/spec/lib/gitlab/backup/repository_spec.rb deleted file mode 100644 index 535cce12780..00000000000 --- a/spec/lib/gitlab/backup/repository_spec.rb +++ /dev/null @@ -1,117 +0,0 @@ -require 'spec_helper' - -describe Backup::Repository do - let(:progress) { StringIO.new } - let!(:project) { create(:project) } - - before do - allow(progress).to receive(:puts) - allow(progress).to receive(:print) - - allow_any_instance_of(String).to receive(:color) do |string, _color| - string - end - - allow_any_instance_of(described_class).to receive(:progress).and_return(progress) - end - - describe '#dump' do - describe 'repo failure' do - before do - allow_any_instance_of(Repository).to receive(:empty_repo?).and_raise(Rugged::OdbError) - allow(Gitlab::Popen).to receive(:popen).and_return(['normal output', 0]) - end - - it 'does not raise error' do - expect { described_class.new.dump }.not_to raise_error - end - - it 'shows the appropriate error' do - described_class.new.dump - - expect(progress).to have_received(:puts).with("Ignoring repository error and continuing backing up project: #{project.full_path} - Rugged::OdbError") - end - end - - describe 'command failure' do - before do - allow_any_instance_of(Repository).to receive(:empty_repo?).and_return(false) - allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1]) - end - - it 'shows the appropriate error' do - described_class.new.dump - - expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error") - end - end - end - - describe '#restore' do - describe 'command failure' do - before do - allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1]) - end - - it 'shows the appropriate error' do - described_class.new.restore - - expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error") - end - end - end - - describe '#empty_repo?' do - context 'for a wiki' do - let(:wiki) { create(:project_wiki) } - - context 'wiki repo has content' do - let!(:wiki_page) { create(:wiki_page, wiki: wiki) } - - before do - wiki.repository.exists? # initial cache - end - - context '`repository.exists?` is incorrectly cached as false' do - before do - repo = wiki.repository - repo.send(:cache).expire(:exists?) - repo.send(:cache).fetch(:exists?) { false } - repo.send(:instance_variable_set, :@exists, false) - end - - it 'returns false, regardless of bad cache value' do - expect(described_class.new.send(:empty_repo?, wiki)).to be_falsey - end - end - - context '`repository.exists?` is correctly cached as true' do - it 'returns false' do - expect(described_class.new.send(:empty_repo?, wiki)).to be_falsey - end - end - end - - context 'wiki repo does not have content' do - context '`repository.exists?` is incorrectly cached as true' do - before do - repo = wiki.repository - repo.send(:cache).expire(:exists?) - repo.send(:cache).fetch(:exists?) { true } - repo.send(:instance_variable_set, :@exists, true) - end - - it 'returns true, regardless of bad cache value' do - expect(described_class.new.send(:empty_repo?, wiki)).to be_truthy - end - end - - context '`repository.exists?` is correctly cached as false' do - it 'returns true' do - expect(described_class.new.send(:empty_repo?, wiki)).to be_truthy - end - end - end - end - end -end diff --git a/spec/lib/gitlab/bare_repository_import/importer_spec.rb b/spec/lib/gitlab/bare_repository_import/importer_spec.rb index 7f3bf5fc41c..8a83e446935 100644 --- a/spec/lib/gitlab/bare_repository_import/importer_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/importer_spec.rb @@ -132,6 +132,23 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do expect(File).to exist(File.join(project.repository_storage_path, project.disk_path + '.git')) end + + it 'moves an existing project to the correct path' do + # This is a quick way to get a valid repository instead of copying an + # existing one. Since it's not persisted, the importer will try to + # create the project. + project = build(:project, :repository) + original_commit_count = project.repository.commit_count + + bare_repo = Gitlab::BareRepositoryImport::Repository.new(project.repository_storage_path, project.repository.path) + gitlab_importer = described_class.new(admin, bare_repo) + + expect(gitlab_importer).to receive(:create_project).and_call_original + + new_project = gitlab_importer.create_project_if_needed + + expect(new_project.repository.commit_count).to eq(original_commit_count) + end end context 'with Wiki' do diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index 2db737f5fb6..61b73abcba4 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -46,6 +46,13 @@ describe ::Gitlab::BareRepositoryImport::Repository do describe '#project_full_path' do it 'returns the project full path' do expect(project_repo_path.repo_path).to eq('/full/path/to/repo.git') + expect(project_repo_path.project_full_path).to eq('to/repo') + end + + it 'with no trailing slash in the root path' do + repo_path = described_class.new('/full/path', '/full/path/to/repo.git') + + expect(repo_path.project_full_path).to eq('to/repo') end end end diff --git a/spec/lib/gitlab/checks/project_moved_spec.rb b/spec/lib/gitlab/checks/project_moved_spec.rb new file mode 100644 index 00000000000..fa1575e2177 --- /dev/null +++ b/spec/lib/gitlab/checks/project_moved_spec.rb @@ -0,0 +1,81 @@ +require 'rails_helper' + +describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do + let(:user) { create(:user) } + let(:project) { create(:project) } + + describe '.fetch_redirct_message' do + context 'with a redirect message queue' do + it 'should return the redirect message' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + project_moved.add_redirect_message + + expect(described_class.fetch_redirect_message(user.id, project.id)).to eq(project_moved.redirect_message) + end + + it 'should delete the redirect message from redis' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + project_moved.add_redirect_message + + expect(Gitlab::Redis::SharedState.with { |redis| redis.get("redirect_namespace:#{user.id}:#{project.id}") }).not_to be_nil + described_class.fetch_redirect_message(user.id, project.id) + expect(Gitlab::Redis::SharedState.with { |redis| redis.get("redirect_namespace:#{user.id}:#{project.id}") }).to be_nil + end + end + + context 'with no redirect message queue' do + it 'should return nil' do + expect(described_class.fetch_redirect_message(1, 2)).to be_nil + end + end + end + + describe '#add_redirect_message' do + it 'should queue a redirect message' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + expect(project_moved.add_redirect_message).to eq("OK") + end + end + + describe '#redirect_message' do + context 'when the push is rejected' do + it 'should return a redirect message telling the user to try again' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + message = "Project 'foo/bar' was moved to '#{project.full_path}'." + + "\n\nPlease update your Git remote:" + + "\n\n git remote set-url origin #{project.http_url_to_repo} and try again.\n" + + expect(project_moved.redirect_message(rejected: true)).to eq(message) + end + end + + context 'when the push is not rejected' do + it 'should return a redirect message' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + message = "Project 'foo/bar' was moved to '#{project.full_path}'." + + "\n\nPlease update your Git remote:" + + "\n\n git remote set-url origin #{project.http_url_to_repo}\n" + + expect(project_moved.redirect_message).to eq(message) + end + end + end + + describe '#permanent_redirect?' do + context 'with a permanent RedirectRoute' do + it 'should return true' do + project.route.create_redirect('foo/bar', permanent: true) + project_moved = described_class.new(project, user, 'foo/bar', 'http') + expect(project_moved.permanent_redirect?).to be_truthy + end + end + + context 'without a permanent RedirectRoute' do + it 'should return false' do + project.route.create_redirect('foo/bar') + project_moved = described_class.new(project, user, 'foo/bar', 'http') + expect(project_moved.permanent_redirect?).to be_falsy + end + end + end +end diff --git a/spec/lib/gitlab/ci/build/policy/kubernetes_spec.rb b/spec/lib/gitlab/ci/build/policy/kubernetes_spec.rb index 15eb01eb472..4884d5f8ba4 100644 --- a/spec/lib/gitlab/ci/build/policy/kubernetes_spec.rb +++ b/spec/lib/gitlab/ci/build/policy/kubernetes_spec.rb @@ -4,11 +4,24 @@ describe Gitlab::Ci::Build::Policy::Kubernetes do let(:pipeline) { create(:ci_pipeline, project: project) } context 'when kubernetes service is active' do - set(:project) { create(:kubernetes_project) } + shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do + it 'is satisfied by a kubernetes pipeline' do + expect(described_class.new('active')) + .to be_satisfied_by(pipeline) + end + end - it 'is satisfied by a kubernetes pipeline' do - expect(described_class.new('active')) - .to be_satisfied_by(pipeline) + context 'when user configured kubernetes from Integration > Kubernetes' do + let(:project) { create(:kubernetes_project) } + + it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' + end + + context 'when user configured kubernetes from CI/CD > Clusters' do + let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } + + it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb new file mode 100644 index 00000000000..3ae7053a995 --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb @@ -0,0 +1,86 @@ +require 'spec_helper' + +describe Gitlab::Ci::Pipeline::Chain::Build do + set(:project) { create(:project, :repository) } + set(:user) { create(:user) } + let(:pipeline) { Ci::Pipeline.new } + + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + source: :push, + origin_ref: 'master', + checkout_sha: project.commit.id, + after_sha: nil, + before_sha: nil, + trigger_request: nil, + schedule: nil, + project: project, + current_user: user) + end + + let(:step) { described_class.new(pipeline, command) } + + before do + stub_repository_ci_yaml_file(sha: anything) + end + + it 'never breaks the chain' do + step.perform! + + expect(step.break?).to be false + end + + it 'fills pipeline object with data' do + step.perform! + + expect(pipeline.sha).not_to be_empty + expect(pipeline.sha).to eq project.commit.id + expect(pipeline.ref).to eq 'master' + expect(pipeline.tag).to be false + expect(pipeline.user).to eq user + expect(pipeline.project).to eq project + end + + it 'sets a valid config source' do + step.perform! + + expect(pipeline.repository_source?).to be true + end + + it 'returns a valid pipeline' do + step.perform! + + expect(pipeline).to be_valid + end + + it 'does not persist a pipeline' do + step.perform! + + expect(pipeline).not_to be_persisted + end + + context 'when pipeline is running for a tag' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + source: :push, + origin_ref: 'mytag', + checkout_sha: project.commit.id, + after_sha: nil, + before_sha: nil, + trigger_request: nil, + schedule: nil, + project: project, + current_user: user) + end + + before do + allow_any_instance_of(Repository).to receive(:tag_exists?).with('mytag').and_return(true) + + step.perform! + end + + it 'correctly indicated that this is a tagged pipeline' do + expect(pipeline).to be_tag + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb new file mode 100644 index 00000000000..75a177d2d1f --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -0,0 +1,185 @@ +require 'spec_helper' + +describe Gitlab::Ci::Pipeline::Chain::Command do + set(:project) { create(:project, :repository) } + + describe '#initialize' do + subject do + described_class.new(origin_ref: 'master') + end + + it 'properly initialises object from hash' do + expect(subject.origin_ref).to eq('master') + end + end + + context 'handling of origin_ref' do + let(:command) { described_class.new(project: project, origin_ref: origin_ref) } + + describe '#branch_exists?' do + subject { command.branch_exists? } + + context 'for existing branch' do + let(:origin_ref) { 'master' } + + it { is_expected.to eq(true) } + end + + context 'for invalid branch' do + let(:origin_ref) { 'something' } + + it { is_expected.to eq(false) } + end + end + + describe '#tag_exists?' do + subject { command.tag_exists? } + + context 'for existing ref' do + let(:origin_ref) { 'v1.0.0' } + + it { is_expected.to eq(true) } + end + + context 'for invalid ref' do + let(:origin_ref) { 'something' } + + it { is_expected.to eq(false) } + end + end + + describe '#ref' do + subject { command.ref } + + context 'for regular ref' do + let(:origin_ref) { 'master' } + + it { is_expected.to eq('master') } + end + + context 'for branch ref' do + let(:origin_ref) { 'refs/heads/master' } + + it { is_expected.to eq('master') } + end + + context 'for tag ref' do + let(:origin_ref) { 'refs/tags/1.0.0' } + + it { is_expected.to eq('1.0.0') } + end + + context 'for other refs' do + let(:origin_ref) { 'refs/merge-requests/11/head' } + + it { is_expected.to eq('refs/merge-requests/11/head') } + end + end + end + + describe '#sha' do + subject { command.sha } + + context 'when invalid checkout_sha is specified' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa') } + + it 'returns empty value' do + is_expected.to be_nil + end + end + + context 'when a valid checkout_sha is specified' do + let(:command) { described_class.new(project: project, checkout_sha: project.commit.id) } + + it 'returns checkout_sha' do + is_expected.to eq(project.commit.id) + end + end + + context 'when a valid after_sha is specified' do + let(:command) { described_class.new(project: project, after_sha: project.commit.id) } + + it 'returns after_sha' do + is_expected.to eq(project.commit.id) + end + end + + context 'when a valid origin_ref is specified' do + let(:command) { described_class.new(project: project, origin_ref: 'HEAD') } + + it 'returns SHA for given ref' do + is_expected.to eq(project.commit.id) + end + end + end + + describe '#origin_sha' do + subject { command.origin_sha } + + context 'when using checkout_sha and after_sha' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa', after_sha: 'bbb') } + + it 'uses checkout_sha' do + is_expected.to eq('aaa') + end + end + + context 'when using after_sha only' do + let(:command) { described_class.new(project: project, after_sha: 'bbb') } + + it 'uses after_sha' do + is_expected.to eq('bbb') + end + end + end + + describe '#before_sha' do + subject { command.before_sha } + + context 'when using checkout_sha and before_sha' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa', before_sha: 'bbb') } + + it 'uses before_sha' do + is_expected.to eq('bbb') + end + end + + context 'when using checkout_sha only' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa') } + + it 'uses checkout_sha' do + is_expected.to eq('aaa') + end + end + + context 'when checkout_sha and before_sha are empty' do + let(:command) { described_class.new(project: project) } + + it 'uses BLANK_SHA' do + is_expected.to eq(Gitlab::Git::BLANK_SHA) + end + end + end + + describe '#protected_ref?' do + let(:command) { described_class.new(project: project, origin_ref: 'my-branch') } + + subject { command.protected_ref? } + + context 'when a ref is protected' do + before do + expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(true) + end + + it { is_expected.to eq(true) } + end + + context 'when a ref is unprotected' do + before do + expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(false) + end + + it { is_expected.to eq(false) } + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb index f54e2326b06..1b03227d67b 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb @@ -10,9 +10,9 @@ describe Gitlab::Ci::Pipeline::Chain::Create do end let(:command) do - double('command', project: project, - current_user: user, - seeds_block: nil) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, seeds_block: nil) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb index e165e0fac2a..eca23694a2b 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::Ci::Pipeline::Chain::Sequence do set(:user) { create(:user) } let(:pipeline) { build_stubbed(:ci_pipeline) } - let(:command) { double('command' ) } + let(:command) { Gitlab::Ci::Pipeline::Chain::Command.new } let(:first_step) { spy('first step') } let(:second_step) { spy('second step') } let(:sequence) { [first_step, second_step] } diff --git a/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb index 32bd5de829b..dc13cae961c 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb @@ -6,10 +6,11 @@ describe Gitlab::Ci::Pipeline::Chain::Skip do set(:pipeline) { create(:ci_pipeline, project: project) } let(:command) do - double('command', project: project, - current_user: user, - ignore_skip_ci: false, - save_incompleted: true) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, + ignore_skip_ci: false, + save_incompleted: true) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb index 0bbdd23f4d6..a973ccda8de 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb @@ -5,11 +5,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do set(:user) { create(:user) } let(:pipeline) do - build_stubbed(:ci_pipeline, ref: ref, project: project) + build_stubbed(:ci_pipeline, project: project) end let(:command) do - double('command', project: project, current_user: user) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: ref) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb index 8357af38f92..5c12c6e6392 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb @@ -5,9 +5,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Config do set(:user) { create(:user) } let(:command) do - double('command', project: project, - current_user: user, - save_incompleted: true) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, + save_incompleted: true) end let!(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb index bb356efe9ad..fb1b53fc55c 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb @@ -3,10 +3,7 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do set(:project) { create(:project, :repository) } set(:user) { create(:user) } - - let(:command) do - double('command', project: project, current_user: user) - end + let(:pipeline) { build_stubbed(:ci_pipeline) } let!(:step) { described_class.new(pipeline, command) } @@ -14,9 +11,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do step.perform! end - context 'when pipeline ref and sha exists' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'master', sha: '123', project: project) + context 'when ref and sha exists' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'master', checkout_sha: project.commit.id) end it 'does not break the chain' do @@ -28,9 +26,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end - context 'when pipeline ref does not exist' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'something', project: project) + context 'when ref does not exist' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'something') end it 'breaks the chain' do @@ -43,9 +42,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end - context 'when pipeline does not have SHA set' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'master', sha: nil, project: project) + context 'when does not have existing SHA set' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'master', checkout_sha: 'something') end it 'breaks the chain' do diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index d72f8553f55..98880fe9f28 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -178,15 +178,29 @@ module Gitlab end context 'when kubernetes is active' do - let(:project) { create(:kubernetes_project) } - let(:pipeline) { create(:ci_empty_pipeline, project: project) } + shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do + it 'returns seeds for kubernetes dependent job' do + seeds = subject.stage_seeds(pipeline) - it 'returns seeds for kubernetes dependent job' do - seeds = subject.stage_seeds(pipeline) + expect(seeds.size).to eq 2 + expect(seeds.first.builds.dig(0, :name)).to eq 'spinach' + expect(seeds.second.builds.dig(0, :name)).to eq 'production' + end + end - expect(seeds.size).to eq 2 - expect(seeds.first.builds.dig(0, :name)).to eq 'spinach' - expect(seeds.second.builds.dig(0, :name)).to eq 'production' + context 'when user configured kubernetes from Integration > Kubernetes' do + let(:project) { create(:kubernetes_project) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } + + it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' + end + + context 'when user configured kubernetes from CI/CD > Clusters' do + let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } + + it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' end end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 3c8350b3aad..664ba0f7234 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -942,8 +942,8 @@ describe Gitlab::Database::MigrationHelpers do end it 'queues jobs in groups of buffer size 1' do - expect(BackgroundMigrationWorker).to receive(:perform_bulk).with([['FooJob', [id1, id2]]]) - expect(BackgroundMigrationWorker).to receive(:perform_bulk).with([['FooJob', [id3, id3]]]) + expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id1, id2]]]) + expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id3, id3]]]) model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) end @@ -960,8 +960,8 @@ describe Gitlab::Database::MigrationHelpers do end it 'queues jobs in bulk all at once (big buffer size)' do - expect(BackgroundMigrationWorker).to receive(:perform_bulk).with([['FooJob', [id1, id2]], - ['FooJob', [id3, id3]]]) + expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id1, id2]], + ['FooJob', [id3, id3]]]) model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) end diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index fcddfad3f9f..b2f13fae73f 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -73,6 +73,28 @@ describe Gitlab::Database do end end + describe '.replication_slots_supported?' do + it 'returns false when using MySQL' do + allow(described_class).to receive(:postgresql?).and_return(false) + + expect(described_class.replication_slots_supported?).to eq(false) + end + + it 'returns false when using PostgreSQL 9.3' do + allow(described_class).to receive(:postgresql?).and_return(true) + allow(described_class).to receive(:version).and_return('9.3.1') + + expect(described_class.replication_slots_supported?).to eq(false) + end + + it 'returns true when using PostgreSQL 9.4.0 or newer' do + allow(described_class).to receive(:postgresql?).and_return(true) + allow(described_class).to receive(:version).and_return('9.4.0') + + expect(described_class.replication_slots_supported?).to eq(true) + end + end + describe '.nulls_last_order' do context 'when using PostgreSQL' do before do @@ -199,6 +221,22 @@ describe Gitlab::Database do described_class.bulk_insert('test', rows) end + it 'does not quote values of a column in the disable_quote option' do + [1, 2, 4, 5].each do |i| + expect(connection).to receive(:quote).with(i) + end + + described_class.bulk_insert('test', rows, disable_quote: :c) + end + + it 'does not quote values of columns in the disable_quote option' do + [2, 5].each do |i| + expect(connection).to receive(:quote).with(i) + end + + described_class.bulk_insert('test', rows, disable_quote: [:a, :c]) + end + it 'handles non-UTF-8 data' do expect { described_class.bulk_insert('test', [{ a: "\255" }]) }.not_to raise_error end diff --git a/spec/lib/gitlab/diff/inline_diff_spec.rb b/spec/lib/gitlab/diff/inline_diff_spec.rb index 15451c2cf99..0a41362f606 100644 --- a/spec/lib/gitlab/diff/inline_diff_spec.rb +++ b/spec/lib/gitlab/diff/inline_diff_spec.rb @@ -31,6 +31,10 @@ describe Gitlab::Diff::InlineDiff do expect(subject[7]).to eq([17..17]) expect(subject[8]).to be_nil end + + it 'can handle unchanged empty lines' do + expect { described_class.for_lines(['- bar', '+ baz', '']) }.not_to raise_error + end end describe "#inline_diffs" do diff --git a/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb b/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb new file mode 100644 index 00000000000..dc1a93367a4 --- /dev/null +++ b/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb @@ -0,0 +1,96 @@ +require 'spec_helper' +require_relative '../email_shared_blocks' + +describe Gitlab::Email::Handler::CreateMergeRequestHandler do + include_context :email_shared_context + it_behaves_like :reply_processing_shared_examples + + before do + stub_incoming_email_setting(enabled: true, address: "incoming+%{key}@appmail.adventuretime.ooo") + stub_config_setting(host: 'localhost') + end + + after do + TestEnv.clean_test_path + end + + let(:email_raw) { fixture_file('emails/valid_new_merge_request.eml') } + let(:namespace) { create(:namespace, path: 'gitlabhq') } + + let!(:project) { create(:project, :public, :repository, namespace: namespace, path: 'gitlabhq') } + let!(:user) do + create( + :user, + email: 'jake@adventuretime.ooo', + incoming_email_token: 'auth_token' + ) + end + + context "as a non-developer" do + before do + project.add_guest(user) + end + + it "raises UserNotAuthorizedError if the user is not a member" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError) + end + end + + context "as a developer" do + before do + project.add_developer(user) + end + + context "when everything is fine" do + it "creates a new merge request" do + expect { receiver.execute }.to change { project.merge_requests.count }.by(1) + merge_request = project.merge_requests.last + + expect(merge_request.author).to eq(user) + expect(merge_request.source_branch).to eq('feature') + expect(merge_request.title).to eq('Feature added') + expect(merge_request.description).to eq('Merge request description') + expect(merge_request.target_branch).to eq(project.default_branch) + end + end + + context "something is wrong" do + context "when the merge request could not be saved" do + before do + allow_any_instance_of(MergeRequest).to receive(:save).and_return(false) + end + + it "raises an InvalidMergeRequestError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidMergeRequestError) + end + end + + context "when we can't find the incoming_email_token" do + let(:email_raw) { fixture_file("emails/wrong_incoming_email_token.eml") } + + it "raises an UserNotFoundError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError) + end + end + + context "when the subject is blank" do + let(:email_raw) { fixture_file("emails/valid_new_merge_request_no_subject.eml") } + + it "raises an InvalidMergeRequestError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidMergeRequestError) + end + end + + context "when the message body is blank" do + let(:email_raw) { fixture_file("emails/valid_new_merge_request_no_description.eml") } + + it "creates a new merge request with description set from the last commit" do + expect { receiver.execute }.to change { project.merge_requests.count }.by(1) + merge_request = project.merge_requests.last + + expect(merge_request.description).to eq('Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>') + end + end + end + end +end diff --git a/spec/lib/gitlab/email/handler_spec.rb b/spec/lib/gitlab/email/handler_spec.rb new file mode 100644 index 00000000000..650b01c4df4 --- /dev/null +++ b/spec/lib/gitlab/email/handler_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe Gitlab::Email::Handler do + describe '.for' do + it 'picks issue handler if there is not merge request prefix' do + expect(described_class.for('email', 'project+key')).to be_an_instance_of(Gitlab::Email::Handler::CreateIssueHandler) + end + + it 'picks merge request handler if there is merge request key' do + expect(described_class.for('email', 'project+merge-request+key')).to be_an_instance_of(Gitlab::Email::Handler::CreateMergeRequestHandler) + end + + it 'returns nil if no handler is found' do + expect(described_class.for('email', '')).to be_nil + end + end +end diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 9f4e3c49adc..5ed639543e0 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -278,6 +278,35 @@ describe Gitlab::Git::Commit, seed_helper: true do it { is_expected.not_to include(SeedRepo::FirstCommit::ID) } end + shared_examples '.shas_with_signatures' do + let(:signed_shas) { %w[5937ac0a7beb003549fc5fd26fc247adbce4a52e 570e7b2abdd848b95f2f578043fc23bd6f6fd24d] } + let(:unsigned_shas) { %w[19e2e9b4ef76b422ce1154af39a91323ccc57434 c642fe9b8b9f28f9225d7ea953fe14e74748d53b] } + let(:first_signed_shas) { %w[5937ac0a7beb003549fc5fd26fc247adbce4a52e c642fe9b8b9f28f9225d7ea953fe14e74748d53b] } + + it 'has 2 signed shas' do + ret = described_class.shas_with_signatures(repository, signed_shas) + expect(ret).to eq(signed_shas) + end + + it 'has 0 signed shas' do + ret = described_class.shas_with_signatures(repository, unsigned_shas) + expect(ret).to eq([]) + end + + it 'has 1 signed sha' do + ret = described_class.shas_with_signatures(repository, first_signed_shas) + expect(ret).to contain_exactly(first_signed_shas.first) + end + end + + describe '.shas_with_signatures with gitaly on' do + it_should_behave_like '.shas_with_signatures' + end + + describe '.shas_with_signatures with gitaly disabled', :disable_gitaly do + it_should_behave_like '.shas_with_signatures' + end + describe '.find_all' do shared_examples 'finding all commits' do it 'should return a return a collection of commits' do diff --git a/spec/lib/gitlab/git/remote_repository_spec.rb b/spec/lib/gitlab/git/remote_repository_spec.rb index 0506210887c..eb148cc3804 100644 --- a/spec/lib/gitlab/git/remote_repository_spec.rb +++ b/spec/lib/gitlab/git/remote_repository_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::Git::RemoteRepository, seed_helper: true do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } subject { described_class.new(repository) } - describe '#empty_repo?' do + describe '#empty?' do using RSpec::Parameterized::TableSyntax where(:repository, :result) do @@ -13,7 +13,7 @@ describe Gitlab::Git::RemoteRepository, seed_helper: true do end with_them do - it { expect(subject.empty_repo?).to eq(result) } + it { expect(subject.empty?).to eq(result) } end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 2f49bd1bcf2..e6845420f7d 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -257,7 +257,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#empty?' do - it { expect(repository.empty?).to be_falsey } + it { expect(repository).not_to be_empty } end describe '#ref_names' do @@ -588,12 +588,12 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe '#fetch_as_mirror_without_shell' do + describe '#fetch_repository_as_mirror' do let(:new_repository) do Gitlab::Git::Repository.new('default', 'my_project.git', '') end - subject { new_repository.fetch_as_mirror_without_shell(repository.path) } + subject { new_repository.fetch_repository_as_mirror(repository) } before do Gitlab::Shell.new.add_repository('default', 'my_project') @@ -603,7 +603,7 @@ describe Gitlab::Git::Repository, seed_helper: true do Gitlab::Shell.new.remove_repository(TestEnv.repos_path, 'my_project') end - it 'fetches a url as a mirror remote' do + it 'fetches a repository as a mirror remote' do subject expect(refs(new_repository.path)).to eq(refs(repository.path)) @@ -1210,6 +1210,16 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + context 'when no root ref is available' do + it 'returns empty list' do + project = create(:project, :empty_repo) + + names = project.repository.merged_branch_names(%w[feature]) + + expect(names).to be_empty + end + end + context 'when no branch names are specified' do before do repository.create_branch('identical', 'master') @@ -1652,21 +1662,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe '#fetch_remote_without_shell' do - let(:git_path) { Gitlab.config.git.bin_path } - let(:remote_name) { 'my_remote' } - - subject { repository.fetch_remote_without_shell(remote_name) } - - it 'fetches the remote and returns true if the command was successful' do - expect(repository).to receive(:popen) - .with(%W(#{git_path} fetch #{remote_name}), repository.path, {}) - .and_return(['', 0]) - - expect(subject).to be(true) - end - end - describe '#merge' do let(:repository) do Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') diff --git a/spec/lib/gitlab/git/storage/checker_spec.rb b/spec/lib/gitlab/git/storage/checker_spec.rb new file mode 100644 index 00000000000..d74c3bcb04c --- /dev/null +++ b/spec/lib/gitlab/git/storage/checker_spec.rb @@ -0,0 +1,132 @@ +require 'spec_helper' + +describe Gitlab::Git::Storage::Checker, :clean_gitlab_redis_shared_state do + let(:storage_name) { 'default' } + let(:hostname) { Gitlab::Environment.hostname } + let(:cache_key) { "storage_accessible:#{storage_name}:#{hostname}" } + + subject(:checker) { described_class.new(storage_name) } + + def value_from_redis(name) + Gitlab::Git::Storage.redis.with do |redis| + redis.hmget(cache_key, name) + end.first + end + + def set_in_redis(name, value) + Gitlab::Git::Storage.redis.with do |redis| + redis.hmset(cache_key, name, value) + end.first + end + + describe '.check_all' do + it 'calls a check for each storage' do + fake_checker_default = double + fake_checker_broken = double + fake_logger = fake_logger + + expect(described_class).to receive(:new).with('default', fake_logger) { fake_checker_default } + expect(described_class).to receive(:new).with('broken', fake_logger) { fake_checker_broken } + expect(fake_checker_default).to receive(:check_with_lease) + expect(fake_checker_broken).to receive(:check_with_lease) + + described_class.check_all(fake_logger) + end + + context 'with broken storage', :broken_storage do + it 'returns the results' do + expected_result = [ + { storage: 'default', success: true }, + { storage: 'broken', success: false } + ] + + expect(described_class.check_all).to eq(expected_result) + end + end + end + + describe '#initialize' do + it 'assigns the settings' do + expect(checker.hostname).to eq(hostname) + expect(checker.storage).to eq('default') + expect(checker.storage_path).to eq(TestEnv.repos_path) + end + end + + describe '#check_with_lease' do + it 'only allows one check at a time' do + expect(checker).to receive(:check).once { sleep 1 } + + thread = Thread.new { checker.check_with_lease } + checker.check_with_lease + thread.join + end + + it 'returns a result hash' do + expect(checker.check_with_lease).to eq(storage: 'default', success: true) + end + end + + describe '#check' do + it 'tracks that the storage was accessible' do + set_in_redis(:failure_count, 10) + set_in_redis(:last_failure, Time.now.to_f) + + checker.check + + expect(value_from_redis(:failure_count).to_i).to eq(0) + expect(value_from_redis(:last_failure)).to be_empty + expect(value_from_redis(:first_failure)).to be_empty + end + + it 'calls the check with the correct arguments' do + stub_application_setting(circuitbreaker_storage_timeout: 30, + circuitbreaker_access_retries: 3) + + expect(Gitlab::Git::Storage::ForkedStorageCheck) + .to receive(:storage_available?).with(TestEnv.repos_path, 30, 3) + .and_call_original + + checker.check + end + + it 'returns `true`' do + expect(checker.check).to eq(true) + end + + it 'maintains known storage keys' do + Timecop.freeze do + # Insert an old key to expire + old_entry = Time.now.to_i - 3.days.to_i + Gitlab::Git::Storage.redis.with do |redis| + redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, old_entry, 'to_be_removed') + end + + checker.check + + known_keys = Gitlab::Git::Storage.redis.with do |redis| + redis.zrange(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, -1) + end + + expect(known_keys).to contain_exactly(cache_key) + end + end + + context 'the storage is not available', :broken_storage do + let(:storage_name) { 'broken' } + + it 'tracks that the storage was inaccessible' do + Timecop.freeze do + expect { checker.check }.to change { value_from_redis(:failure_count).to_i }.by(1) + + expect(value_from_redis(:last_failure)).not_to be_empty + expect(value_from_redis(:first_failure)).not_to be_empty + end + end + + it 'returns `false`' do + expect(checker.check).to eq(false) + end + end + end +end diff --git a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb index 72dabca793a..210b90bfba9 100644 --- a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb +++ b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb @@ -1,11 +1,18 @@ require 'spec_helper' -describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: true, broken_storage: true do +describe Gitlab::Git::Storage::CircuitBreaker, :broken_storage do let(:storage_name) { 'default' } let(:circuit_breaker) { described_class.new(storage_name, hostname) } let(:hostname) { Gitlab::Environment.hostname } let(:cache_key) { "storage_accessible:#{storage_name}:#{hostname}" } + def set_in_redis(name, value) + Gitlab::Git::Storage.redis.with do |redis| + redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, cache_key) + redis.hmset(cache_key, name, value) + end.first + end + before do # Override test-settings for the circuitbreaker with something more realistic # for these specs. @@ -19,35 +26,7 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: ) end - def value_from_redis(name) - Gitlab::Git::Storage.redis.with do |redis| - redis.hmget(cache_key, name) - end.first - end - - def set_in_redis(name, value) - Gitlab::Git::Storage.redis.with do |redis| - redis.hmset(cache_key, name, value) - end.first - end - - describe '.reset_all!' do - it 'clears all entries form redis' do - set_in_redis(:failure_count, 10) - - described_class.reset_all! - - key_exists = Gitlab::Git::Storage.redis.with { |redis| redis.exists(cache_key) } - - expect(key_exists).to be_falsey - end - - it 'does not break when there are no keys in redis' do - expect { described_class.reset_all! }.not_to raise_error - end - end - - describe '.for_storage' do + describe '.for_storage', :request_store do it 'only builds a single circuitbreaker per storage' do expect(described_class).to receive(:new).once.and_call_original @@ -70,7 +49,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: it 'assigns the settings' do expect(circuit_breaker.hostname).to eq(hostname) expect(circuit_breaker.storage).to eq('default') - expect(circuit_breaker.storage_path).to eq(TestEnv.repos_path) end end @@ -90,9 +68,9 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: end end - describe '#failure_wait_time' do + describe '#check_interval' do it 'reads the value from settings' do - expect(circuit_breaker.failure_wait_time).to eq(1) + expect(circuit_breaker.check_interval).to eq(1) end end @@ -113,12 +91,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: expect(circuit_breaker.access_retries).to eq(4) end end - - describe '#backoff_threshold' do - it 'reads the value from settings' do - expect(circuit_breaker.backoff_threshold).to eq(5) - end - end end describe '#perform' do @@ -133,19 +105,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: end end - it 'raises the correct exception when backing off' do - Timecop.freeze do - set_in_redis(:last_failure, 1.second.ago.to_f) - set_in_redis(:failure_count, 90) - - expect { |b| circuit_breaker.perform(&b) } - .to raise_error do |exception| - expect(exception).to be_kind_of(Gitlab::Git::Storage::Failing) - expect(exception.retry_after).to eq(30) - end - end - end - it 'yields the block' do expect { |b| circuit_breaker.perform(&b) } .to yield_control @@ -169,36 +128,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: .to raise_error(Rugged::OSError) end - it 'tracks that the storage was accessible' do - set_in_redis(:failure_count, 10) - set_in_redis(:last_failure, Time.now.to_f) - - circuit_breaker.perform { '' } - - expect(value_from_redis(:failure_count).to_i).to eq(0) - expect(value_from_redis(:last_failure)).to be_empty - expect(circuit_breaker.failure_count).to eq(0) - expect(circuit_breaker.last_failure).to be_nil - end - - it 'only performs the accessibility check once' do - expect(Gitlab::Git::Storage::ForkedStorageCheck) - .to receive(:storage_available?).once.and_call_original - - 2.times { circuit_breaker.perform { '' } } - end - - it 'calls the check with the correct arguments' do - stub_application_setting(circuitbreaker_storage_timeout: 30, - circuitbreaker_access_retries: 3) - - expect(Gitlab::Git::Storage::ForkedStorageCheck) - .to receive(:storage_available?).with(TestEnv.repos_path, 30, 3) - .and_call_original - - circuit_breaker.perform { '' } - end - context 'with the feature disabled' do before do stub_feature_flags(git_storage_circuit_breaker: false) @@ -221,31 +150,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: expect(result).to eq('hello') end end - - context 'the storage is not available' do - let(:storage_name) { 'broken' } - - it 'raises the correct exception' do - expect(circuit_breaker).to receive(:track_storage_inaccessible) - - expect { circuit_breaker.perform { '' } } - .to raise_error do |exception| - expect(exception).to be_kind_of(Gitlab::Git::Storage::Inaccessible) - expect(exception.retry_after).to eq(30) - end - end - - it 'tracks that the storage was inaccessible' do - Timecop.freeze do - expect { circuit_breaker.perform { '' } }.to raise_error(Gitlab::Git::Storage::Inaccessible) - - expect(value_from_redis(:failure_count).to_i).to eq(1) - expect(value_from_redis(:last_failure)).not_to be_empty - expect(circuit_breaker.failure_count).to eq(1) - expect(circuit_breaker.last_failure).to be_within(1.second).of(Time.now) - end - end - end end describe '#circuit_broken?' do @@ -264,32 +168,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: end end - describe '#backing_off?' do - it 'is true when there was a recent failure' do - Timecop.freeze do - set_in_redis(:last_failure, 1.second.ago.to_f) - set_in_redis(:failure_count, 90) - - expect(circuit_breaker.backing_off?).to be_truthy - end - end - - context 'the `failure_wait_time` is set to 0' do - before do - stub_application_setting(circuitbreaker_failure_wait_time: 0) - end - - it 'is working even when there are failures' do - Timecop.freeze do - set_in_redis(:last_failure, 0.seconds.ago.to_f) - set_in_redis(:failure_count, 90) - - expect(circuit_breaker.backing_off?).to be_falsey - end - end - end - end - describe '#last_failure' do it 'returns the last failure time' do time = Time.parse("2017-05-26 17:52:30") diff --git a/spec/lib/gitlab/git/storage/failure_info_spec.rb b/spec/lib/gitlab/git/storage/failure_info_spec.rb new file mode 100644 index 00000000000..bae88fdda86 --- /dev/null +++ b/spec/lib/gitlab/git/storage/failure_info_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe Gitlab::Git::Storage::FailureInfo, :broken_storage do + let(:storage_name) { 'default' } + let(:hostname) { Gitlab::Environment.hostname } + let(:cache_key) { "storage_accessible:#{storage_name}:#{hostname}" } + + def value_from_redis(name) + Gitlab::Git::Storage.redis.with do |redis| + redis.hmget(cache_key, name) + end.first + end + + def set_in_redis(name, value) + Gitlab::Git::Storage.redis.with do |redis| + redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, cache_key) + redis.hmset(cache_key, name, value) + end.first + end + + describe '.reset_all!' do + it 'clears all entries form redis' do + set_in_redis(:failure_count, 10) + + described_class.reset_all! + + key_exists = Gitlab::Git::Storage.redis.with { |redis| redis.exists(cache_key) } + + expect(key_exists).to be_falsey + end + + it 'does not break when there are no keys in redis' do + expect { described_class.reset_all! }.not_to raise_error + end + end + + describe '.load' do + it 'loads failure information for a storage on a host' do + first_failure = Time.parse("2017-11-14 17:52:30") + last_failure = Time.parse("2017-11-14 18:54:37") + failure_count = 11 + + set_in_redis(:first_failure, first_failure.to_i) + set_in_redis(:last_failure, last_failure.to_i) + set_in_redis(:failure_count, failure_count.to_i) + + info = described_class.load(cache_key) + + expect(info.first_failure).to eq(first_failure) + expect(info.last_failure).to eq(last_failure) + expect(info.failure_count).to eq(failure_count) + end + end + + describe '#no_failures?' do + it 'is true when there are no failures' do + info = described_class.new(nil, nil, 0) + + expect(info.no_failures?).to be_truthy + end + + it 'is false when there are failures' do + info = described_class.new(Time.parse("2017-11-14 17:52:30"), + Time.parse("2017-11-14 18:54:37"), + 20) + + expect(info.no_failures?).to be_falsy + end + end +end diff --git a/spec/lib/gitlab/git/storage/health_spec.rb b/spec/lib/gitlab/git/storage/health_spec.rb index 4a14a5201d1..bb670fc5d94 100644 --- a/spec/lib/gitlab/git/storage/health_spec.rb +++ b/spec/lib/gitlab/git/storage/health_spec.rb @@ -1,11 +1,12 @@ require 'spec_helper' -describe Gitlab::Git::Storage::Health, clean_gitlab_redis_shared_state: true, broken_storage: true do +describe Gitlab::Git::Storage::Health, broken_storage: true do let(:host1_key) { 'storage_accessible:broken:web01' } let(:host2_key) { 'storage_accessible:default:kiq01' } def set_in_redis(cache_key, value) Gitlab::Git::Storage.redis.with do |redis| + redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, cache_key) redis.hmset(cache_key, :failure_count, value) end.first end diff --git a/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb b/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb index 5db37f55e03..93ad20011de 100644 --- a/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb +++ b/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb @@ -27,7 +27,7 @@ describe Gitlab::Git::Storage::NullCircuitBreaker do end describe '#failure_info' do - it { Timecop.freeze { expect(breaker.failure_info).to eq(Gitlab::Git::Storage::CircuitBreaker::FailureInfo.new(Time.now, breaker.failure_count_threshold)) } } + it { expect(breaker.failure_info.no_failures?).to be_falsy } end end @@ -49,7 +49,7 @@ describe Gitlab::Git::Storage::NullCircuitBreaker do end describe '#failure_info' do - it { expect(breaker.failure_info).to eq(Gitlab::Git::Storage::CircuitBreaker::FailureInfo.new(nil, 0)) } + it { expect(breaker.failure_info.no_failures?).to be_truthy } end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index c9643c5da47..2db560c2cec 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -193,7 +193,15 @@ describe Gitlab::GitAccess do let(:actor) { build(:rsa_deploy_key_2048, user: user) } end - describe '#check_project_moved!' do + shared_examples 'check_project_moved' do + it 'enqueues a redirected message' do + push_access_check + + expect(Gitlab::Checks::ProjectMoved.fetch_redirect_message(user.id, project.id)).not_to be_nil + end + end + + describe '#check_project_moved!', :clean_gitlab_redis_shared_state do before do project.add_master(user) end @@ -207,7 +215,40 @@ describe Gitlab::GitAccess do end end - context 'when a redirect was followed to find the project' do + context 'when a permanent redirect and ssh protocol' do + let(:redirected_path) { 'some/other-path' } + + before do + allow_any_instance_of(Gitlab::Checks::ProjectMoved).to receive(:permanent_redirect?).and_return(true) + end + + it 'allows push and pull access' do + aggregate_failures do + expect { push_access_check }.not_to raise_error + end + end + + it_behaves_like 'check_project_moved' + end + + context 'with a permanent redirect and http protocol' do + let(:redirected_path) { 'some/other-path' } + let(:protocol) { 'http' } + + before do + allow_any_instance_of(Gitlab::Checks::ProjectMoved).to receive(:permanent_redirect?).and_return(true) + end + + it 'allows_push and pull access' do + aggregate_failures do + expect { push_access_check }.not_to raise_error + end + end + + it_behaves_like 'check_project_moved' + end + + context 'with a temporal redirect and ssh protocol' do let(:redirected_path) { 'some/other-path' } it 'blocks push and pull access' do @@ -219,16 +260,15 @@ describe Gitlab::GitAccess do expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.ssh_url_to_repo}/) end end + end - context 'http protocol' do - let(:protocol) { 'http' } + context 'with a temporal redirect and http protocol' do + let(:redirected_path) { 'some/other-path' } + let(:protocol) { 'http' } - it 'includes the path to the project using HTTP' do - aggregate_failures do - expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) - expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) - end - end + it 'does not allow to push and pull access' do + expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) + expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) end end end diff --git a/spec/lib/gitlab/git_spec.rb b/spec/lib/gitlab/git_spec.rb index 494dfe0e595..ce15057dd7d 100644 --- a/spec/lib/gitlab/git_spec.rb +++ b/spec/lib/gitlab/git_spec.rb @@ -38,4 +38,29 @@ describe Gitlab::Git do expect(described_class.ref_name(utf8_invalid_ref)).to eq("an_invalid_ref_å") end end + + describe '.shas_eql?' do + using RSpec::Parameterized::TableSyntax + + where(:sha1, :sha2, :result) do + sha = RepoHelpers.sample_commit.id + short_sha = sha[0, Gitlab::Git::Commit::MIN_SHA_LENGTH] + too_short_sha = sha[0, Gitlab::Git::Commit::MIN_SHA_LENGTH - 1] + + [ + [sha, sha, true], + [sha, short_sha, true], + [sha, sha.reverse, false], + [sha, too_short_sha, false], + [sha, nil, false] + ] + end + + with_them do + it { expect(described_class.shas_eql?(sha1, sha2)).to eq(result) } + it 'is commutative' do + expect(described_class.shas_eql?(sha2, sha1)).to eq(result) + end + end + end end diff --git a/spec/lib/gitlab/identifier_spec.rb b/spec/lib/gitlab/identifier_spec.rb index cfaeb1f0d4f..0385dd762c2 100644 --- a/spec/lib/gitlab/identifier_spec.rb +++ b/spec/lib/gitlab/identifier_spec.rb @@ -70,6 +70,10 @@ describe Gitlab::Identifier do expect(identifier.identify_using_commit(project, '123')).to eq(user) end end + + it 'returns nil if the project & ref are not present' do + expect(identifier.identify_using_commit(nil, nil)).to be_nil + end end describe '#identify_using_user' do diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index 5341addf911..78767d06462 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -20,9 +20,39 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is enabled' do before do + allow(Feature.get(:prometheus_metrics_method_instrumentation)).to receive(:enabled?).and_call_original + described_class.measurement_enabled_cache_expires_at.value = Time.now.to_i - 1 Feature.get(:prometheus_metrics_method_instrumentation).enable end + around do |example| + Timecop.freeze do + example.run + end + end + + it 'caches subsequent invocations of feature check' do + 10.times do + method_call.measure { 'foo' } + end + + expect(Feature.get(:prometheus_metrics_method_instrumentation)).to have_received(:enabled?).once + end + + it 'expires feature check cache after 1 minute' do + method_call.measure { 'foo' } + + Timecop.travel(1.minute.from_now) do + method_call.measure { 'foo' } + end + + Timecop.travel(1.minute.from_now + 1.second) do + method_call.measure { 'foo' } + end + + expect(Feature.get(:prometheus_metrics_method_instrumentation)).to have_received(:enabled?).twice + end + it 'observes the performance of the supplied block' do expect(described_class.call_duration_histogram) .to receive(:observe) @@ -34,6 +64,8 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is disabled' do before do + described_class.measurement_enabled_cache_expires_at.value = Time.now.to_i - 1 + Feature.get(:prometheus_metrics_method_instrumentation).disable end diff --git a/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb index 667e4747897..f66451c5188 100644 --- a/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb @@ -21,7 +21,6 @@ describe Gitlab::Metrics::Samplers::InfluxSampler do it 'samples various statistics' do expect(sampler).to receive(:sample_memory_usage) expect(sampler).to receive(:sample_file_descriptors) - expect(sampler).to receive(:sample_objects) expect(sampler).to receive(:sample_gc) expect(sampler).to receive(:flush) @@ -72,28 +71,6 @@ describe Gitlab::Metrics::Samplers::InfluxSampler do end end - if Gitlab::Metrics.mri? - describe '#sample_objects' do - it 'adds a metric containing the amount of allocated objects' do - expect(sampler).to receive(:add_metric) - .with(/object_counts/, an_instance_of(Hash), an_instance_of(Hash)) - .at_least(:once) - .and_call_original - - sampler.sample_objects - end - - it 'ignores classes without a name' do - expect(Allocations).to receive(:to_hash).and_return({ Class.new => 4 }) - - expect(sampler).not_to receive(:add_metric) - .with('object_counts', an_instance_of(Hash), type: nil) - - sampler.sample_objects - end - end - end - describe '#sample_gc' do it 'adds a metric containing garbage collection statistics' do expect(GC::Profiler).to receive(:total_time).and_return(0.24) diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index 53699327da1..375cbf8a9ca 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -11,7 +11,6 @@ describe Gitlab::Metrics::Samplers::RubySampler do it 'samples various statistics' do expect(Gitlab::Metrics::System).to receive(:memory_usage) expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) - expect(sampler).to receive(:sample_objects) expect(sampler).to receive(:sample_gc) sampler.sample @@ -65,26 +64,4 @@ describe Gitlab::Metrics::Samplers::RubySampler do sampler.sample end end - - if Gitlab::Metrics.mri? - describe '#sample_objects' do - it 'adds a metric containing the amount of allocated objects' do - expect(sampler.metrics[:objects_total]).to receive(:set) - .with(include(class: anything), be > 0) - .at_least(:once) - .and_call_original - - sampler.sample - end - - it 'ignores classes without a name' do - expect(Allocations).to receive(:to_hash).and_return({ Class.new => 4 }) - - expect(sampler.metrics[:objects_total]).not_to receive(:set) - .with(include(class: 'object_counts'), anything) - - sampler.sample - end - end - end end diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index 9c3e7d7e9ba..a424f0f5cfe 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -70,6 +70,15 @@ describe Gitlab::ProjectSearchResults do subject { described_class.parse_search_result(search_result) } + it 'can correctly parse filenames including ":"' do + special_char_result = "\nmaster:testdata/project::function1.yaml-1----\nmaster:testdata/project::function1.yaml:2:test: data1\n" + + blob = described_class.parse_search_result(special_char_result) + + expect(blob.ref).to eq('master') + expect(blob.filename).to eq('testdata/project::function1.yaml') + end + it "returns a valid FoundBlob" do is_expected.to be_an Gitlab::SearchResults::FoundBlob expect(subject.id).to be_nil diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 476a3f1998d..8ec3f55e6de 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -115,6 +115,15 @@ describe Gitlab::ReferenceExtractor do end end + it 'does not include anchors from table of contents in issue references' do + issue1 = create(:issue, project: project) + issue2 = create(:issue, project: project) + + subject.analyze("not real issue <h4>#{issue1.iid}</h4>, real issue #{issue2.to_reference}") + + expect(subject.issues).to match_array([issue2]) + end + it 'accesses valid issue objects' do @i0 = create(:issue, project: project) @i1 = create(:issue, project: project) @@ -250,4 +259,34 @@ describe Gitlab::ReferenceExtractor do subject { described_class.references_pattern } it { is_expected.to be_kind_of Regexp } end + + describe 'referables prefixes' do + def prefixes + described_class::REFERABLES.each_with_object({}) do |referable, result| + klass = referable.to_s.camelize.constantize + + next unless klass.respond_to?(:reference_prefix) + + prefix = klass.reference_prefix + result[prefix] ||= [] + result[prefix] << referable + end + end + + it 'returns all supported prefixes' do + expect(prefixes.keys.uniq).to match_array(%w(@ # ~ % ! $ &)) + end + + it 'does not allow one prefix for multiple referables if not allowed specificly' do + # make sure you are not overriding existing prefix before changing this hash + multiple_allowed = { + '@' => 3 + } + + prefixes.each do |prefix, referables| + expected_count = multiple_allowed[prefix] || 1 + expect(referables.count).to eq(expected_count) + end + end + end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 2158b2837e2..eec6858a5de 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -200,18 +200,18 @@ describe Gitlab::Shell do describe '#fork_repository' do it 'returns true when the command succeeds' do expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'fork-project', 'current/storage', 'project/path.git', 'new/storage', 'new-namespace'], + .with([projects_path, 'fork-repository', 'current/storage', 'project/path.git', 'new/storage', 'fork/path.git'], nil, popen_vars).and_return([nil, 0]) - expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'new-namespace')).to be true + expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'fork/path')).to be true end it 'return false when the command fails' do expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'fork-project', 'current/storage', 'project/path.git', 'new/storage', 'new-namespace'], + .with([projects_path, 'fork-repository', 'current/storage', 'project/path.git', 'new/storage', 'fork/path.git'], nil, popen_vars).and_return(["error", 1]) - expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'new-namespace')).to be false + expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'fork/path')).to be false end end diff --git a/spec/lib/gitlab/sidekiq_config_spec.rb b/spec/lib/gitlab/sidekiq_config_spec.rb new file mode 100644 index 00000000000..0c66d764851 --- /dev/null +++ b/spec/lib/gitlab/sidekiq_config_spec.rb @@ -0,0 +1,45 @@ +require 'rails_helper' + +describe Gitlab::SidekiqConfig do + describe '.workers' do + it 'includes all workers' do + workers = described_class.workers + + expect(workers).to include(PostReceive) + expect(workers).to include(MergeWorker) + end + end + + describe '.worker_queues' do + it 'includes all queues' do + queues = described_class.worker_queues + + expect(queues).to include('post_receive') + expect(queues).to include('merge') + expect(queues).to include('cronjob:stuck_import_jobs') + expect(queues).to include('mailers') + expect(queues).to include('default') + end + end + + describe '.expand_queues' do + it 'expands queue namespaces to concrete queue names' do + queues = described_class.expand_queues(%w[cronjob]) + + expect(queues).to include('cronjob:stuck_import_jobs') + expect(queues).to include('cronjob:stuck_merge_jobs') + end + + it 'lets concrete queue names pass through' do + queues = described_class.expand_queues(%w[post_receive]) + + expect(queues).to include('post_receive') + end + + it 'lets unknown queues pass through' do + queues = described_class.expand_queues(%w[unknown]) + + expect(queues).to include('unknown') + end + end +end diff --git a/spec/lib/gitlab/sidekiq_versioning/manager_spec.rb b/spec/lib/gitlab/sidekiq_versioning/manager_spec.rb new file mode 100644 index 00000000000..7debf70a16f --- /dev/null +++ b/spec/lib/gitlab/sidekiq_versioning/manager_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Gitlab::SidekiqVersioning::Manager do + before do + Sidekiq::Manager.prepend described_class + end + + describe '#initialize' do + it 'listens on all expanded queues' do + manager = Sidekiq::Manager.new(queues: %w[post_receive repository_fork cronjob unknown]) + + queues = manager.options[:queues] + + expect(queues).to include('post_receive') + expect(queues).to include('repository_fork') + expect(queues).to include('cronjob') + expect(queues).to include('cronjob:stuck_import_jobs') + expect(queues).to include('cronjob:stuck_merge_jobs') + expect(queues).to include('unknown') + end + end +end diff --git a/spec/lib/gitlab/sidekiq_versioning_spec.rb b/spec/lib/gitlab/sidekiq_versioning_spec.rb new file mode 100644 index 00000000000..fa6d42e730d --- /dev/null +++ b/spec/lib/gitlab/sidekiq_versioning_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe Gitlab::SidekiqVersioning, :sidekiq, :redis do + let(:foo_worker) do + Class.new do + def self.name + 'FooWorker' + end + + include ApplicationWorker + end + end + + let(:bar_worker) do + Class.new do + def self.name + 'BarWorker' + end + + include ApplicationWorker + end + end + + before do + allow(Gitlab::SidekiqConfig).to receive(:workers).and_return([foo_worker, bar_worker]) + allow(Gitlab::SidekiqConfig).to receive(:worker_queues).and_return([foo_worker.queue, bar_worker.queue]) + end + + describe '.install!' do + it 'prepends SidekiqVersioning::Manager into Sidekiq::Manager' do + described_class.install! + + expect(Sidekiq::Manager).to include(Gitlab::SidekiqVersioning::Manager) + end + + it 'registers all versionless and versioned queues with Redis' do + described_class.install! + + queues = Sidekiq::Queue.all.map(&:name) + expect(queues).to include('foo') + expect(queues).to include('bar') + end + end +end diff --git a/spec/lib/gitlab/storage_check/cli_spec.rb b/spec/lib/gitlab/storage_check/cli_spec.rb new file mode 100644 index 00000000000..6db0925899c --- /dev/null +++ b/spec/lib/gitlab/storage_check/cli_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe Gitlab::StorageCheck::CLI do + let(:options) { Gitlab::StorageCheck::Options.new('unix://tmp/socket.sock', nil, 1, false) } + subject(:runner) { described_class.new(options) } + + describe '#update_settings' do + it 'updates the interval when changed in a valid response and logs the change' do + fake_response = double + expect(fake_response).to receive(:valid?).and_return(true) + expect(fake_response).to receive(:check_interval).and_return(42) + expect(runner.logger).to receive(:info) + + runner.update_settings(fake_response) + + expect(options.interval).to eq(42) + end + end +end diff --git a/spec/lib/gitlab/storage_check/gitlab_caller_spec.rb b/spec/lib/gitlab/storage_check/gitlab_caller_spec.rb new file mode 100644 index 00000000000..d869022fd31 --- /dev/null +++ b/spec/lib/gitlab/storage_check/gitlab_caller_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe Gitlab::StorageCheck::GitlabCaller do + let(:options) { Gitlab::StorageCheck::Options.new('unix://tmp/socket.sock', nil, nil, false) } + subject(:gitlab_caller) { described_class.new(options) } + + describe '#call!' do + context 'when a socket is given' do + it 'calls a socket' do + fake_connection = double + expect(fake_connection).to receive(:post) + expect(Excon).to receive(:new).with('unix://tmp/socket.sock', socket: "tmp/socket.sock") { fake_connection } + + gitlab_caller.call! + end + end + + context 'when a host is given' do + let(:options) { Gitlab::StorageCheck::Options.new('http://localhost:8080', nil, nil, false) } + + it 'it calls a http response' do + fake_connection = double + expect(Excon).to receive(:new).with('http://localhost:8080', socket: nil) { fake_connection } + expect(fake_connection).to receive(:post) + + gitlab_caller.call! + end + end + end + + describe '#headers' do + it 'Adds the JSON header' do + headers = gitlab_caller.headers + + expect(headers['Content-Type']).to eq('application/json') + end + + context 'when a token was provided' do + let(:options) { Gitlab::StorageCheck::Options.new('unix://tmp/socket.sock', 'atoken', nil, false) } + + it 'adds it to the headers' do + expect(gitlab_caller.headers['TOKEN']).to eq('atoken') + end + end + end +end diff --git a/spec/lib/gitlab/storage_check/option_parser_spec.rb b/spec/lib/gitlab/storage_check/option_parser_spec.rb new file mode 100644 index 00000000000..cad4dfbefcf --- /dev/null +++ b/spec/lib/gitlab/storage_check/option_parser_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Gitlab::StorageCheck::OptionParser do + describe '.parse!' do + it 'assigns all options' do + args = %w(--target unix://tmp/hello/world.sock --token thetoken --interval 42) + + options = described_class.parse!(args) + + expect(options.token).to eq('thetoken') + expect(options.interval).to eq(42) + expect(options.target).to eq('unix://tmp/hello/world.sock') + end + + it 'requires the interval to be a number' do + args = %w(--target unix://tmp/hello/world.sock --interval fortytwo) + + expect { described_class.parse!(args) }.to raise_error(OptionParser::InvalidArgument) + end + + it 'raises an error if the scheme is not included' do + args = %w(--target tmp/hello/world.sock) + + expect { described_class.parse!(args) }.to raise_error(OptionParser::InvalidArgument) + end + + it 'raises an error if both socket and host are missing' do + expect { described_class.parse!([]) }.to raise_error(OptionParser::InvalidArgument) + end + end +end diff --git a/spec/lib/gitlab/storage_check/response_spec.rb b/spec/lib/gitlab/storage_check/response_spec.rb new file mode 100644 index 00000000000..0ff2963e443 --- /dev/null +++ b/spec/lib/gitlab/storage_check/response_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe Gitlab::StorageCheck::Response do + let(:fake_json) do + { + check_interval: 42, + results: [ + { storage: 'working', success: true }, + { storage: 'skipped', success: nil }, + { storage: 'failing', success: false } + ] + }.to_json + end + + let(:fake_http_response) do + fake_response = instance_double("Excon::Response - Status check") + allow(fake_response).to receive(:status).and_return(200) + allow(fake_response).to receive(:body).and_return(fake_json) + allow(fake_response).to receive(:headers).and_return('Content-Type' => 'application/json') + + fake_response + end + let(:response) { described_class.new(fake_http_response) } + + describe '#valid?' do + it 'is valid for a success response with parseable JSON' do + expect(response).to be_valid + end + end + + describe '#check_interval' do + it 'returns the result from the JSON' do + expect(response.check_interval).to eq(42) + end + end + + describe '#responsive_shards' do + it 'contains the names of working shards' do + expect(response.responsive_shards).to contain_exactly('working') + end + end + + describe '#skipped_shards' do + it 'contains the names of skipped shards' do + expect(response.skipped_shards).to contain_exactly('skipped') + end + end + + describe '#failing_shards' do + it 'contains the name of failing shards' do + expect(response.failing_shards).to contain_exactly('failing') + end + end +end diff --git a/spec/lib/gitlab/tcp_checker_spec.rb b/spec/lib/gitlab/tcp_checker_spec.rb new file mode 100644 index 00000000000..4acf0334496 --- /dev/null +++ b/spec/lib/gitlab/tcp_checker_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::TcpChecker do + before do + @server = TCPServer.new('localhost', 0) + _, @port, _, @ip = @server.addr + end + + after do + @server.close + end + + subject(:checker) { described_class.new(@ip, @port) } + + describe '#check' do + subject { checker.check } + + it 'can connect to an open port' do + is_expected.to be_truthy + + expect(checker.error).to be_nil + end + + it 'fails to connect to a closed port' do + @server.close + + is_expected.to be_falsy + + expect(checker.error).to be_a(Errno::ECONNREFUSED) + end + end +end diff --git a/spec/lib/gitlab/utils/strong_memoize_spec.rb b/spec/lib/gitlab/utils/strong_memoize_spec.rb index 4a104ab6d97..473f8100771 100644 --- a/spec/lib/gitlab/utils/strong_memoize_spec.rb +++ b/spec/lib/gitlab/utils/strong_memoize_spec.rb @@ -49,4 +49,16 @@ describe Gitlab::Utils::StrongMemoize do end end end + + describe '#clear_memoization' do + let(:value) { 'mepmep' } + + it 'removes the instance variable' do + object.method_name + + object.clear_memoization(:method_name) + + expect(object.instance_variable_defined?(:@method_name)).to be(false) + end + end end diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index 3137a72fdc4..e872a5290c5 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Utils do - delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, to: :described_class + delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, to: :described_class describe '.slugify' do { @@ -59,4 +59,12 @@ describe Gitlab::Utils do expect(random_string).to be_kind_of(String) end end + + describe '.which' do + it 'finds the full path to an executable binary' do + expect(File).to receive(:executable?).with('/bin/sh').and_return(true) + + expect(which('sh', 'PATH' => '/bin')).to eq('/bin/sh') + end + end end diff --git a/spec/lib/gitlab/view/presenter/factory_spec.rb b/spec/lib/gitlab/view/presenter/factory_spec.rb index 70d2e22b48f..6120bafb2e3 100644 --- a/spec/lib/gitlab/view/presenter/factory_spec.rb +++ b/spec/lib/gitlab/view/presenter/factory_spec.rb @@ -27,5 +27,13 @@ describe Gitlab::View::Presenter::Factory do expect(presenter).to be_a(Ci::BuildPresenter) end + + it 'uses the presenter_class if given on #initialize' do + MyCustomPresenter = Class.new(described_class) + + presenter = described_class.new(build, presenter_class: MyCustomPresenter).fabricate! + + expect(presenter).to be_a(MyCustomPresenter) + end end end |