Merge branch 'mk/authorized-push-to-protected-branch-test-qa' into 'master'

Add QA test for authorized push to protected branch

See merge request gitlab-org/gitlab-ce!18696

7 files changed, 59 insertions, 56 deletions

diff --git a/qa/qa/factory/repository/push.rb b/qa/qa/factory/repository/push.rb
index 28711c12701..7bb808652da 100644
--- a/qa/qa/factory/repository/push.rb
+++ b/qa/qa/factory/repository/push.rb
@@ -3,7 +3,7 @@ module QA
     module Repository
       class Push < Factory::Base
         attr_accessor :file_name, :file_content, :commit_message,
-                      :branch_name, :new_branch
+                      :branch_name, :new_branch, :output
 
         attr_writer :remote_branch
 
@@ -12,6 +12,10 @@ module QA
           project.description = 'Project with repository'
         end
 
+        product :output do |factory|
+          factory.output
+        end
+
         def initialize
           @file_name = 'file.txt'
           @file_content = '# This is test project'
@@ -58,7 +62,7 @@ module QA
             end
 
             repository.commit(commit_message)
-            repository.push_changes("#{branch_name}:#{remote_branch}")
+            @output = repository.push_changes("#{branch_name}:#{remote_branch}")
           end
         end
       end
diff --git a/qa/qa/factory/resource/branch.rb b/qa/qa/factory/resource/branch.rb
index 1785441f5a8..4cabe7eab45 100644
--- a/qa/qa/factory/resource/branch.rb
+++ b/qa/qa/factory/resource/branch.rb
@@ -46,7 +46,9 @@ module QA
             resource.remote_branch = @branch_name
           end
 
-          Page::Project::Show.act { wait_for_push }
+          Page::Project::Show.perform do |page|
+            page.wait { page.has_content?(branch_name) }
+          end
 
           # The upcoming process will make it access the Protected Branches page,
           # select the already created branch and protect it according
@@ -62,13 +64,13 @@ module QA
               page.select_branch(branch_name)
 
               if allow_to_push
-                page.allow_devs_and_masters_to_push
+                page.allow_devs_and_maintainers_to_push
               else
                 page.allow_no_one_to_push
               end
 
               if allow_to_merge
-                page.allow_devs_and_masters_to_merge
+                page.allow_devs_and_maintainers_to_merge
               else
                 page.allow_no_one_to_merge
               end
@@ -79,9 +81,13 @@ module QA
 
               page.protect_branch
 
-              # Wait for page load, which resets the expanded sections
-              page.wait(reload: false) do
-                !page.has_content?('Collapse')
+              # Avoid Selenium::WebDriver::Error::StaleElementReferenceError
+              # without sleeping. I.e. this completes fast on fast machines.
+              page.refresh
+
+              # It is possible for the protected branch row to "disappear" at first
+              page.wait do
+                page.has_content?(branch_name)
               end
             end
           end
diff --git a/qa/qa/git/repository.rb b/qa/qa/git/repository.rb
index 5bc4ffbb036..fc753554fc4 100644
--- a/qa/qa/git/repository.rb
+++ b/qa/qa/git/repository.rb
@@ -7,8 +7,6 @@ module QA
     class Repository
       include Scenario::Actable
 
-      attr_reader :push_output
-
       def self.perform(*args)
         Dir.mktmpdir do |dir|
           Dir.chdir(dir) { super }
@@ -71,7 +69,9 @@ module QA
       end
 
       def push_changes(branch = 'master')
-        @push_output, _ = run_and_redact_credentials("git push #{@uri} #{branch}")
+        output, _ = run_and_redact_credentials("git push #{@uri} #{branch}")
+
+        output
       end
 
       def commits
diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb
index 0a69af88570..30e35bf7abb 100644
--- a/qa/qa/page/base.rb
+++ b/qa/qa/page/base.rb
@@ -13,7 +13,7 @@ module QA
         visit current_url
       end
 
-      def wait(max: 60, time: 1, reload: true)
+      def wait(max: 60, time: 0.1, reload: true)
         start = Time.now
 
         while Time.now - start < max
diff --git a/qa/qa/page/group/show.rb b/qa/qa/page/group/show.rb
index 89125bd2e59..3e0eaa392f5 100644
--- a/qa/qa/page/group/show.rb
+++ b/qa/qa/page/group/show.rb
@@ -28,11 +28,9 @@ module QA
         def has_subgroup?(name)
           filter_by_name(name)
 
-          wait(reload: false) do
-            break false if page.has_content?('Sorry, no groups or projects matched your search')
+          page.has_text?(/#{name}|Sorry, no groups or projects matched your search/, wait: 60)
 
-            page.has_link?(name)
-          end
+          page.has_text?(name, wait: 0)
         end
 
         def go_to_new_subgroup
diff --git a/qa/qa/page/project/settings/protected_branches.rb b/qa/qa/page/project/settings/protected_branches.rb
index a0903c3c4dc..0bd031e96b5 100644
--- a/qa/qa/page/project/settings/protected_branches.rb
+++ b/qa/qa/page/project/settings/protected_branches.rb
@@ -40,7 +40,7 @@ module QA
             click_allow(:push, 'No one')
           end
 
-          def allow_devs_and_masters_to_push
+          def allow_devs_and_maintainers_to_push
             click_allow(:push, 'Developers + Maintainers')
           end
 
@@ -48,7 +48,7 @@ module QA
             click_allow(:merge, 'No one')
           end
 
-          def allow_devs_and_masters_to_merge
+          def allow_devs_and_maintainers_to_merge
             click_allow(:merge, 'Developers + Maintainers')
           end
 
@@ -75,10 +75,6 @@ module QA
 
             within_element(:"allowed_to_#{action}_dropdown") do
               click_on text
-
-              wait(reload: false) do
-                has_css?('.is-active')
-              end
             end
           end
         end
diff --git a/qa/qa/specs/features/repository/protected_branches_spec.rb b/qa/qa/specs/features/repository/protected_branches_spec.rb
index efe7863dc87..491675875b9 100644
--- a/qa/qa/specs/features/repository/protected_branches_spec.rb
+++ b/qa/qa/specs/features/repository/protected_branches_spec.rb
@@ -7,12 +7,6 @@ module QA
         resource.name = 'protected-branch-project'
       end
     end
-    given(:location) do
-      Page::Project::Show.act do
-        choose_repository_clone_http
-        repository_location
-      end
-    end
 
     before do
       Runtime::Browser.visit(:gitlab, Page::Main::Login)
@@ -26,44 +20,49 @@ module QA
       Capybara.execute_script 'localStorage.clear()'
     end
 
-    scenario 'user is able to protect a branch' do
-      protected_branch = Factory::Resource::Branch.fabricate! do |resource|
-        resource.branch_name = branch_name
-        resource.project = project
-        resource.allow_to_push = true
-        resource.protected = true
+    context 'when developers and maintainers are allowed to push to a protected branch' do
+      let!(:protected_branch) { create_protected_branch(allow_to_push: true) }
+
+      scenario 'user with push rights successfully pushes to the protected branch' do
+        expect(protected_branch.name).to have_content(branch_name)
+        expect(protected_branch.push_allowance).to have_content('Developers + Maintainers')
+
+        push = push_new_file(branch_name)
+
+        expect(push.output).to match(/remote: To create a merge request for protected-branch, visit/)
       end
+    end
+
+    context 'when developers and maintainers are not allowed to push to a protected branch' do
+      scenario 'user without push rights fails to push to the protected branch' do
+        create_protected_branch(allow_to_push: false)
+
+        push = push_new_file(branch_name)
 
-      expect(protected_branch.name).to have_content(branch_name)
-      expect(protected_branch.push_allowance).to have_content('Developers + Maintainers')
+        expect(push.output)
+          .to match(/remote\: GitLab\: You are not allowed to push code to protected branches on this project/)
+        expect(push.output)
+          .to match(/\[remote rejected\] #{branch_name} -> #{branch_name} \(pre-receive hook declined\)/)
+      end
     end
 
-    scenario 'users without authorization cannot push to protected branch' do
+    def create_protected_branch(allow_to_push:)
       Factory::Resource::Branch.fabricate! do |resource|
         resource.branch_name = branch_name
         resource.project = project
-        resource.allow_to_push = false
+        resource.allow_to_push = allow_to_push
         resource.protected = true
       end
+    end
 
-      project.visit!
-
-      Git::Repository.perform do |repository|
-        repository.uri = location.uri
-        repository.use_default_credentials
-
-        repository.act do
-          clone
-          configure_identity('GitLab QA', 'root@gitlab.com')
-          checkout('protected-branch')
-          commit_file('README.md', 'readme content', 'Add a readme')
-          push_changes('protected-branch')
-        end
-
-        expect(repository.push_output)
-          .to match(/remote\: GitLab\: You are not allowed to push code to protected branches on this project/)
-        expect(repository.push_output)
-          .to match(/\[remote rejected\] #{branch_name} -> #{branch_name} \(pre-receive hook declined\)/)
+    def push_new_file(branch)
+      Factory::Repository::Push.fabricate! do |resource|
+        resource.project = project
+        resource.file_name = 'new_file.md'
+        resource.file_content = '# This is a new file'
+        resource.commit_message = 'Add new_file.md'
+        resource.branch_name = branch_name
+        resource.new_branch = false
       end
     end
   end