diff options
Diffstat (limited to 'spec/models/merge_request_diff_spec.rb')
-rw-r--r-- | spec/models/merge_request_diff_spec.rb | 115 |
1 files changed, 112 insertions, 3 deletions
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 0839dde696a..d153ccedf8c 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequestDiff do +RSpec.describe MergeRequestDiff do using RSpec::Parameterized::TableSyntax include RepoHelpers @@ -162,7 +162,8 @@ describe MergeRequestDiff do let(:uploader) { ExternalDiffUploader } let(:file_store) { uploader::Store::LOCAL } let(:remote_store) { uploader::Store::REMOTE } - let(:diff) { create(:merge_request).merge_request_diff } + let(:merge_request) { create(:merge_request) } + let(:diff) { merge_request.merge_request_diff } it 'converts from in-database to external file storage' do expect(diff).not_to be_stored_externally @@ -177,6 +178,30 @@ describe MergeRequestDiff do expect(diff.external_diff_store).to eq(file_store) end + it 'safely handles a transaction error when migrating to external storage' do + expect(diff).not_to be_stored_externally + expect(diff.external_diff).not_to be_exists + + stub_external_diffs_setting(enabled: true) + + expect(diff).not_to receive(:save!) + expect(Gitlab::Database) + .to receive(:bulk_insert) + .with('merge_request_diff_files', anything) + .and_raise(ActiveRecord::Rollback) + + expect { diff.migrate_files_to_external_storage! }.not_to change(diff, :merge_request_diff_files) + + diff.reload + + expect(diff).not_to be_stored_externally + + # The diff is written outside of the transaction, which is desirable to + # avoid long transaction times when migrating, but it does mean we can + # leave the file dangling on failure + expect(diff.external_diff).to be_exists + end + it 'converts from in-database to external object storage' do expect(diff).not_to be_stored_externally @@ -209,6 +234,33 @@ describe MergeRequestDiff do diff.migrate_files_to_external_storage! end + + context 'diff adds an empty file' do + let(:project) { create(:project, :test_repo) } + let(:merge_request) do + create( + :merge_request, + source_project: project, + target_project: project, + source_branch: 'empty-file', + target_branch: 'master' + ) + end + + it 'migrates the diff to object storage' do + create_file_in_repo(project, 'master', 'empty-file', 'empty-file', '') + + expect(diff).not_to be_stored_externally + + stub_external_diffs_setting(enabled: true) + stub_external_diffs_object_storage(uploader, direct_upload: true) + + diff.migrate_files_to_external_storage! + + expect(diff).to be_stored_externally + expect(diff.external_diff_store).to eq(remote_store) + end + end end describe '#migrate_files_to_database!' do @@ -476,7 +528,7 @@ describe MergeRequestDiff do include_examples 'merge request diffs' end - describe 'external diffs always enabled' do + describe 'external diffs on disk always enabled' do before do stub_external_diffs_setting(enabled: true, when: 'always') end @@ -484,6 +536,63 @@ describe MergeRequestDiff do include_examples 'merge request diffs' end + describe 'external diffs in object storage always enabled' do + let(:uploader) { ExternalDiffUploader } + let(:remote_store) { uploader::Store::REMOTE } + + subject(:diff) { merge_request.merge_request_diff } + + before do + stub_external_diffs_setting(enabled: true, when: 'always') + stub_external_diffs_object_storage(uploader, direct_upload: true) + end + + # We can't use the full merge request diffs shared examples here because + # reading from the fake object store isn't implemented yet + + context 'empty diff' do + let(:merge_request) { create(:merge_request, :without_diffs) } + + it 'creates an empty diff' do + expect(diff.state).to eq('empty') + expect(diff).not_to be_stored_externally + end + end + + context 'normal diff' do + let(:merge_request) { create(:merge_request) } + + it 'creates a diff in object storage' do + expect(diff).to be_stored_externally + expect(diff.state).to eq('collected') + expect(diff.external_diff_store).to eq(remote_store) + end + end + + context 'diff adding an empty file' do + let(:project) { create(:project, :test_repo) } + let(:merge_request) do + create( + :merge_request, + source_project: project, + target_project: project, + source_branch: 'empty-file', + target_branch: 'master' + ) + end + + it 'creates a diff in object storage' do + create_file_in_repo(project, 'master', 'empty-file', 'empty-file', '') + + diff.reload + + expect(diff).to be_stored_externally + expect(diff.state).to eq('collected') + expect(diff.external_diff_store).to eq(remote_store) + end + end + end + describe 'exernal diffs enabled for outdated diffs' do before do stub_external_diffs_setting(enabled: true, when: 'outdated') |