Commit 47f1423d authored by Mark Lapierre's avatar Mark Lapierre Committed by Sanad Liaquat

Extend a test to cover the MR resolve button fix

See: https://gitlab.com/gitlab-org/gitlab/issues/32157

Adds a couple of steps to an existing test to provide additional
cover for the fix in the MR above.

Updates the test to be more consistent with our standards.

Updates Resource::Repository::Commit add or update files, and
updates the one test that was using that resource.
parent 05db0a6a
...@@ -58,6 +58,7 @@ export default { ...@@ -58,6 +58,7 @@ export default {
<div class="btn-group"> <div class="btn-group">
<resolve-discussion-button <resolve-discussion-button
v-if="discussion.resolvable" v-if="discussion.resolvable"
data-qa-selector="resolve_discussion_button"
:is-resolving="isResolving" :is-resolving="isResolving"
:button-title="resolveButtonTitle" :button-title="resolveButtonTitle"
@onClick="$emit('resolve')" @onClick="$emit('resolve')"
......
...@@ -306,7 +306,11 @@ export default { ...@@ -306,7 +306,11 @@ export default {
<template> <template>
<timeline-entry-item class="note note-discussion"> <timeline-entry-item class="note note-discussion">
<div class="timeline-content"> <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-if="shouldRenderDiffs" class="discussion-header note-wrapper">
<div v-once class="timeline-icon align-self-start flex-shrink-0"> <div v-once class="timeline-icon align-self-start flex-shrink-0">
<user-avatar-link <user-avatar-link
......
...@@ -151,6 +151,11 @@ module QA ...@@ -151,6 +151,11 @@ module QA
end end
def within_element_by_index(name, index) 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 page.within all_elements(name)[index] do
yield yield
end end
......
...@@ -10,6 +10,10 @@ module QA ...@@ -10,6 +10,10 @@ module QA
element :discussion_option element :discussion_option
end 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 base.view 'app/assets/javascripts/notes/components/note_actions.vue' do
element :note_edit_button element :note_edit_button
end end
...@@ -21,6 +25,7 @@ module QA ...@@ -21,6 +25,7 @@ module QA
base.view 'app/assets/javascripts/notes/components/discussion_actions.vue' do base.view 'app/assets/javascripts/notes/components/discussion_actions.vue' do
element :discussion_reply_tab element :discussion_reply_tab
element :resolve_discussion_button
end end
base.view 'app/assets/javascripts/notes/components/toggle_replies_widget.vue' do base.view 'app/assets/javascripts/notes/components/toggle_replies_widget.vue' do
...@@ -54,6 +59,12 @@ module QA ...@@ -54,6 +59,12 @@ module QA
click_element :reply_comment_button click_element :reply_comment_button
end 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 def collapse_replies
click_element :collapse_replies click_element :collapse_replies
end end
......
...@@ -21,14 +21,16 @@ module QA ...@@ -21,14 +21,16 @@ module QA
@commit_message = 'QA Test - Commit message' @commit_message = 'QA Test - Commit message'
end end
def files=(files) def add_files(files)
if !files.is_a?(Array) || validate_files!(files)
files.empty? ||
files.any? { |file| !file.has_key?(:file_path) || !file.has_key?(:content) } @add_files = files
raise ArgumentError, "Please provide an array of hashes e.g.: [{file_path: 'file1', content: 'foo'}]" end
end
def update_files(files)
validate_files!(files)
@files = files @update_files = files
end end
def resource_web_url(resource) def resource_web_url(resource)
...@@ -56,8 +58,17 @@ module QA ...@@ -56,8 +58,17 @@ module QA
end end
def actions def actions
@files.map do |file| @add_files.map { |file| file.merge({ action: "create" }) } if @add_files
file.merge({ action: "create" }) @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 end
end end
......
...@@ -3,71 +3,99 @@ ...@@ -3,71 +3,99 @@
module QA module QA
context 'Create' do context 'Create' do
describe 'batch comments in merge request' do describe 'batch comments in merge request' do
it 'user submits, discards batch comments' do let(:project) do
Runtime::Browser.visit(:gitlab, Page::Main::Login) Resource::Project.fabricate_via_api! do |project|
Page::Main::Login.perform(&:sign_in_using_credentials)
project = Resource::Project.fabricate! do |project|
project.name = 'project-with-merge-request' project.name = 'project-with-merge-request'
end end
end
mr = Resource::MergeRequest.fabricate! do |merge_request| let(:merge_request) do
Resource::MergeRequest.fabricate_via_api! do |merge_request|
merge_request.title = 'This is a merge request' merge_request.title = 'This is a merge request'
merge_request.description = 'Great feature' merge_request.description = 'Great feature'
merge_request.project = project merge_request.project = project
end 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| Page::MergeRequest::Show.perform do |show|
show_page.click_discussions_tab show.click_discussions_tab
show_page.start_discussion("I'm starting a new discussion") show.start_discussion("I'm starting a new discussion")
expect(show_page).to have_content("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.type_reply_to_discussion("Could you please check this?")
show_page.comment_now show.comment_now
expect(show_page).to have_content("Could you please check this?") expect(show).to have_content("Could you please check this?")
expect(show_page).to have_content("0/1 thread resolved") expect(show).to have_content("0/1 thread resolved")
show_page.type_reply_to_discussion("Could you also check that?") show.type_reply_to_discussion("Could you also check that?")
show_page.resolve_review_discussion show.resolve_review_discussion
show_page.start_review show.start_review
expect(show_page).to have_content("Could you also check that?") expect(show).to have_content("Could you also check that?")
expect(show_page).to have_content("Finish review 1") 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.add_comment_to_diff("Can you check this line of code?")
show_page.comment_now show.comment_now
expect(show_page).to have_content("Can you check this line of code?") expect(show).to have_content("Can you check this line of code?")
show_page.type_reply_to_discussion("And this syntax as well?") show.type_reply_to_discussion("And this syntax as well?")
show_page.resolve_review_discussion show.resolve_review_discussion
show_page.start_review show.start_review
expect(show_page).to have_content("And this syntax as well?") expect(show).to have_content("And this syntax as well?")
expect(show_page).to have_content("Finish review 2") expect(show).to have_content("Finish review 2")
show_page.submit_pending_reviews show.submit_pending_reviews
expect(show_page).to have_content("2/2 threads resolved") expect(show).to have_content("2/2 threads resolved")
show_page.toggle_comments show.toggle_comments
show_page.type_reply_to_discussion("Unresolving this discussion") show.type_reply_to_discussion("Unresolving this discussion")
show_page.unresolve_review_discussion show.unresolve_review_discussion
show_page.comment_now show.comment_now
expect(show_page).to have_content("1/2 threads resolved") expect(show).to have_content("1/2 threads resolved")
end end
Page::MergeRequest::Show.perform do |show_page| Page::MergeRequest::Show.perform do |show|
show_page.click_discussions_tab 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") expect(show).to have_content("Finish review 1")
show_page.start_review 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") merge_request.visit!
show_page.discard_pending_reviews 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 end
end end
......
...@@ -13,15 +13,17 @@ module QA ...@@ -13,15 +13,17 @@ module QA
Resource::Repository::Commit.fabricate_via_api! do |commit| Resource::Repository::Commit.fabricate_via_api! do |commit|
commit.project = project commit.project = project
commit.commit_message = 'Add .gitlab/.gitlab-webide.yml' commit.commit_message = 'Add .gitlab/.gitlab-webide.yml'
commit.files = [ commit.add_files(
{ [
{
file_path: '.gitlab/.gitlab-webide.yml', file_path: '.gitlab/.gitlab-webide.yml',
content: <<~YAML content: <<~YAML
terminal: terminal:
script: sleep 60 script: sleep 60
YAML YAML
} }
] ]
)
end end
@runner = Resource::Runner.fabricate_via_api! do |runner| @runner = Resource::Runner.fabricate_via_api! do |runner|
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment