Merge branch 'acunskis-mr-comments-import' into 'master'

E2E: Validate merge request comments are migrated

See merge request gitlab-org/gitlab!77170

qa/qa/resource/merge_request.rb

@@ -168,7 +168,13 @@ module QA
         )
       end
 
-      protected
+      # Add mr comment
+      #
+      # @param [String] body
+      # @return [Hash]
+      def add_comment(body)
+        api_post_to(api_comments_path, body: body)
+      end
 
       # Return subset of fields for comparing merge requests
       #
@@ -182,7 +188,9 @@ module QA
           :project_id,
           :source_project_id,
           :target_project_id,
+          :merge_status,
           # these can differ depending on user fetching mr
+          :user,
           :subscribed,
           :first_contribution
         ).merge({ references: api_resource[:references].except(:full) })

qa/qa/specs/features/ee/api/1_manage/bulk_import_project_spec.rb → qa/qa/specs/features/api/1_manage/bulk_import_project_spec.rb

@@ -5,6 +5,7 @@ module QA
   # on staging environment
   RSpec.describe 'Manage', :requires_admin, except: { subdomain: :staging } do
     describe 'Gitlab migration' do
+      let(:source_project_with_readme) { false }
       let(:import_wait_duration) { { max_duration: 300, sleep_interval: 2 } }
       let(:admin_api_client) { Runtime::API::Client.as_admin }
       let(:user) do
@@ -33,7 +34,7 @@ module QA
         Resource::Project.fabricate_via_api! do |project|
           project.api_client = api_client
           project.group = source_group
-          project.initialize_with_readme = true
+          project.initialize_with_readme = source_project_with_readme
         end
       end
 
@@ -63,8 +64,7 @@ module QA
         Runtime::Feature.enable(:bulk_import_projects)
 
         sandbox.add_member(user, Resource::Members::AccessLevel::MAINTAINER)
-
-        source_project.tap { |project| project.add_push_rules(member_check: true) } # fabricate source group and project
+        source_project # fabricate source group and project
       end
 
       after do |example|
@@ -126,6 +126,7 @@ module QA
       end
 
       context 'with repository' do
+        let(:source_project_with_readme) { true }
         let(:source_commits) { source_project.commits.map { |c| c.except(:web_url) } }
         let(:source_tags) do
           source_project.repository_tags.tap do |tags|
@@ -193,27 +194,42 @@ module QA
       end
 
       context 'with merge request' do
+        let(:other_user) do
+          Resource::User.fabricate_via_api! do |usr|
+            usr.api_client = admin_api_client
+          end
+        end
+
         let(:source_mr) do
           Resource::MergeRequest.fabricate_via_api! do |mr|
-            mr.no_preparation = true
             mr.project = source_project
-            mr.api_client = api_client
+            mr.api_client = Runtime::API::Client.new(user: other_user)
           end
         end
 
-        let(:imported_mrs) do
-          imported_project.merge_requests
-        end
+        let(:source_comment) { source_mr.add_comment("This is a test comment!") }
+
+        let(:imported_mrs) { imported_project.merge_requests }
+        let(:imported_mr_comments) { imported_mr.comments }
 
         let(:imported_mr) do
           Resource::MergeRequest.init do |mr|
             mr.project = imported_project
             mr.iid = imported_mrs.first[:iid]
+            mr.api_client = api_client
           end
         end
 
         before do
-          source_mr # fabricate mr for import
+          other_user.set_public_email
+          source_project.add_member(other_user, Resource::Members::AccessLevel::MAINTAINER)
+
+          source_comment # fabricate mr and comment
+          source_mr.reload! # update notes count attribute on object
+        end
+
+        after do
+          other_user.remove_via_api!
         end
 
         it(
@@ -222,8 +238,17 @@ module QA
         ) do
           expect_import_finished
 
-          expect(imported_mrs.count).to eq(1)
-          expect(imported_mr).to eq(source_mr)
+          aggregate_failures do
+            expect(imported_mrs.count).to eq(1)
+            # TODO: remove custom comparison after member migration is implemented
+            # https://gitlab.com/gitlab-org/gitlab/-/issues/341886
+            expect(imported_mr.comparable.except(:author)).to eq(source_mr.comparable.except(:author))
+
+            expect(imported_mr_comments.count).to eq(1)
+            expect(imported_mr_comments.first[:body]).to include(source_comment[:body])
+            # Comment will have mention of original user since members are not migrated yet
+            expect(imported_mr_comments.first[:body]).to include(other_user.name)
+          end
         end
       end
     end

qa/qa/support/matchers/eventually_matcher.rb

@@ -28,27 +28,21 @@ module QA
           RSpec::Matchers.define(:"eventually_#{op}") do |*expected|
             chain(:within) do |kwargs = {}|
               @retry_args = kwargs
-              @retry_args[:sleep_interval] = 0.5 unless @retry_args[:sleep_interval]
+              @retry_args[:sleep_interval] = 0.5 unless kwargs[:sleep_interval]
             end
 
-            def supports_block_expectations?
-              true
-            end
+            description { "eventually #{operator_msg}: #{expected_formatted}" }
 
             match { |actual| wait_and_check(actual, :default_expectation) }
 
             match_when_negated { |actual| wait_and_check(actual, :when_negated_expectation) }
 
-            description do
-              "eventually #{operator_msg} #{expected.inspect}"
-            end
+            failure_message { fail_message }
 
-            failure_message do
-              "#{e}:\nexpected to #{description}, last attempt was #{@result.nil? ? 'nil' : @result}"
-            end
+            failure_message_when_negated { fail_message(negate: true) }
 
-            failure_message_when_negated do
-              "#{e}:\nexpected not to #{description}, last attempt was #{@result.nil? ? 'nil' : @result}"
+            def supports_block_expectations?
+              true
             end
 
             # Execute rspec expectation within retrier
@@ -60,9 +54,9 @@ module QA
               attempt = 0
 
               QA::Runtime::Logger.debug(
-                "Running eventually matcher with '#{operator_msg}' operator with: #{@retry_args}"
+                "Running eventually matcher with '#{operator_msg}' operator with: '#{retry_args}' arguments"
               )
-              QA::Support::Retrier.retry_until(**@retry_args, log: false) do
+              QA::Support::Retrier.retry_until(**retry_args, log: false) do
                 QA::Runtime::Logger.debug("evaluating expectation, attempt: #{attempt += 1}")
 
                 public_send(expectation_name, actual)
@@ -132,6 +126,44 @@ module QA
                 [operator, expected]
               end
             end
+
+            # Custom retry arguments
+            #
+            # @return [Hash]
+            def retry_args
+              @retry_args ||= { sleep_interval: 0.5 }
+            end
+
+            # Custom failure message
+            #
+            # @param [Boolean] negate
+            # @return [String]
+            def fail_message(negate: false)
+              "#{e}:\n\nexpected #{negate ? 'not ' : ''}to #{description}\n\n"\
+              "last attempt was: #{@result.nil? ? 'nil' : actual_formatted}\n\n"\
+              "Diff:#{diff}"
+            end
+
+            # Formatted expect
+            #
+            # @return [String]
+            def expected_formatted
+              RSpec::Support::ObjectFormatter.format(expected)
+            end
+
+            # Formatted actual result
+            #
+            # @return [String]
+            def actual_formatted
+              RSpec::Support::ObjectFormatter.format(@result)
+            end
+
+            # Object diff
+            #
+            # @return [String]
+            def diff
+              RSpec::Support::Differ.new(color: true).diff(@result, expected)
+            end
           end
         end
       end