Refactor `push_rule_check` GitAccess specs

- Reduces duplication of long ref strings
- Passes push rule attributes directly to `create_push_rule`, rather
  than creating one and then updating it.

spec/lib/gitlab/git_access_spec.rb

@@ -721,31 +721,31 @@ describe Gitlab::GitAccess do
     end
 
     describe "push_rule_check" do
+      let(:start_sha) { '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' }
+      let(:end_sha)   { '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' }
+
       before do
-        project.team << [user, :developer]
+        project.add_developer(user)
 
-        allow(project.repository).to receive(:new_commits).and_return(
-          project.repository.commits_between('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9', '570e7b2abdd848b95f2f578043fc23bd6f6fd24d')
-        )
+        allow(project.repository).to receive(:new_commits)
+          .and_return(project.repository.commits_between(start_sha, end_sha))
       end
 
       describe "author email check" do
         it 'returns true' do
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
 
         it 'returns false' do
-          project.create_push_rule
-          project.push_rule.update(commit_message_regex: "@only.com")
+          project.create_push_rule(commit_message_regex: "@only.com")
 
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
 
         it 'returns true for tags' do
-          project.create_push_rule
-          project.push_rule.update(commit_message_regex: "@only.com")
+          project.create_push_rule(commit_message_regex: "@only.com")
 
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/tags/v1') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/tags/v1") }.not_to raise_error
         end
 
         it 'allows githook for new branch with an old bad commit' do
@@ -754,11 +754,10 @@ describe Gitlab::GitAccess do
           allow(bad_commit).to receive(:refs).and_return([ref_object])
           allow_any_instance_of(Repository).to receive(:commits_between).and_return([bad_commit])
 
-          project.create_push_rule
-          project.push_rule.update(commit_message_regex: "Change some files")
+          project.create_push_rule(commit_message_regex: "Change some files")
 
           # push to new branch, so use a blank old rev and new ref
-          expect { access.send(:check_push_access!, "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new-branch") }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{Gitlab::Git::BLANK_SHA} #{end_sha} refs/heads/new-branch") }.not_to raise_error
         end
 
         it 'allows githook for any change with an old bad commit' do
@@ -767,11 +766,10 @@ describe Gitlab::GitAccess do
           allow(bad_commit).to receive(:refs).and_return([ref_object])
           allow(project.repository).to receive(:commits_between).and_return([bad_commit])
 
-          project.create_push_rule
-          project.push_rule.update(commit_message_regex: "Change some files")
+          project.create_push_rule(commit_message_regex: "Change some files")
 
           # push to new branch, so use a blank old rev and new ref
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
 
         it 'does not allow any change from Web UI with bad commit' do
@@ -782,78 +780,76 @@ describe Gitlab::GitAccess do
           allow(project.repository).to receive(:commits_between).and_return([bad_commit])
           allow(project.repository).to receive(:new_commits).and_return([bad_commit])
 
-          project.create_push_rule
-          project.push_rule.update(commit_message_regex: "Change some files")
+          project.create_push_rule(commit_message_regex: "Change some files")
 
           # push to new branch, so use a blank old rev and new ref
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
       end
 
       describe "member_check" do
         before do
-          project.create_push_rule
-          project.push_rule.update(member_check: true)
+          project.create_push_rule(member_check: true)
         end
 
         it 'returns false for non-member user' do
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
 
         it 'returns true if committer is a gitlab member' do
           create(:user, email: 'dmitriy.zaporozhets@gmail.com')
 
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
       end
 
       describe "file names check" do
+        let(:start_sha) { '913c66a37b4a45b9769037c55c2d238bd0942d2e' }
+        let(:end_sha)   { '33f3729a45c02fc67d00adb1b8bca394b0e761d9' }
+
         before do
-          allow(project.repository).to receive(:new_commits).and_return(
-            project.repository.commits_between('913c66a37b4a45b9769037c55c2d238bd0942d2e', '33f3729a45c02fc67d00adb1b8bca394b0e761d9')
-          )
+          allow(project.repository).to receive(:new_commits)
+            .and_return(project.repository.commits_between(start_sha, end_sha))
         end
 
         it 'returns false when filename is prohibited' do
-          project.create_push_rule
-          project.push_rule.update(file_name_regex: "jpg$")
+          project.create_push_rule(file_name_regex: "jpg$")
 
-          expect { access.send(:check_push_access!, '913c66a37b4a45b9769037c55c2d238bd0942d2e 33f3729a45c02fc67d00adb1b8bca394b0e761d9 refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
 
         it 'returns true if file name is allowed' do
-          project.create_push_rule
-          project.push_rule.update(file_name_regex: "exe$")
+          project.create_push_rule(file_name_regex: "exe$")
 
-          expect { access.send(:check_push_access!, '913c66a37b4a45b9769037c55c2d238bd0942d2e 33f3729a45c02fc67d00adb1b8bca394b0e761d9 refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
       end
 
       describe "max file size check" do
+        let(:start_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' }
+        let(:end_sha)   { '913c66a37b4a45b9769037c55c2d238bd0942d2e' }
+
         before do
           allow_any_instance_of(Gitlab::Git::Blob).to receive(:size).and_return(1.5.megabytes.to_i)
         end
 
         it "returns false when size is too large" do
-          project.create_push_rule
-          project.push_rule.update(max_file_size: 1)
+          project.create_push_rule(max_file_size: 1)
 
-          expect { access.send(:check_push_access!, 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660 913c66a37b4a45b9769037c55c2d238bd0942d2e refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
 
         it "returns true when size is allowed" do
-          project.create_push_rule
-          project.push_rule.update(max_file_size: 2)
+          project.create_push_rule(max_file_size: 2)
 
-          expect { access.send(:check_push_access!, 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660 913c66a37b4a45b9769037c55c2d238bd0942d2e refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
 
         it "returns true when size is nil" do
           allow_any_instance_of(Gitlab::Git::Blob).to receive(:size).and_return(nil)
-          project.create_push_rule
-          project.push_rule.update(max_file_size: 2)
+          project.create_push_rule(max_file_size: 2)
 
-          expect { access.send(:check_push_access!, 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660 913c66a37b4a45b9769037c55c2d238bd0942d2e refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
       end
 
@@ -865,13 +861,13 @@ describe Gitlab::GitAccess do
         it 'returns false when blob is too big' do
           allow_any_instance_of(Gitlab::Git::Blob).to receive(:size).and_return(100.megabytes.to_i)
 
-          expect { access.send(:check_push_access!, 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660 913c66a37b4a45b9769037c55c2d238bd0942d2e refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
 
         it 'returns true when blob is just right' do
           allow_any_instance_of(Gitlab::Git::Blob).to receive(:size).and_return(2.megabytes.to_i)
 
-          expect { access.send(:check_push_access!, 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660 913c66a37b4a45b9769037c55c2d238bd0942d2e refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
       end
     end
@@ -913,7 +909,7 @@ describe Gitlab::GitAccess do
     let(:project) { create(:project, :repository, :read_only_repository) }
 
     it 'denies push access' do
-      project.team << [user, :master]
+      project.add_master(user)
 
       expect { push_access_check }.to raise_unauthorized('The repository is temporarily read-only. Please try again later.')
     end