Merge branch 'ml-qa-fix-merge-request-approvals-test' into 'master'

Update E2E test of assigning code owners

Closes #33680

See merge request gitlab-org/gitlab!18358

ee/app/views/projects/merge_requests/_code_owner_approval_rules.html.haml

@@ -14,9 +14,9 @@
         %tr.d-none.d-sm-table-row
           %th.w-25= s_('CodeOwner|Pattern')
           %th= _('Members')
-      %tbody.qa-approver-list
+      %tbody{ data: { qa_selector: "approver_list" } }
         - code_owner_rules.each do |code_owner_approval_rule|
-          %tr.qa-approver
+          %tr{ data: { qa_selector: "approver" } }
             %td.monospace= code_owner_approval_rule.name
             %td.d-none.d-sm-table-cell
               - code_owner_approval_rule.approvers.each do |approver|

qa/qa/ee.rb

@@ -113,7 +113,6 @@ module QA
         module Settings
           autoload :ProtectedBranches, 'qa/ee/page/project/settings/protected_branches'
           autoload :MirroringRepositories, 'qa/ee/page/project/settings/mirroring_repositories'
-          autoload :Main, 'qa/ee/page/project/settings/main'
           autoload :MergeRequest, 'qa/ee/page/project/settings/merge_request'
           autoload :Repository, 'qa/ee/page/project/settings/repository'
           autoload :PushRules, 'qa/ee/page/project/settings/push_rules'

qa/qa/ee/page/project/settings/main.rb

-# frozen_string_literal: true
-
-module QA
-  module EE
-    module Page
-      module Project
-        module Settings
-          module Main
-            def self.prepended(page)
-              page.module_eval do
-                view 'ee/app/views/projects/_merge_request_approvals_settings.html.haml' do
-                  element :merge_request_approval_settings
-                end
-              end
-            end
-
-            def expand_request_approval_settings(&block)
-              expand_section(:merge_request_approval_settings) do
-                MergeRequestApproval.perform(&block)
-              end
-            end
-          end
-        end
-      end
-    end
-  end
-end

qa/qa/page/project/settings/main.rb

@@ -58,5 +58,3 @@ module QA
     end
   end
 end
-
-QA::Page::Project::Settings::Main.prepend_if_ee('QA::EE::Page::Project::Settings::Main')

qa/qa/resource/repository/commit.rb

@@ -58,8 +58,10 @@ module QA
         end
 
         def actions
-          @add_files.map { |file| file.merge({ action: "create" }) } if @add_files
-          @update_files.map { |file| file.merge({ action: "update" }) } if @update_files
+          pending_actions = []
+          @add_files.map { |file| pending_actions << file.merge({ action: "create" }) } if @add_files
+          @update_files.map { |file| pending_actions << file.merge({ action: "update" }) } if @update_files
+          pending_actions
         end
 
         private

qa/qa/specs/features/ee/browser_ui/3_create/repository/assign_code_owners_spec.rb

@@ -3,66 +3,79 @@
 module QA
   context 'Create' do
     describe 'Codeowners' do
-      it 'merge request assigns code owners as approvers' do
-        Runtime::Browser.visit(:gitlab, Page::Main::Login)
-        Page::Main::Login.perform(&:sign_in_using_credentials)
+      # Create one user to be the assigned approver and another user who will not be an approver
+      let(:approver) { Resource::User.fabricate_or_use(Runtime::Env.gitlab_qa_username_1, Runtime::Env.gitlab_qa_password_1) }
+      let(:non_approver) { Resource::User.fabricate_or_use(Runtime::Env.gitlab_qa_username_2, Runtime::Env.gitlab_qa_password_2) }
 
-        # Create one user to be the assigned approver and another user who will
-        # not be an approver
-        approver = Resource::User.fabricate_or_use(Runtime::Env.gitlab_qa_username_1, Runtime::Env.gitlab_qa_password_1)
-        non_approver = Resource::User.fabricate_or_use(Runtime::Env.gitlab_qa_username_2, Runtime::Env.gitlab_qa_password_2)
-
-        # Create a project and assign the users to it
-        project = Resource::Project.fabricate! do |project|
+      let(:project) do
+        Resource::Project.fabricate_via_api! do |project|
           project.name = "assign-approvers"
