Rename MergeRequest methods that return commits or shas to be more clear and consistent

3 files changed, 18 insertions, 17 deletions

diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index ceb4d64698f..bb83676cddf 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -62,7 +62,7 @@ describe MergeRequest, models: true do
     end
   end
 
-  describe '#target_sha' do
+  describe '#target_branch_sha' do
     context 'when the target branch does not exist anymore' do
       let(:project) { create(:project) }
 
@@ -73,32 +73,32 @@ describe MergeRequest, models: true do
       end
 
       it 'returns nil' do
-        expect(subject.target_sha).to be_nil
+        expect(subject.target_branch_sha).to be_nil
       end
     end
   end
 
-  describe '#source_sha' do
+  describe '#source_branch_sha' do
     let(:last_branch_commit) { subject.source_project.repository.commit(subject.source_branch) }
 
     context 'with diffs' do
       subject { create(:merge_request, :with_diffs) }
       it 'returns the sha of the source branch last commit' do
-        expect(subject.source_sha).to eq(last_branch_commit.sha)
+        expect(subject.source_branch_sha).to eq(last_branch_commit.sha)
       end
     end
 
     context 'without diffs' do
       subject { create(:merge_request, :without_diffs) }
       it 'returns the sha of the source branch last commit' do
-        expect(subject.source_sha).to eq(last_branch_commit.sha)
+        expect(subject.source_branch_sha).to eq(last_branch_commit.sha)
       end
     end
 
     context 'when the merge request is being created' do
       subject { build(:merge_request, source_branch: nil, compare_commits: []) }
       it 'returns nil' do
-        expect(subject.source_sha).to be_nil
+        expect(subject.source_branch_sha).to be_nil
       end
     end
   end
@@ -252,12 +252,14 @@ describe MergeRequest, models: true do
     end
 
     it "can be removed if the last commit is the head of the source branch" do
-      allow(subject.source_project).to receive(:commit).and_return(subject.last_commit)
+      allow(subject.source_project).to receive(:commit).and_return(subject.diff_head_commit)
 
       expect(subject.can_remove_source_branch?(user)).to be_truthy
     end
 
     it "cannot be removed if the last commit is not also the head of the source branch" do
+      subject.source_branch = "lfs"
+
       expect(subject.can_remove_source_branch?(user)).to be_falsey
     end
   end
@@ -363,7 +365,7 @@ describe MergeRequest, models: true do
           and_return(2)
 
         subject.diverged_commits_count
-        allow(subject).to receive(:source_sha).and_return('123abc')
+        allow(subject).to receive(:source_branch_sha).and_return('123abc')
         subject.diverged_commits_count
       end
 
@@ -373,7 +375,7 @@ describe MergeRequest, models: true do
           and_return(2)
 
         subject.diverged_commits_count
-        allow(subject).to receive(:target_sha).and_return('123abc')
+        allow(subject).to receive(:target_branch_sha).and_return('123abc')
         subject.diverged_commits_count
       end
     end
@@ -392,11 +394,10 @@ describe MergeRequest, models: true do
 
   describe '#pipeline' do
     describe 'when the source project exists' do
-      it 'returns the latest commit' do
-        commit   = double(:commit, id: '123abc')
+      it 'returns the latest pipeline' do
         pipeline = double(:ci_pipeline, ref: 'master')
 
-        allow(subject).to receive(:last_commit).and_return(commit)
+        allow(subject).to receive(:diff_head_sha).and_return('123abc')
 
         expect(subject.source_project).to receive(:pipeline).
           with('123abc', 'master').
@@ -464,7 +465,7 @@ describe MergeRequest, models: true do
     context 'when it is not broken and has no conflicts' do
       it 'is marked as mergeable' do
         allow(subject).to receive(:broken?) { false }
-        allow(project.repository).to receive(:can_be_merged?) { true }
+        allow(project.repository).to receive(:can_be_merged?).and_return(true)
 
         expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged')
       end
@@ -481,7 +482,7 @@ describe MergeRequest, models: true do
     context 'when it has conflicts' do
       before do
         allow(subject).to receive(:broken?) { false }
-        allow(project.repository).to receive(:can_be_merged?) { false }
+        allow(project.repository).to receive(:can_be_merged?).and_return(false)
       end
 
       it 'becomes unmergeable' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 2e89d6de3a2..1b434a726dc 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -312,7 +312,7 @@ describe Project, models: true do
     it 'should update merge request commits with new one if pushed to source branch' do
       project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.source_branch}", key.user)
       merge_request.reload
-      expect(merge_request.last_commit.id).to eq(commit_id)
+      expect(merge_request.diff_head_sha).to eq(commit_id)
     end
   end
 
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 7975fc64e59..24e49c8def3 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -12,8 +12,8 @@ describe Repository, models: true do
   end
   let(:merge_commit) do
     source_sha = repository.find_branch('feature').target
-    merge_commit_id = repository.merge(user, source_sha, 'master', commit_options)
-    repository.commit(merge_commit_id)
+    merge_commit_sha = repository.merge(user, source_sha, 'master', commit_options)
+    repository.commit(merge_commit_sha)
   end
 
   describe :branch_names_contains do