Add latest changes from gitlab-org/gitlab@master

5 files changed, 62 insertions, 45 deletions

diff --git a/doc/administration/instance_review.md b/doc/administration/instance_review.md
index cee27ea99f7..f444589bdf3 100644
--- a/doc/administration/instance_review.md
+++ b/doc/administration/instance_review.md
@@ -6,8 +6,6 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 
 # Instance review **(FREE SELF)**
 
-> Introduced in GitLab 11.3.
-
 If you run a self-managed instance with 50 or more users on the Free tier
 ([either Community Edition or unlicensed Enterprise Edition](https://about.gitlab.com/install/ce-or-ee/)),
 you can request a free instance review.
diff --git a/lib/gitlab/lfs/client.rb b/lib/gitlab/lfs/client.rb
index a05e8107cad..55d47530593 100644
--- a/lib/gitlab/lfs/client.rb
+++ b/lib/gitlab/lfs/client.rb
@@ -36,7 +36,7 @@ module Gitlab
           headers: build_request_headers
         )
 
-        raise BatchSubmitError unless rsp.success?
+        raise BatchSubmitError.new(http_response: rsp) unless rsp.success?
 
         # HTTParty provides rsp.parsed_response, but it only kicks in for the
         # application/json content type in the response, which we can't rely on
@@ -65,7 +65,7 @@ module Gitlab
 
         rsp = Gitlab::HTTP.put(upload_action['href'], params)
 
-        raise ObjectUploadError unless rsp.success?
+        raise ObjectUploadError.new(http_response: rsp) unless rsp.success?
       ensure
         file&.close
       end
@@ -81,7 +81,7 @@ module Gitlab
 
         rsp = Gitlab::HTTP.post(verify_action['href'], params)
 
-        raise ObjectVerifyError unless rsp.success?
+        raise ObjectVerifyError.new(http_response: rsp) unless rsp.success?
       end
 
       private
@@ -105,9 +105,21 @@ module Gitlab
         { username: credentials[:user], password: credentials[:password] }
       end
 
-      class BatchSubmitError < StandardError
+      class HttpError < StandardError
+        def initialize(http_response:)
+          super
+
+          @http_response = http_response
+        end
+
+        def http_error
+          "HTTP status #{@http_response.code}"
+        end
+      end
+
+      class BatchSubmitError < HttpError
         def message
-          "Failed to submit batch"
+          "Failed to submit batch: #{http_error}"
         end
       end
 
@@ -122,15 +134,15 @@ module Gitlab
         end
       end
 
-      class ObjectUploadError < StandardError
+      class ObjectUploadError < HttpError
         def message
-          "Failed to upload object"
+          "Failed to upload object: #{http_error}"
         end
       end
 
-      class ObjectVerifyError < StandardError
+      class ObjectVerifyError < HttpError
         def message
-          "Failed to verify object"
+          "Failed to verify object: #{http_error}"
         end
       end
     end
diff --git a/qa/qa/resource/issue.rb b/qa/qa/resource/issue.rb
index 14d138a5558..1e38de97c1e 100644
--- a/qa/qa/resource/issue.rb
+++ b/qa/qa/resource/issue.rb
@@ -3,7 +3,7 @@
 module QA
   module Resource
     class Issue < Base
-      attr_writer :description, :milestone, :template, :weight
+      attr_writer :milestone, :template, :weight
 
       attribute :project do
         Project.fabricate! do |resource|
@@ -95,6 +95,15 @@ module QA
         )
       end
 
+      # Create a new comment
+      #
+      # @param [String] body
+      # @param [Boolean] confidential
+      # @return [Hash]
+      def add_comment(body:, confidential: false)
+        api_post_to(api_comments_path, body: body, confidential: confidential)
+      end
+
       protected
 
       # Return subset of fields for comparing issues
diff --git a/qa/qa/specs/features/api/1_manage/bulk_import_project_spec.rb b/qa/qa/specs/features/api/1_manage/bulk_import_project_spec.rb
index d36862b3c93..53d96e13d5e 100644
--- a/qa/qa/specs/features/api/1_manage/bulk_import_project_spec.rb
+++ b/qa/qa/specs/features/api/1_manage/bulk_import_project_spec.rb
@@ -5,9 +5,9 @@ module QA
   # on staging environment
   RSpec.describe 'Manage', :requires_admin, except: { subdomain: :staging } do
     describe 'Gitlab migration', quarantine: {
-      only: { job: "praefect-parallel" },
+      only: { job: 'praefect-parallel' },
       type: :investigating,
-      issue: "https://gitlab.com/gitlab-org/gitlab/-/issues/348999"
+      issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/348999'
     } do
       let(:source_project_with_readme) { false }
       let(:import_wait_duration) { { max_duration: 300, sleep_interval: 2 } }
@@ -93,7 +93,7 @@ module QA
       end
 
       context 'with project issues' do