+          project.initialize_with_readme = true
         end
-        project.visit!
-
-        Page::Project::Menu.perform(&:go_to_members_settings)
-        Page::Project::Settings::Members.perform do |members_page|
-          members_page.add_member(approver.username)
-          members_page.add_member(non_approver.username)
       end
+      let(:branch_name) { 'protected-branch' }
 
-        Page::Project::Menu.perform(&:go_to_general_settings)
+      before do
+        project.add_member(approver, Resource::Members::AccessLevel::DEVELOPER)
+        project.add_member(non_approver, Resource::Members::AccessLevel::DEVELOPER)
 
-        Page::Project::Settings::Main.perform do |settings|
-          settings.expand_request_approval_settings do |page|
-            page.click_require_code_owners_approval_checkbox
-            page.click_save_merge_request_approval_button
-          end
+        Runtime::Browser.visit(:gitlab, Page::Main::Login)
+        Page::Main::Login.perform(&:sign_in_using_credentials)
+
+        project.visit!
       end
 
-        # Push CODEOWNERS to master
-        project_push = Resource::Repository::ProjectPush.fabricate! do |push|
-          push.project = project
-          push.file_name = 'CODEOWNERS'
-          push.file_content = <<~CONTENT
+      it 'merge request assigns code owners as approvers' do
+        # Commit CODEOWNERS to master
+        Resource::Repository::Commit.fabricate_via_api! do |commit|
+          commit.project = project
+          commit.commit_message = 'Add CODEOWNERS and test files'
+          commit.add_files(
+            [
+              {
+                file_path: 'CODEOWNERS',
+                content: <<~CONTENT
                   CODEOWNERS @#{approver.username}
                 CONTENT
-          push.commit_message = 'Add CODEOWNERS and test files'
+              }
+            ]
+          )
         end
 
-        # Push a new CODEOWNERS file and create a merge request
-        merge_request = Resource::MergeRequest.fabricate! do |merge_request|
-          merge_request.title = 'This is a merge request'
-          merge_request.description = 'Change code owners'
-          merge_request.project = project_push.project
-          merge_request.file_name = 'CODEOWNERS'
-          merge_request.target_new_branch = false
-          merge_request.file_content = <<~CONTENT
+        # Create a projected branch that requires approval from code owners
+        Resource::ProtectedBranch.fabricate! do |protected_branch|
+          protected_branch.branch_name = branch_name
+          protected_branch.project = project
+        end
+
+        # Push a new CODEOWNERS file
+        Resource::Repository::Push.fabricate! do |push|
+          push.repository_http_uri = project.repository_http_location.uri
+          push.branch_name = branch_name + '-patch'
+          push.file_name = 'CODEOWNERS'
+          push.file_content = <<~CONTENT
             CODEOWNERS @#{non_approver.username}
           CONTENT
         end
 
-        merge_request.visit!
+        # Create a merge request
+        Resource::MergeRequest.fabricate! do |merge_request|
+          merge_request.project = project
+          merge_request.target_new_branch = false
+          merge_request.source_branch = branch_name + '-patch'
+          merge_request.target_branch = branch_name
+          merge_request.no_preparation = true
+        end.visit!
+
         # Check that the merge request assigns the original code owner as an
         # approver (because the current CODEOWNERS file in the master branch
         # doesn't have the new owner yet)
-        Page::MergeRequest::Show.perform do |mr_page|
-          mr_page.edit!
-          expect(mr_page.approvers).to include(approver.name)
-          expect(mr_page.approvers).not_to include(non_approver.name)
+        Page::MergeRequest::Show.perform do |show|
+          show.edit!
+          approvers = show.approvers
+
+          expect(approvers).to include(approver.name)
+          expect(approvers).not_to include(non_approver.name)
         end
       end
     end