Merge branch 'ml-extend-test-to-cover-missing-resolve-button' into 'master'

Extend an E2E test to cover the MR resolve button fix

Closes gitlab-org/quality/testcases#239

See merge request gitlab-org/gitlab!17669

app/assets/javascripts/notes/components/discussion_actions.vue

@@ -58,6 +58,7 @@ export default {
       <div class="btn-group">
         <resolve-discussion-button
           v-if="discussion.resolvable"
+          data-qa-selector="resolve_discussion_button"
           :is-resolving="isResolving"
           :button-title="resolveButtonTitle"
           @onClick="$emit('resolve')"

app/assets/javascripts/notes/components/noteable_discussion.vue

@@ -306,7 +306,11 @@ export default {
 <template>
   <timeline-entry-item class="note note-discussion">
     <div class="timeline-content">
-      <div :data-discussion-id="discussion.id" class="discussion js-discussion-container">
+      <div
+        :data-discussion-id="discussion.id"
+        class="discussion js-discussion-container"
+        data-qa-selector="discussion_content"
+      >
         <div v-if="shouldRenderDiffs" class="discussion-header note-wrapper">
           <div v-once class="timeline-icon align-self-start flex-shrink-0">
             <user-avatar-link

qa/qa/page/base.rb

@@ -151,6 +151,11 @@ module QA
       end
 
       def within_element_by_index(name, index)
+        # Finding all elements can be flaky if the elements don't all load
+        # immediately. So we wait for any to appear before trying to find a
+        # specific one.
+        has_element?(name)
+
         page.within all_elements(name)[index] do
           yield
         end

qa/qa/page/component/note.rb

@@ -10,6 +10,10 @@ module QA
             element :discussion_option
           end
 
+          base.view 'app/assets/javascripts/notes/components/noteable_discussion.vue' do
+            element :discussion_content
+          end
+
           base.view 'app/assets/javascripts/notes/components/note_actions.vue' do
             element :note_edit_button
           end
@@ -21,6 +25,7 @@ module QA
 
           base.view 'app/assets/javascripts/notes/components/discussion_actions.vue' do
             element :discussion_reply_tab
+            element :resolve_discussion_button
           end
 
           base.view 'app/assets/javascripts/notes/components/toggle_replies_widget.vue' do
@@ -54,6 +59,12 @@ module QA
           click_element :reply_comment_button
         end
 
+        def resolve_discussion_at_index(index)
+          within_element_by_index(:discussion_content, index) do
+            click_element :resolve_discussion_button
+          end
+        end
+
         def collapse_replies
           click_element :collapse_replies
         end

qa/qa/resource/repository/commit.rb

@@ -21,14 +21,16 @@ module QA
           @commit_message = 'QA Test - Commit message'
         end
 
-        def files=(files)
-          if !files.is_a?(Array) ||
-              files.empty? ||
-              files.any? { |file| !file.has_key?(:file_path) || !file.has_key?(:content) }
-            raise ArgumentError, "Please provide an array of hashes e.g.: [{file_path: 'file1', content: 'foo'}]"
-          end
+        def add_files(files)
+          validate_files!(files)
+
+          @add_files = files
+        end
+
+        def update_files(files)
+          validate_files!(files)
 
-          @files = files
+          @update_files = files
         end
 
         def resource_web_url(resource)
@@ -56,8 +58,17 @@ module QA
         end
 
         def actions
-          @files.map do |file|
-            file.merge({ action: "create" })
+          @add_files.map { |file| file.merge({ action: "create" }) } if @add_files
+          @update_files.map { |file| file.merge({ action: "update" }) } if @update_files
+        end
+
+        private
+
+        def validate_files!(files)
+          if !files.is_a?(Array) ||
+              files.empty? ||
+              files.any? { |file| !file.has_key?(:file_path) || !file.has_key?(:content) }
+            raise ArgumentError, "Please provide an array of hashes e.g.: [{file_path: 'file1', content: 'foo'}]"
           end
         end
       end

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

@@ -3,71 +3,99 @@
 module QA
   context 'Create' do
     describe 'batch comments in merge request' do
-      it 'user submits, discards batch comments' do
-        Runtime::Browser.visit(:gitlab, Page::Main::Login)
-        Page::Main::Login.perform(&:sign_in_using_credentials)
-
-        project = Resource::Project.fabricate! do |project|
+      let(:project) do
+        Resource::Project.fabricate_via_api! do |project|
           project.name = 'project-with-merge-request'
         end
-
-        mr = Resource::MergeRequest.fabricate! do |merge_request|
+      end
+      let(:merge_request) do
+        Resource::MergeRequest.fabricate_via_api! do |merge_request|
           merge_request.title = 'This is a merge request'
           merge_request.description = 'Great feature'
           merge_request.project = project
         end
+      end
+
+      it 'user submits, discards batch comments' do
+        Runtime::Browser.visit(:gitlab, Page::Main::Login)
+        Page::Main::Login.perform(&:sign_in_using_credentials)
 
-        mr.visit!
+        merge_request.visit!
 
-        Page::MergeRequest::Show.perform do |show_page|
-          show_page.click_discussions_tab
+        Page::MergeRequest::Show.perform do |show|
+          show.click_discussions_tab
 
-          show_page.start_discussion("I'm starting a new discussion")
-          expect(show_page).to have_content("I'm starting a new discussion")
+          show.start_discussion("I'm starting a new discussion")
+          expect(show).to have_content("I'm starting a new discussion")
 
-          show_page.type_reply_to_discussion("Could you please check this?")
-          show_page.comment_now
-          expect(show_page).to have_content("Could you please check this?")
-          expect(show_page).to have_content("0/1 thread resolved")
+          show.type_reply_to_discussion("Could you please check this?")
+          show.comment_now
+          expect(show).to have_content("Could you please check this?")
+          expect(show).to have_content("0/1 thread resolved")
 
-          show_page.type_reply_to_discussion("Could you also check that?")
-          show_page.resolve_review_discussion
-          show_page.start_review
-          expect(show_page).to have_content("Could you also check that?")
-          expect(show_page).to have_content("Finish review 1")
+          show.type_reply_to_discussion("Could you also check that?")
+          show.resolve_review_discussion
+          show.start_review
+          expect(show).to have_content("Could you also check that?")
+          expect(show).to have_content("Finish review 1")
 
-          show_page.click_diffs_tab
+          show.click_diffs_tab
 
-          show_page.add_comment_to_diff("Can you check this line of code?")
-          show_page.comment_now
-          expect(show_page).to have_content("Can you check this line of code?")
+          show.add_comment_to_diff("Can you check this line of code?")
+          show.comment_now
+          expect(show).to have_content("Can you check this line of code?")
 
-          show_page.type_reply_to_discussion("And this syntax as well?")
-          show_page.resolve_review_discussion
-          show_page.start_review
-          expect(show_page).to have_content("And this syntax as well?")
-          expect(show_page).to have_content("Finish review 2")
+          show.type_reply_to_discussion("And this syntax as well?")
+          show.resolve_review_discussion
+          show.start_review
+          expect(show).to have_content("And this syntax as well?")
+          expect(show).to have_content("Finish review 2")
 
-          show_page.submit_pending_reviews
-          expect(show_page).to have_content("2/2 threads resolved")
+          show.submit_pending_reviews
+          expect(show).to have_content("2/2 threads resolved")
 
-          show_page.toggle_comments
-          show_page.type_reply_to_discussion("Unresolving this discussion")
-          show_page.unresolve_review_discussion
-          show_page.comment_now
-          expect(show_page).to have_content("1/2 threads resolved")
+          show.toggle_comments
+          show.type_reply_to_discussion("Unresolving this discussion")
+          show.unresolve_review_discussion
+          show.comment_now
+          expect(show).to have_content("1/2 threads resolved")
         end
 
-        Page::MergeRequest::Show.perform do |show_page|
-          show_page.click_discussions_tab
+        Page::MergeRequest::Show.perform do |show|
+          show.click_discussions_tab
+
+          show.type_reply_to_discussion("Planning to discard this comment")
+          show.start_review
 
-          show_page.type_reply_to_discussion("Planning to discard this comment")
-          show_page.start_review
+          expect(show).to have_content("Finish review 1")
+          show.discard_pending_reviews
+
+          expect(show).not_to have_content("Planning to discard this comment")
+        end
+
+        # Overwrite the added file to create a system note as required to
+        # trigger the bug described here: https://gitlab.com/gitlab-org/gitlab/issues/32157
+        commit_message = 'Update file'
+        Resource::Repository::Commit.fabricate_via_api! do |commit|
+          commit.project = project
+          commit.commit_message = commit_message
+          commit.branch = merge_request.source_branch
+          commit.update_files(
+            [
+              {
+                file_path: 'added_file.txt',
+                content: "File updated"
+              }
+            ]
+          )
+        end
+        project.wait_for_push(commit_message)
 
-          expect(show_page).to have_content("Finish review 1")
-          show_page.discard_pending_reviews
+        merge_request.visit!
+        Page::MergeRequest::Show.perform do |show|
+          show.resolve_discussion_at_index(1)
 
-          expect(show_page).not_to have_content("Planning to discard this comment")
+          expect(show).to have_content("2/2 threads resolved")
         end
       end
     end

qa/qa/specs/features/ee/browser_ui/3_create/web_ide/web_terminal_spec.rb

@@ -13,15 +13,17 @@ module QA
         Resource::Repository::Commit.fabricate_via_api! do |commit|
           commit.project = project
           commit.commit_message = 'Add .gitlab/.gitlab-webide.yml'
-          commit.files = [
-            {
+          commit.add_files(
+            [
+              {
                 file_path: '.gitlab/.gitlab-webide.yml',
                 content: <<~YAML
                   terminal:
                     script: sleep 60
                 YAML
-            }
-          ]
+              }
+            ]
+          )
         end
 
         @runner = Resource::Runner.fabricate_via_api! do |runner|