Merge branch 'qa-ml-approval-rules-tests' into 'master'

Add E2E test of approval rules

Closes gitlab-org/quality/testcases#115

See merge request gitlab-org/gitlab-ee!15795

doc/development/testing_guide/end_to_end/page_objects.md

@@ -167,6 +167,18 @@ There are two supported methods of defining elements within a view.
 Any existing `.qa-selector` class should be considered deprecated
 and we should prefer the `data-qa-selector` method of definition.
 
+### Exceptions
+
+In some cases it might not be possible or worthwhile to add a selector.
+
+Some UI components use external libraries, including some maintained by third parties.
+Even if a library is maintained by GitLab, the selector sanity test only runs
+on code within the GitLab project, so it's not possible to specify the path for
+the view for code in a library.
+
+In such rare cases it's reasonable to use CSS selectors in page object methods,
+with a comment explaining why an `element` can't be added.
+
 ## Running the test locally
 
 During development, you can run the `qa:selectors` test by running

ee/app/assets/javascripts/approvals/components/app.vue

@@ -42,15 +42,21 @@ export default {
     <gl-loading-icon v-if="!hasLoaded" :size="2" />
     <template v-else>
       <div class="border-bottom">
-        <slot v-if="isEmpty" name="fallback"> <fallback-rules /> </slot>
+        <slot v-if="isEmpty" name="fallback">
+          <fallback-rules />
+        </slot>
         <slot v-else name="rules"></slot>
       </div>
       <div v-if="settings.canEdit" class="border-bottom py-3 px-2">
         <gl-loading-icon v-if="isLoading" />
         <div v-if="settings.allowMultiRule" class="d-flex">
-          <gl-button class="ml-auto btn-info btn-inverted" @click="openCreateModal(null)">{{
-            __('Add approval rule')
-          }}</gl-button>
+          <gl-button
+            class="ml-auto btn-info btn-inverted"
+            data-qa-selector="add_approvers_button"
+            @click="openCreateModal(null)"
+          >
+            {{ __('Add approval rule') }}
+          </gl-button>
         </div>
       </div>
       <slot name="footer"></slot>

ee/app/assets/javascripts/approvals/components/rule_form.vue

@@ -229,6 +229,7 @@ export default {
             class="form-control"
             name="name"
             type="text"
+            data-qa-selector="rule_name_field"
           />
           <span class="invalid-feedback">{{ validation.name }}</span>
           <span class="text-secondary">{{ s__('ApprovalRule|e.g. QA, Security, etc.') }}</span>
@@ -236,9 +237,7 @@ export default {
       </div>
       <div class="form-group col-sm-6">
         <label class="label-wrapper">
-          <span class="mb-2 bold inline">
-            {{ s__('ApprovalRule|No. approvals required') }}
-          </span>
+          <span class="mb-2 bold inline">{{ s__('ApprovalRule|No. approvals required') }}</span>
           <input
             v-model.number="approvalsRequired"
             :class="{ 'is-invalid': validation.approvalsRequired }"
@@ -246,6 +245,7 @@ export default {
             name="approvals_required"
             type="number"
             :min="minApprovalsRequired"
+            data-qa-selector="approvals_required_field"
           />
           <span class="invalid-feedback">{{ validation.approvalsRequired }}</span>
         </label>
@@ -254,7 +254,7 @@ export default {
     <div class="form-group">
       <label class="label-bold">{{ s__('ApprovalRule|Approvers') }}</label>
       <div class="d-flex align-items-start">
-        <div class="w-100">
+        <div class="w-100" data-qa-selector="member_select_field">
           <approvers-select
             v-model="approversToAdd"
             :project-id="settings.projectId"
@@ -264,9 +264,14 @@ export default {
           />
           <div class="invalid-feedback">{{ validation.approvers }}</div>
         </div>
-        <gl-button variant="success" class="btn-inverted prepend-left-8" @click="addSelection">{{
-          __('Add')
-        }}</gl-button>
+        <gl-button
+          variant="success"
+          class="btn-inverted prepend-left-8"
+          data-qa-selector="add_member_button"
+          @click="addSelection"
+        >
+          {{ __('Add') }}
+        </gl-button>
       </div>
     </div>
     <div class="bordered-box overflow-auto h-12em">

ee/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue

@@ -215,6 +215,7 @@ export default {
           :class="{ 'btn-inverted': action.inverted }"
           size="sm"
           class="mr-3"
+          data-qa-selector="approve_button"
           @click="action.action"
         >
           <gl-loading-icon v-if="isApproving" inline />

ee/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary.vue

@@ -59,7 +59,7 @@ export default {
 </script>
 
 <template>
-  <div>
+  <div data-qa-selector="approvals_summary_content">
     <strong>{{ message }}</strong>
     <template v-if="hasApprovers">
       <span>{{ s__('mrWidget|Approved by') }}</span>

qa/qa/ee.rb

@@ -111,6 +111,7 @@ module QA
       end
 
       module MergeRequest
+        autoload :New, 'qa/ee/page/merge_request/new'
         autoload :Show, 'qa/ee/page/merge_request/show'
       end
 

qa/qa/ee/page/merge_request/new.rb

+# frozen_string_literal: true
+
+module QA
+  module EE
+    module Page
+      module MergeRequest
+        module New
+          def self.prepended(page)
+            page.module_eval do
+              view 'ee/app/assets/javascripts/approvals/components/app.vue' do
+                element :add_approvers_button
+              end
+
+              view 'ee/app/assets/javascripts/approvals/components/rule_form.vue' do
+                element :add_member_button
+                element :approvals_required_field
+                element :member_select_field
+                element :rule_name_field
+              end
+
+              def add_approval_rules(rules)
+                rules.each do |rule|
+                  click_element :add_approvers_button
+
+                  wait_for_animated_element :rule_name_field
+
+                  fill_element :rule_name_field, rule[:name]
+                  fill_element :approvals_required_field, rule[:approvals_required]
+
+                  rule.key?(:users) && rule[:users].each do |user|
+                    select_user_member user.username
+                    click_element :add_member_button
+                  end
+                  rule.key?(:groups) && rule[:groups].each do |group|
+                    select_group_member group.name
+                    click_element :add_member_button
+                  end
+
+                  click_approvers_modal_ok_button
+                end
+              end
+
+              # The Add/Update approvers modal is a gitlab-ui component built on
+              # a bootstrap-vue component. It doesn't seem straightforward to
+              # add a data attribute to the 'Ok' button without overriding it
+              # So we break the rules and use a CSS selector instead of an element
+              def click_approvers_modal_ok_button
+                find("#mr-edit-approvals-create-modal footer button.btn-success").click
+              end
+
+              # Select2 is an external library, so we can't add our own selector
+              def select_user_member(name)
+                enter_member(name)
+                find('.select2-results .user-username', text: "@#{name}").click
+              end
+
+              def select_group_member(name)
+                enter_member(name)
+                find('.select2-results .group-name', text: "#{name}").click
+              end
+
+              private
+
+              def enter_member(name)
+                within_element(:member_select_field) do
+                  find(".select2-input").set(name)
+                end
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

qa/qa/ee/page/merge_request/show.rb

@@ -45,76 +45,124 @@ module QA
                 element :expand_report_button
               end
 
-              view 'ee/app/assets/javascripts/vue_shared/security_reports/components/modal_footer.vue' do
-                element :resolve_split_button
+              view 'ee/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue' do
+                element :approve_button
               end
 
-              def start_review
-                click_element :start_review
+              view 'ee/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary.vue' do
+                element :approvals_summary_content
               end
 
-              def comment_now
-                click_element :comment_now
+              view 'ee/app/assets/javascripts/vue_shared/security_reports/components/modal_footer.vue' do
+                element :resolve_split_button
               end
+            end
+          end
 
-              def submit_pending_reviews
-                within_element :review_bar do
-                  click_element :review_preview_toggle
-                  click_element :submit_review
-                end
-              end
+          def approvals_required_from
+            approvals_content.match(/approvals? from (.*)/)[1]
+          end
 
-              def discard_pending_reviews
-                within_element :review_bar do
-                  click_element :discard_review
-                end
-                click_element :modal_delete_pending_comments
-              end
+          def approved?
+            approvals_content =~ /Merge request approved/
+          end
 
-              def resolve_review_discussion
-                scroll_to_element :start_review
-                check_element :resolve_review_discussion_checkbox
-              end
+          def approvers
+            within_element :approver_list do
+              all_elements(:approver).map { |item| item.find('img')['title'] }
+            end
+          end
 
-              def unresolve_review_discussion
-                check_element :unresolve_review_discussion_checkbox
-              end
+          def click_approve
+            click_element :approve_button
+          end
 
-              def approvers
-                within_element :approver_list do
-                  all_elements(:approver).map { |item| item.find('img')['title'] }
-                end
-              end
+          def start_review
+            click_element :start_review
+          end
 
-              def expand_vulnerability_report
-                click_element :expand_report_button
-              end
+          def comment_now
+            click_element :comment_now
+          end
 
-              def click_vulnerability(name)
-                within_element :vulnerability_report_grouped do
-                  click_on name
-                end
-              end
+          def submit_pending_reviews
+            within_element :review_bar do
+              click_element :review_preview_toggle
+              click_element :submit_review
+            end
+          end
 
-              def resolve_vulnerability_with_mr(name)
-                expand_vulnerability_report
-                click_vulnerability(name)
-                click_element :resolve_split_button
-              end
+          def discard_pending_reviews
+            within_element :review_bar do
+              click_element :discard_review
+            end
+            click_element :modal_delete_pending_comments
+          end
 
-              def has_vulnerability_report?(timeout: 60)
-                wait(reload: true, max: timeout, interval: 1) do
-                  finished_loading?
-                  has_element?(:vulnerability_report_grouped, wait: 1)
-                end
-              end
+          def resolve_review_discussion
+            scroll_to_element :start_review
+            check_element :resolve_review_discussion_checkbox
+          end
 
-              def has_detected_vulnerability_count_of?(expected)
-                # Match text cut off in order to find both "1 vulnerability" and "X vulnerabilities"
-                find_element(:vulnerability_report_grouped).has_content?("detected #{expected} vulnerabilit")
-              end
+          def unresolve_review_discussion
+            check_element :unresolve_review_discussion_checkbox
+          end
+
+          def expand_vulnerability_report
+            click_element :expand_report_button
+          end
+
+          def click_vulnerability(name)
+            within_element :vulnerability_report_grouped do
+              click_on name
+            end
+          end
+
+          def resolve_vulnerability_with_mr(name)
+            expand_vulnerability_report
+            click_vulnerability(name)
+            click_element :resolve_split_button
+          end
+
+          def has_vulnerability_report?(timeout: 60)
+            wait(reload: true, max: timeout, interval: 1) do
+              finished_loading?
+              has_element?(:vulnerability_report_grouped, wait: 1)
             end
           end
+
+          def has_detected_vulnerability_count_of?(expected)
+            # Match text cut off in order to find both "1 vulnerability" and "X vulnerabilities"
+            find_element(:vulnerability_report_grouped).has_content?("detected #{expected} vulnerabilit")
+          end
+
+          def num_approvals_required
+            approvals_content.match(/Requires (\d+) more approvals/)[1].to_i
+          end
+
+          private
+
+          def approvals_content
+            # The approvals widget displays "Checking approval status" briefly
+            # while loading the widget, so before returning the text we wait
+            # for it to include terms from content we expect. The kinds
+            # of content we expect are:
+            #
+            # * Requires X more approvals from Quality, UX, and frontend.
+            # * Merge request approved
+            #
+            # It can also briefly display cached data while loading so we
+            # wait for it to update first
+            sleep 1
+
+            text = nil
+            wait(reload: false) do
+              text = find_element(:approvals_summary_content).text
+              text =~ /Requires|approved/
+            end
+
+            text
+          end
         end
       end
     end

qa/qa/page/main/menu.rb

@@ -61,6 +61,10 @@ module QA
           end
         end
 
+        def sign_out_if_signed_in
+          sign_out if has_personal_area?(wait: 0)
+        end
+
         def click_settings_link
           retry_until(reload: false) do
             within_user_menu do

qa/qa/page/merge_request/new.rb

@@ -64,3 +64,5 @@ module QA
     end
   end
 end
+
+QA::Page::MergeRequest::New.prepend_if_ee('QA::EE::Page::MergeRequest::New')

qa/qa/resource/group.rb

@@ -10,6 +10,7 @@ module QA
       end
 
       attribute :id
+      attribute :name
 
       def initialize
         @path = Runtime::Namespace.name
@@ -47,6 +48,11 @@ module QA
         super
       end
 
+      def add_member(user, access_level = '30')
+        # 30 = developer access
+        post Runtime::API::Request.new(api_client, api_members_path).url, { user_id: user.id, access_level: access_level }
+      end
+
       def api_get_path
         "/groups/#{CGI.escape("#{sandbox.path}/#{path}")}"
       end

qa/qa/resource/merge_request.rb

@@ -5,7 +5,8 @@ require 'securerandom'
 module QA
   module Resource
     class MergeRequest < Base
-      attr_accessor :id,
+      attr_accessor :approval_rules,
+                    :id,
                     :title,
                     :description,
                     :source_branch,
@@ -46,6 +47,7 @@ module QA
       end
 
       def initialize
+        @approval_rules = nil
         @title = 'QA test - merge request'
         @description = 'This is a test merge request'
         @source_branch = "qa-test-feature-#{SecureRandom.hex(8)}"
@@ -63,16 +65,17 @@ module QA
 
         project.visit!
         Page::Project::Show.perform(&:new_merge_request)
-        Page::MergeRequest::New.perform do |page|
-          page.fill_title(@title)
-          page.fill_description(@description)
-          page.choose_milestone(@milestone) if @milestone
-          page.assign_to_me if @assignee == 'me'
+        Page::MergeRequest::New.perform do |new|
+          new.fill_title(@title)
+          new.fill_description(@description)
+          new.choose_milestone(@milestone) if @milestone
+          new.assign_to_me if @assignee == 'me'
           labels.each do |label|
-            page.select_label(label)
+            new.select_label(label)
           end
+          new.add_approval_rules(approval_rules) if approval_rules
 
-          page.create_merge_request
+          new.create_merge_request
         end
       end
 

qa/qa/resource/project.rb

@@ -75,6 +75,11 @@ module QA
         super
       end
 
+      def add_member(user, access_level = '30')
+        # 30 = developer access
+        post Runtime::API::Request.new(api_client, api_members_path).url, { user_id: user.id, access_level: access_level }
+      end
+
       def api_get_path
         "/projects/#{CGI.escape(path_with_namespace)}"
       end
@@ -83,6 +88,10 @@ module QA
         "#{api_get_path}/repository/archive.#{type}"
       end
 
+      def api_members_path
+        "#{api_get_path}/members"
+      end
+
       def api_post_path
         '/projects'
       end

qa/qa/resource/user.rb

@@ -9,6 +9,7 @@ module QA
       attr_writer :username, :password
       attr_accessor :provider, :extern_uid
 
+      attribute :id
       attribute :name
       attribute :email
 

qa/qa/specs/features/ee/browser_ui/3_create/merge_request/approval_rules_spec.rb

+# frozen_string_literal: true
+
+module QA
+  context 'Create' do
+    describe 'Approval rules' do
+      let(:approver1) { Resource::User.fabricate_or_use(Runtime::Env.gitlab_qa_username_1, Runtime::Env.gitlab_qa_password_1) }
+      let(:approver2) { Resource::User.fabricate_or_use(Runtime::Env.gitlab_qa_username_2, Runtime::Env.gitlab_qa_password_2) }
+      let(:project) do
+        Resource::Project.fabricate_via_api! { |project| project.name = "approval-rules" }
+      end
+
+      def login(user = nil)
+        Runtime::Browser.visit(:gitlab, Page::Main::Login)
+        Page::Main::Login.perform { |login| login.sign_in_using_credentials(user) }
+      end
+
+      before do
+        project.add_member(approver1)
+        project.group.add_member(approver2)
+
+        Page::Main::Menu.perform(&:sign_out_if_signed_in)
+
+        login
+      end
+
+      it 'allows multiple approval rules with users and groups' do
+        # Create a merge request with 2 rules
+        merge_request = Resource::MergeRequest.fabricate_via_browser_ui! do |resource|
+          resource.title = 'Add a new feature'
+          resource.description = 'Great feature, much approval'
+          resource.project = project
+          resource.approval_rules = [
+            {
+              name: "user",
+              approvals_required: 1,
+              users: [approver1]
+            },
+            {
+              name: "group",
+              approvals_required: 1,
+              groups: [project.group]
+            }
+          ]
+        end
+
+        Page::MergeRequest::Show.perform do |show|
+          expect(show.num_approvals_required).to eq(2)
+          expect(show.approvals_required_from).to include("user", "group")
+        end
+
+        # As approver1, approve the MR
+        Page::Main::Menu.perform(&:sign_out)
+        login(approver1)
+
+        merge_request.visit!
+
+        Page::MergeRequest::Show.perform do |show|
+          show.click_approve
+        end
+
+        # Confirm that an approval was granted but it is not yet fully approved
+        Page::MergeRequest::Show.perform do |show|
+          expect(show).not_to be_approved
+          expect(show.approvals_required_from).to include("group")
+          expect(show.approvals_required_from).not_to include("user")
+        end
+
+        # As approver2, approve the MR
+        Page::Main::Menu.perform(&:sign_out)
+        login(approver2)
+
+        merge_request.visit!
+
+        Page::MergeRequest::Show.perform do |show|
+          show.click_approve
+        end
+
+        # Confirm that the MR is fully approved
+        Page::MergeRequest::Show.perform do |show|
+          expect(show).to be_approved
+        end
+
+        # Merge the MR as the original user
+        Page::Main::Menu.perform(&:sign_out)
+        login
+
+        merge_request.visit!
+
+        Page::MergeRequest::Show.perform do |show|
+          show.merge!
+        end
+
+        expect(page).to have_content('The changes were merged')
+      end
+    end
+  end
+end