Merge branch 'ml-qa-make-all-elements-more-reliable' into 'master'

Update all_elements to require constraints

Closes gitlab-org/quality/team-tasks#86

See merge request gitlab-org/gitlab!22352

qa/qa/ee/page/group/epic/index.rb

@@ -29,7 +29,7 @@ module QA
             end
 
             def click_first_epic(page = nil)
-              all_elements(:epic_title_text).first.click
+              all_elements(:epic_title_text, minimum: 1).first.click
               page.validate_elements_present! if page
             end
 

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

@@ -106,7 +106,7 @@ module QA
 
           def approvers
             within_element :approver_list do
-              all_elements(:approver).map { |item| item.find('img')['title'] }
+              all_elements(:approver, minimum: 1).map { |item| item.find('img')['title'] }
             end
           end
 

qa/qa/page/base.rb

@@ -89,6 +89,10 @@ module QA
       end
 
       def all_elements(name, **kwargs)
+        if kwargs.keys.none? { |key| [:minimum, :maximum, :count, :between].include?(key) }
+          raise ArgumentError, "Please use :minimum, :maximum, :count, or :between so that all is more reliable"
+        end
+
         wait_for_requests
 
         all(element_selector_css(name), **kwargs)

qa/qa/page/component/note.rb

@@ -45,17 +45,17 @@ module QA
           click_element :comment_button
         end
 
-        def toggle_comments
-          all_elements(:toggle_comments_button).last.click
+        def toggle_comments(position)
+          all_elements(:toggle_comments_button, minimum: position)[position - 1].click
         end
 
-        def type_reply_to_discussion(reply_text)
-          all_elements(:discussion_reply_tab).last.click
+        def type_reply_to_discussion(position, reply_text)
+          all_elements(:discussion_reply_tab, minimum: position)[position - 1].click
           fill_element :reply_input, reply_text
         end
 
-        def reply_to_discussion(reply_text)
-          type_reply_to_discussion(reply_text)
+        def reply_to_discussion(position, reply_text)
+          type_reply_to_discussion(position, reply_text)
           click_element :reply_comment_button
         end
 

qa/qa/page/layout/performance_bar.rb

@@ -23,7 +23,7 @@ module QA
 
         def has_detailed_metrics?
           retry_until(sleep_interval: 1) do
-            all_elements(:detailed_metric_content).all? do |metric|
+            all_elements(:detailed_metric_content, count: count).all? do |metric|
               metric.has_text?(%r{\d+})
             end
           end

qa/qa/page/merge_request/show.rb

@@ -76,7 +76,7 @@ module QA
           wait(interval: 5) do
             has_text?("No newline at end of file")
           end
-          all_elements(:new_diff_line).first.hover
+          all_elements(:new_diff_line, minimum: 1).first.hover
           click_element(:diff_comment)
           fill_element(:reply_input, text)
         end

qa/qa/page/project/issue/index.rb

@@ -21,10 +21,6 @@ module QA
             element :closed_issues_link
           end
 
-          def assignee_link_count
-            all_elements(:assignee_link).count
-          end
-
           def avatar_counter
             find_element(:avatar_counter)
           end
@@ -37,6 +33,10 @@ module QA
             click_element :closed_issues_link
           end
 
+          def has_assignee_link_count?(count)
+            all_elements(:assignee_link, count: count)
+          end
+
           def has_issue?(issue)
             has_element? :issue, issue_title: issue.to_s
           end

qa/qa/page/project/issue/show.rb

@@ -56,12 +56,6 @@ module QA
             element :new_note_form, 'attr: :note' # rubocop:disable QA/ElementWithPattern
           end
 
-          def avatar_image_count
-            wait_assignees_block_finish_loading do
-              all_elements(:avatar_image).count
-            end
-          end
-
           def click_milestone_link
             click_element(:milestone_link)
           end
@@ -88,6 +82,12 @@ module QA
             click_element :comment_button
           end
 
+          def has_avatar_image_count?(count)
+            wait_assignees_block_finish_loading do
+              all_elements(:avatar_image, count: count)
+            end
+          end
+
           def has_comment?(comment_text)
             wait(reload: false) do
               has_element?(:noteable_note_item, text: comment_text)

qa/qa/page/project/settings/ci_variables.rb

@@ -20,13 +20,13 @@ module QA
           end
 
           def fill_variable(key, value, masked)
-            keys = all_elements(:ci_variable_input_key)
+            keys = all_elements(:ci_variable_input_key, minimum: 1)
             index = keys.size - 1
 
             # After we fill the key, JS would generate another field so
             # we need to use the same index to find the corresponding one.
             keys[index].set(key)
-            node = all_elements(:ci_variable_input_value)[index]
+            node = all_elements(:ci_variable_input_value, count: keys.size + 1)[index]
 
             # Simply run `node.set(value)` is too slow for long text here,
             # so we need to run JavaScript directly to set the value.
@@ -34,7 +34,7 @@ module QA
             # https://github.com/teamcapybara/capybara/blob/679548cea10773d45e32808f4d964377cfe5e892/lib/capybara/selenium/node.rb#L217
             execute_script("arguments[0].value = #{value.to_json}", node)
 
-            masked_node = all_elements(:variable_masked)[index]
+            masked_node = all_elements(:variable_masked, count: keys.size + 1)[index]
             toggle_masked(masked_node, masked)
           end
 