-        let(:source_issue) do
+        let!(:source_issue) do
           Resource::Issue.fabricate_via_api! do |issue|
             issue.api_client = api_client
             issue.project = source_project
@@ -101,9 +101,9 @@ module QA
           end
         end
 
-        let(:imported_issues) do
-          imported_projects.first.issues
-        end
+        let!(:source_comment) { source_issue.add_comment(body: 'This is a test comment!') }
+
+        let(:imported_issues) { imported_projects.first.issues }
 
         let(:imported_issue) do
           issue = imported_issues.first
@@ -114,9 +114,7 @@ module QA
           end
         end
 
-        before do
-          source_issue # fabricate source group, project, issue
-        end
+        let(:imported_comments) { imported_issue.comments }
 
         it(
           'successfully imports issue',
@@ -124,8 +122,13 @@ module QA
         ) do
           expect_import_finished
 
-          expect(imported_issues.count).to eq(1)
-          expect(imported_issue).to eq(source_issue)
+          aggregate_failures do
+            expect(imported_issues.count).to eq(1)
+            expect(imported_issue).to eq(source_issue.reload!)
+
+            expect(imported_comments.count).to eq(1)
+            expect(imported_comments.first[:body]).to include(source_comment[:body])
+          end
         end
       end
 
@@ -198,22 +201,25 @@ module QA
       end
 
       context 'with merge request' do
-        let(:source_project_with_readme) { true }
-
-        let(:other_user) do
-          Resource::User.fabricate_via_api! do |usr|
-            usr.api_client = admin_api_client
-          end
+        let!(:source_project_with_readme) { true }
+
+        let!(:other_user) do
+          Resource::User
+            .fabricate_via_api! { |usr| usr.api_client = admin_api_client }
+            .tap do |usr|
+              usr.set_public_email
+              source_project.add_member(usr, Resource::Members::AccessLevel::MAINTAINER)
+            end
         end
 
-        let(:source_mr) do
+        let!(:source_mr) do
           Resource::MergeRequest.fabricate_via_api! do |mr|
             mr.project = source_project
             mr.api_client = Runtime::API::Client.new(user: other_user)
           end
         end
 
-        let(:source_comment) { source_mr.add_comment("This is a test comment!") }
+        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 }
@@ -226,21 +232,13 @@ module QA
           end
         end
 
-        before do
-          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(
           'successfully imports merge request',
-          tesecase: "https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/348478"
+          tesecase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/348478'
         ) do
           expect_import_finished
 
@@ -248,7 +246,7 @@ module QA
             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.comparable.except(:author)).to eq(source_mr.reload!.comparable.except(:author))
 
             expect(imported_mr_comments.count).to eq(1)
             expect(imported_mr_comments.first[:body]).to include(source_comment[:body])
diff --git a/spec/lib/gitlab/lfs/client_spec.rb b/spec/lib/gitlab/lfs/client_spec.rb
index 0f9637e8ca4..fa5ff529d17 100644
--- a/spec/lib/gitlab/lfs/client_spec.rb
+++ b/spec/lib/gitlab/lfs/client_spec.rb
@@ -159,7 +159,7 @@ RSpec.describe Gitlab::Lfs::Client do
       it 'raises an error' do
         stub_upload(object: object, headers: upload_action['header']).to_return(status: 400)
 
-        expect { lfs_client.upload!(object, upload_action, authenticated: true) }.to raise_error(/Failed/)
+        expect { lfs_client.upload!(object, upload_action, authenticated: true) }.to raise_error(/Failed to upload object: HTTP status 400/)
       end
     end
 
@@ -167,7 +167,7 @@ RSpec.describe Gitlab::Lfs::Client do
       it 'raises an error' do
         stub_upload(object: object, headers: upload_action['header']).to_return(status: 500)
 
-        expect { lfs_client.upload!(object, upload_action, authenticated: true) }.to raise_error(/Failed/)
+        expect { lfs_client.upload!(object, upload_action, authenticated: true) }.to raise_error(/Failed to upload object: HTTP status 500/)
       end
     end
 
@@ -226,7 +226,7 @@ RSpec.describe Gitlab::Lfs::Client do
       it 'raises an error' do
         stub_verify(object: object, headers: verify_action['header']).to_return(status: 400)
 
-        expect { lfs_client.verify!(object, verify_action, authenticated: true) }.to raise_error(/Failed/)
+        expect { lfs_client.verify!(object, verify_action, authenticated: true) }.to raise_error(/Failed to verify object: HTTP status 400/)
       end
     end
 
@@ -234,7 +234,7 @@ RSpec.describe Gitlab::Lfs::Client do
       it 'raises an error' do
         stub_verify(object: object, headers: verify_action['header']).to_return(status: 500)
 
-        expect { lfs_client.verify!(object, verify_action, authenticated: true) }.to raise_error(/Failed/)
+        expect { lfs_client.verify!(object, verify_action, authenticated: true) }.to raise_error(/Failed to verify object: HTTP status 500/)
       end
     end