Merge branch 'qa-extensible-data-qa-selectors' into 'master'

Add extensibility to the data-qa selection mechanism

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

See merge request gitlab-org/gitlab!17790

app/views/projects/issues/_issue.html.haml

 -# DANGER: Any changes to this file need to be reflected in issuables_list/components/issuable.vue!
-%li{ id: dom_id(issue), class: issue_css_classes(issue), url: issue_path(issue), data: { labels: issue.label_ids, id: issue.id } }
+%li{ id: dom_id(issue), class: issue_css_classes(issue), url: issue_path(issue), data: { labels: issue.label_ids, id: issue.id, qa_selector: 'issue', qa_issue_title: issue.title } }
   .issue-box
     - if @can_bulk_update
       .issue-check.hidden

doc/development/testing_guide/end_to_end/page_objects.md

@@ -167,6 +167,65 @@ There are two supported methods of defining elements within a view.
 Any existing `.qa-selector` class should be considered deprecated
 and we should prefer the `data-qa-selector` method of definition.
 
+### Dynamic element selection
+
+> Introduced in GitLab 12.5
+
+A common occurrence in automated testing is selecting a single "one-of-many" element.  
+In a list of several items, how do you differentiate what you are selecting on?  
+The most common workaround for this is via text matching.  Instead, a better practice is
+by matching on that specific element by a unique identifier, rather than by text.
+
+We got around this by adding the `data-qa-*` extensible selection mechanism.
+
+#### Examples
+
+**Example 1**
+
+Given the following Rails view (using GitLab Issues as an example):
+
+```haml
+%ul.issues-list
+ - @issues.each do |issue|
+   %li.issue{data: { qa_selector: 'issue', qa_issue_title: issue.title } }= link_to issue
+```
+
+We can select on that specific issue by matching on the Rails model.
+
+```ruby
+class Page::Project::Issues::Index < Page::Base
+  def has_issue?(issue)
+    has_element? :issue, issue_title: issue
+  end
+end
+```
+
+In our test, we can validate that this particular issue exists.
+
+```ruby
+describe 'Issue' do
+  it 'has an issue titled "hello"' do
+    Page::Project::Issues::Index.perform do |index|
+      expect(index).to have_issue('hello')
+    end
+  end
+end
+```
+
+**Example 2**
+
+*By an index...*
+
+```haml
+%ol
+  - @some_model.each_with_index do |model, idx|
+    %li.model{ data: { qa_selector: 'model', qa_index: idx } }
+```
+
+```ruby
+expect(the_page).to have_element(:model, index: 1) #=> select on the first model that appears in the list
+```
+
 ### Exceptions
 
 In some cases it might not be possible or worthwhile to add a selector.

qa/qa/page/base.rb

@@ -111,12 +111,18 @@ module QA
         element.select value
       end
 
-      def has_element?(name, text: nil, wait: Capybara.default_max_wait_time)
-        has_css?(element_selector_css(name), wait: wait, text: text)
+      def has_element?(name, **kwargs)
+        wait = kwargs[:wait] ? kwargs[:wait] && kwargs.delete(:wait) : Capybara.default_max_wait_time
+        text = kwargs[:text] ? kwargs[:text] && kwargs.delete(:text) : nil
+
+        has_css?(element_selector_css(name, kwargs), text: text, wait: wait)
       end
 
-      def has_no_element?(name, text: nil, wait: Capybara.default_max_wait_time)
-        has_no_css?(element_selector_css(name), wait: wait, text: text)
+      def has_no_element?(name, **kwargs)
+        wait = kwargs[:wait] ? kwargs[:wait] && kwargs.delete(:wait) : Capybara.default_max_wait_time
+        text = kwargs[:text] ? kwargs[:text] && kwargs.delete(:text) : nil
+
+        has_no_css?(element_selector_css(name, kwargs), wait: wait, text: text)
       end
 
       def has_text?(text)
@@ -199,8 +205,8 @@ module QA
         scroll_to(element_selector_css(name), *args)
       end
 
-      def element_selector_css(name)
-        Page::Element.new(name).selector_css
+      def element_selector_css(name, *attributes)
+        Page::Element.new(name, *attributes).selector_css
       end
 
       def click_link_with_text(text)

qa/qa/page/element.rb

@@ -28,7 +28,7 @@ module QA
       end
 
       def selector_css
-        %Q([data-qa-selector="#{@name}"],.#{selector})
+        %Q([data-qa-selector="#{@name}"]#{additional_selectors},.#{selector})
       end
 
       def expression
@@ -42,6 +42,14 @@ module QA
       def matches?(line)
         !!(line =~ /["']#{name}['"]|#{expression}/)
       end
+
+      private
+
+      def additional_selectors
+        @attributes.dup.delete_if { |attr| attr == :pattern || attr == :required }.map do |key, value|
+          %Q([data-qa-#{key.to_s.tr('_', '-')}="#{value}"])
+        end.join
+      end
     end
   end
 end

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

@@ -36,6 +36,10 @@ module QA
           def click_closed_issues_link
             click_element :closed_issues_link
           end
+
+          def has_issue?(issue)
+            has_element? :issue, issue_title: issue.to_s
+          end
         end
       end
     end

qa/qa/resource/issue.rb

@@ -38,6 +38,10 @@ module QA
         end
       end
 
+      def to_s
+        @title
+      end
+
       def api_get_path
         "/projects/#{project.id}/issues/#{id}"
       end

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

@@ -10,13 +10,15 @@ module QA
       end
 
       it 'user creates an issue' do
-        Resource::Issue.fabricate_via_browser_ui! do |issue|
+        issue = Resource::Issue.fabricate_via_browser_ui! do |issue|
           issue.title = issue_title
         end
 
         Page::Project::Menu.perform(&:click_issues)
 
-        expect(page).to have_content(issue_title)
+        Page::Project::Issue::Index.perform do |index|
+          expect(index).to have_issue(issue)
+        end
       end
 
       context 'when using attachments in comments', :object_storage do

qa/spec/page/element_spec.rb

@@ -117,5 +117,23 @@ describe QA::Page::Element do
     it 'properly translates to a data-qa-selector' do
       expect(subject.selector_css).to include(%q([data-qa-selector="my_element"]))
     end
+
+    context 'additional selectors' do
+      let(:element) { described_class.new(:my_element, index: 3, another_match: 'something') }
+      let(:required_element) { described_class.new(:my_element, required: true, index: 3) }
+
+      it 'matches on additional data-qa properties' do
+        expect(element.selector_css).to include(%q([data-qa-selector="my_element"][data-qa-index="3"]))
+      end
+
+      it 'doesnt conflict with element requirement' do
+        expect(required_element).to be_required
+        expect(required_element.selector_css).not_to include(%q(data-qa-required))
+      end
+
+      it 'translates snake_case to kebab-case' do
+        expect(element.selector_css).to include(%q(data-qa-another-match))
+      end
+    end
   end
 end