qa/qa/page/project/settings/mirroring_repositories.rb

@@ -118,7 +118,7 @@ module QA
 
           def find_repository_row_index(target_url)
             wait(max: 5, reload: false) do
-              all_elements(:mirror_repository_url_cell).index do |url|
+              all_elements(:mirror_repository_url_cell, minimum: 1).index do |url|
                 # The url might be a sanitized url but the target_url won't be so
                 # we compare just the paths instead of the full url
                 URI.parse(url.text).path == target_url.path

qa/qa/specs/features/browser_ui/2_plan/issue/collapse_comments_in_discussions_spec.rb

@@ -17,7 +17,7 @@ module QA
         Page::Project::Issue::Show.perform do |show|
           show.select_all_activities_filter
           show.start_discussion('My first discussion')
-          show.reply_to_discussion(my_first_reply)
+          show.reply_to_discussion(1, my_first_reply)
         end
       end
 

qa/qa/specs/features/browser_ui/non_devops/performance_bar_spec.rb

@@ -25,7 +25,7 @@ module QA
 
         Page::Layout::PerformanceBar.perform do |bar_component|
           expect(bar_component).to have_performance_bar
-          expect(bar_component).to have_detailed_metrics
+          expect(bar_component).to have_detailed_metrics(4)
           expect(bar_component).to have_request_for('realtime_changes') # Always requested on issue pages
         end
       end

qa/qa/specs/features/ee/browser_ui/2_plan/multiple_assignees_for_issues/four_assignees_spec.rb

@@ -38,7 +38,7 @@ module QA
         Page::Project::Menu.perform(&:click_issues)
 
         Page::Project::Issue::Index.perform do |index|
-          expect(index.assignee_link_count).to be 4
+          expect(index).to have_assignee_link_count(4)
         end
       end
     end

qa/qa/specs/features/ee/browser_ui/2_plan/multiple_assignees_for_issues/more_than_four_assignees_spec.rb

@@ -46,7 +46,7 @@ module QA
         Page::Project::Menu.perform(&:click_issues)
 
         Page::Project::Issue::Index.perform do |index|
-          expect(index.assignee_link_count).to be 3
+          expect(index).to have_assignee_link_count(3)
           expect(index.avatar_counter).to be_visible
           expect(index.avatar_counter).to have_content('+3')
         end
@@ -56,13 +56,13 @@ module QA
         @issue.visit!
 
         Page::Project::Issue::Show.perform do |show|
-          expect(show.avatar_image_count).to be 5
+          expect(show).to have_avatar_image_count(5)
           expect(show.more_assignees_link).to be_visible
           expect(show.more_assignees_link).to have_content('+ 1 more')
 
           show.toggle_more_assignees_link
 
-          expect(show.avatar_image_count).to be 6
+          expect(show).to have_avatar_image_count(6)
           expect(show.more_assignees_link).to have_content('- show less')
         end
       end

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

@@ -28,7 +28,7 @@ module QA
           # You can't start a review immediately, so we have to add a
           # comment (or start a thread) first
           show.start_discussion("I'm starting a new discussion")
-          show.type_reply_to_discussion("Could you please check this?")
+          show.type_reply_to_discussion(1, "Could you please check this?")
           show.start_review
           show.submit_pending_reviews
 

qa/qa/specs/features/ee/browser_ui/3_create/repository/assign_code_owners_spec.rb

@@ -73,6 +73,7 @@ module QA
           show.edit!
           approvers = show.approvers
 
+          expect(approvers.size).to eq(1)
           expect(approvers).to include(approver.name)
           expect(approvers).not_to include(non_approver.name)
         end

qa/spec/page/base_spec.rb

@@ -91,4 +91,20 @@ describe QA::Page::Base do
       end
     end
   end
+
+  describe '#all_elements' do
+    before do
+      allow(subject).to receive(:all)
+    end
+
+    it 'raises an error if count or minimum are not specified' do
+      expect { subject.all_elements(:foo) }.to raise_error ArgumentError
+    end
+
+    it 'does not raise an error if :minimum, :maximum, :count, or :between is specified' do
+      [:minimum, :maximum, :count, :between].each do |param|
+        expect { subject.all_elements(:foo, param => 1) }.not_to raise_error
+      end
+    end
+  end
 end

qa/spec/page/logging_spec.rb

@@ -145,18 +145,18 @@ describe QA::Support::Page::Logging do
     it 'logs the number of elements found' do
       allow(page).to receive(:all).and_return([1, 2])
 
-      expect { subject.all_elements(:element) }
+      expect { subject.all_elements(:element, count: 2) }
         .to output(/finding all :element/).to_stdout_from_any_process
-      expect { subject.all_elements(:element) }
+      expect { subject.all_elements(:element, count: 2) }
         .to output(/found 2 :element/).to_stdout_from_any_process
     end
 
     it 'logs 0 if no elements are found' do
       allow(page).to receive(:all).and_return([])
 
-      expect { subject.all_elements(:element) }
+      expect { subject.all_elements(:element, count: 1) }
         .to output(/finding all :element/).to_stdout_from_any_process
-      expect { subject.all_elements(:element) }
+      expect { subject.all_elements(:element, count: 1) }
         .not_to output(/found 0 :elements/).to_stdout_from_any_process
     end
   end