Merge branch 'ml-push-pipeline-commit-just-before-mwps' into 'master'

Push commit to start pipeline as late as possible before MWPS

See merge request gitlab-org/gitlab!78977

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -578,7 +578,7 @@ export default {
         :endpoint="mr.accessibilityReportPath"
       />
 
-      <div class="mr-widget-section">
+      <div class="mr-widget-section" data-qa-selector="mr_widget_content">
         <component :is="componentName" :mr="mr" :service="service" />
         <ready-to-merge
           v-if="isRestructuredMrWidgetEnabled && mr.commitsCount"

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

@@ -15,10 +15,6 @@ module QA
             base.class_eval do
               prepend Page::Component::LicenseManagement
 
-              view 'app/assets/javascripts/vue_merge_request_widget/components/states/sha_mismatch.vue' do
-                element :head_mismatch_content
-              end
-
               view 'ee/app/views/projects/merge_requests/_code_owner_approval_rules.html.haml' do
                 element :approver_content
                 element :approver_list_content
@@ -225,12 +221,7 @@ module QA
           end
 
           def merge_via_merge_train
-            # Revisit after merge page re-architect is done https://gitlab.com/gitlab-org/gitlab/-/issues/300042
-            # To remove page refresh logic if possible
-            wait_until_ready_to_merge
-            wait_until { !find_element(:merge_button).has_text?("when pipeline succeeds") }
-
-            click_element(:merge_button)
+            try_to_merge!
 
             finished_loading?
           end

qa/qa/page/merge_request/show.rb

@@ -83,10 +83,18 @@ module QA
           element :merge_immediately_menu_item
         end
 
+        view 'app/assets/javascripts/vue_merge_request_widget/components/states/sha_mismatch.vue' do
+          element :head_mismatch_content
+        end
+
         view 'app/assets/javascripts/vue_merge_request_widget/components/states/squash_before_merge.vue' do
           element :squash_checkbox
         end
 
+        view 'app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue' do
+          element :mr_widget_content
+        end
+
         view 'app/assets/javascripts/vue_shared/components/markdown/apply_suggestion.vue' do
           element :apply_suggestion_dropdown
           element :commit_message_field
@@ -269,13 +277,29 @@ module QA
           has_element?(:merge_button, disabled: false)
         end
 
-        # Waits up 60 seconds and raises an error if unable to merge
-        def wait_until_ready_to_merge
-          has_element?(:merge_button)
+        # Waits up 60 seconds and raises an error if unable to merge.
+        #
+        # If a state is encountered in which a user would typically refresh the page, this will refresh the page and
+        # then check again if it's ready to merge. For example, it will refresh if a new change was pushed and the page
+        # needs to be refreshed to show the change.
+        #
+        # @param [Boolean] transient_test true if the current test is a transient test (default: false)
+        def wait_until_ready_to_merge(transient_test: false)
+          wait_until do
+            has_element?(:merge_button)
 
-          # The merge button is enabled via JS
-          wait_until(reload: false) do
-            !find_element(:merge_button).disabled?
+            break true unless find_element(:merge_button).disabled?
+
+            # If the widget shows "Merge blocked: new changes were just added" we can refresh the page and check again
+            next false if has_element?(:head_mismatch_content)
+
+            # Stop waiting if we're in a transient test. By this point we're in an unexpected state and should let the
+            # test fail so we can investigate. If we're not in a transient test we keep trying until we reach timeout.
+            next true unless transient_test
+
+            QA::Runtime::Logger.debug("MR widget text: #{mr_widget_text}")
+
+            false
           end
         end
 
@@ -385,6 +409,10 @@ module QA
         def cancel_auto_merge!
           click_element(:cancel_auto_merge_button)
         end
+
+        def mr_widget_text
+          find_element(:mr_widget_content).text
+        end
       end
     end
   end

qa/qa/specs/features/browser_ui/3_create/merge_request/merge_when_pipeline_succeeds_spec.rb

@@ -20,25 +20,6 @@ module QA
         end
 
         before do
-          Resource::Repository::Commit.fabricate_via_api! do |commit|
-            commit.project = project
-            commit.commit_message = 'Add .gitlab-ci.yml'
-            commit.add_files(
-              [
-                {
-                  file_path: '.gitlab-ci.yml',
-                  content: <<~EOF
-                    test:
-                      tags: ["runner-for-#{project.name}"]
-                      script: sleep 20
-                      only:
-                        - merge_requests
-                  EOF
-                }
-              ]
-            )
-          end
-
           Flow::Login.sign_in
         end
 
@@ -48,8 +29,10 @@ module QA
         end
 
         it 'merges after pipeline succeeds' do
+          transient_test = repeat > 1
+
           repeat.times do |i|
-            QA::Runtime::Logger.info("Transient bug test - Trial #{i}") if repeat > 1
+            QA::Runtime::Logger.info("Transient bug test - Trial #{i}") if transient_test
 
             branch_name = "mr-test-#{SecureRandom.hex(6)}-#{i}"
 
@@ -68,19 +51,59 @@ module QA
               merge_request.no_preparation = true
             end
 
+            # Load the page so that the browser is as prepared as possible to display the pipeline in progress when we
+            # start it.
             merge_request.visit!
 
+            # Push a new pipeline config file
+            Resource::Repository::Commit.fabricate_via_api! do |commit|
+              commit.project = project
+              commit.commit_message = 'Add .gitlab-ci.yml'
+              commit.branch = branch_name
+              commit.add_files(
+                [
+                  {
+                    file_path: '.gitlab-ci.yml',
+                    content: <<~EOF
+                      test:
+                        tags: ["runner-for-#{project.name}"]
+                        script: sleep 20
+                        only:
+                          - merge_requests
+                    EOF
+                  }
+                ]
+              )
+            end
+
             Page::MergeRequest::Show.perform do |mr|
-              mr.merge_when_pipeline_succeeds!
+              refresh
 
-              Support::Waiter.wait_until(sleep_interval: 5) do
+              # Part of the challenge with this test is that the MR widget has many components that could be displayed
+              # and many errors states that those components could encounter. Most of the time few of those
+              # possible components will be relevant, so it would be inefficient for this test to check for each of
+              # them. Instead, we fail on anything but the expected state.
+              #
+              # The following method allows us to handle and ignore states (as we find them) that users could safely ignore.
+              mr.wait_until_ready_to_merge(transient_test: transient_test)
+
+              mr.retry_until(reload: true, message: 'Wait until ready to click MWPS') do
                 merge_request = merge_request.reload!
-                merge_request.state == 'merged'
+
+                # Don't try to click MWPS if the MR is merged or the pipeline is complete
+                break if merge_request.state == 'merged' || project.pipelines.last[:status] == 'success'
+
+                # Try to click MWPS if this is a transient test, or if the MWPS button is visible,
+                # otherwise reload the page and retry
+                next false unless transient_test || mr.has_element?(:merge_button, text: 'Merge when pipeline succeeds')
+
+                # No need to keep retrying if we can click MWPS
+                break mr.merge_when_pipeline_succeeds!
               end
 
               aggregate_failures do
-                expect(merge_request.merge_when_pipeline_succeeds).to be_truthy
                 expect(mr.merged?).to be_truthy, "Expected content 'The changes were merged' but it did not appear."
+                expect(merge_request.reload!.merge_when_pipeline_succeeds).to be_truthy
               end
             end
           end