diff options
author | Rubén Dávila <ruben@gitlab.com> | 2017-12-20 10:57:27 -0500 |
---|---|---|
committer | Rubén Dávila <ruben@gitlab.com> | 2017-12-20 10:57:27 -0500 |
commit | c927e57466b6c705891f09c95f566259d8e1ec0e (patch) | |
tree | c464abfd7c7f7138ea5ae0dabebd7e9f3a0680b2 /spec/services | |
parent | c210ddab9289e29fadc9a5a0462ffbe864af736c (diff) | |
download | gitlab-ce-c927e57466b6c705891f09c95f566259d8e1ec0e.tar.gz |
Updates from last code review:38356-add-last_commit_sha-to-the-commit-api
- Apply some refactoring for code reuse
- Add file status validation for Files::DeleteService
- Write additional specs
Diffstat (limited to 'spec/services')
-rw-r--r-- | spec/services/files/delete_service_spec.rb | 64 | ||||
-rw-r--r-- | spec/services/files/multi_service_spec.rb | 59 |
2 files changed, 118 insertions, 5 deletions
diff --git a/spec/services/files/delete_service_spec.rb b/spec/services/files/delete_service_spec.rb new file mode 100644 index 00000000000..e9f8f0efe6b --- /dev/null +++ b/spec/services/files/delete_service_spec.rb @@ -0,0 +1,64 @@ +require "spec_helper" + +describe Files::DeleteService do + subject { described_class.new(project, user, commit_params) } + + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + let(:file_path) { 'files/ruby/popen.rb' } + let(:branch_name) { project.default_branch } + let(:last_commit_sha) { nil } + + let(:commit_params) do + { + file_path: file_path, + commit_message: "Delete File", + last_commit_sha: last_commit_sha, + start_project: project, + start_branch: project.default_branch, + branch_name: branch_name + } + end + + shared_examples 'successfully deletes the file' do + it 'returns a hash with the :success status' do + results = subject.execute + + expect(results[:status]).to match(:success) + end + + it 'deletes the file' do + subject.execute + + blob = project.repository.blob_at_branch(project.default_branch, file_path) + + expect(blob).to be_nil + end + end + + before do + project.team << [user, :master] + end + + describe "#execute" do + context "when the file's last commit sha does not match the supplied last_commit_sha" do + let(:last_commit_sha) { "foo" } + + it "returns a hash with the correct error message and a :error status " do + expect { subject.execute } + .to raise_error(Files::UpdateService::FileChangedError, + "You are attempting to delete a file that has been previously updated.") + end + end + + context "when the file's last commit sha does match the supplied last_commit_sha" do + let(:last_commit_sha) { Gitlab::Git::Commit.last_for_path(project.repository, project.default_branch, file_path).sha } + + it_behaves_like 'successfully deletes the file' + end + + context "when the last_commit_sha is not supplied" do + it_behaves_like 'successfully deletes the file' + end + end +end diff --git a/spec/services/files/multi_service_spec.rb b/spec/services/files/multi_service_spec.rb index 6e11f126f14..085a28d267f 100644 --- a/spec/services/files/multi_service_spec.rb +++ b/spec/services/files/multi_service_spec.rb @@ -6,18 +6,20 @@ describe Files::MultiService do let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:branch_name) { project.default_branch } - let(:file_path) { 'files/ruby/popen.rb' } + let(:original_file_path) { 'files/ruby/popen.rb' } + let(:new_file_path) { 'files/ruby/popen.rb' } let(:action) { 'update' } let!(:original_commit_id) do - Gitlab::Git::Commit.last_for_path(project.repository, branch_name, file_path).sha + Gitlab::Git::Commit.last_for_path(project.repository, branch_name, original_file_path).sha end let(:actions) do [ { action: action, - file_path: file_path, + file_path: new_file_path, + previous_path: original_file_path, content: 'New content', last_commit_id: original_commit_id } @@ -60,14 +62,14 @@ describe Files::MultiService do describe 'Updating files' do context 'when the file has been previously updated' do before do - update_file(file_path) + update_file(original_file_path) end it 'rejects the commit' do results = subject.execute expect(results[:status]).to eq(:error) - expect(results[:message]).to match(file_path) + expect(results[:message]).to match(new_file_path) end end @@ -79,6 +81,53 @@ describe Files::MultiService do end end end + + context 'when moving a file' do + let(:action) { 'move' } + let(:new_file_path) { 'files/ruby/new_popen.rb' } + + context 'when original file has been updated' do + before do + update_file(original_file_path) + end + + it 'rejects the commit' do + results = subject.execute + + expect(results[:status]).to eq(:error) + expect(results[:message]).to match(original_file_path) + end + end + + context 'when original file have not been updated' do + it 'moves the file' do + results = subject.execute + blob = project.repository.blob_at_branch(branch_name, new_file_path) + + expect(results[:status]).to eq(:success) + expect(blob).to be_present + end + end + end + + context 'when file status validation is skipped' do + let(:action) { 'create' } + let(:new_file_path) { 'files/ruby/new_file.rb' } + + it 'does not check the last commit' do + expect(Gitlab::Git::Commit).not_to receive(:last_for_path) + + subject.execute + end + + it 'creates the file' do + subject.execute + + blob = project.repository.blob_at_branch(branch_name, new_file_path) + + expect(blob).to be_present + end + end end def update_file(path) |