From 374fa6c7f67503ababe2b40c36bd0a7547100ed5 Mon Sep 17 00:00:00 2001
From: Will Meek <wmeek@gitlab.com>
Date: Thu, 18 May 2023 13:14:16 +0000
Subject: [PATCH] Allow QA E2E specs to use data-testid

This will allow for consistency
with frontend test attributes
---
 .../security_configuration/components/app.vue | 11 ++---
 .../components/auto_dev_ops_alert.vue         |  2 +-
 .../components/feature_card.vue               |  6 +--
 .../_create_protected_branch.html.haml        |  4 +-
 .../ee/_create_protected_branch.html.haml     |  4 +-
 .../project/settings/protected_branches.rb    |  6 +--
 qa/qa/page/element.rb                         | 15 +++---
 .../page/project/secure/configuration_form.rb | 48 +++++--------------
 .../project/settings/protected_branches.rb    |  6 +--
 ...enable_scanning_from_configuration_spec.rb | 18 +++----
 qa/spec/page/element_spec.rb                  |  8 +++-
 11 files changed, 52 insertions(+), 76 deletions(-)

diff --git a/app/assets/javascripts/security_configuration/components/app.vue b/app/assets/javascripts/security_configuration/components/app.vue
index d57b3fda342b..e7d979891950 100644
--- a/app/assets/javascripts/security_configuration/components/app.vue
+++ b/app/assets/javascripts/security_configuration/components/app.vue
@@ -164,7 +164,7 @@ export default {
 
     <gl-tabs
       content-class="gl-pt-0"
-      data-qa-selector="security_configuration_container"
+      data-testid="security-configuration-container"
       sync-active-tab-with-query-params
       lazy
     >
@@ -196,12 +196,9 @@ export default {
               {{ $options.i18n.description }}
             </p>
             <p v-if="canViewCiHistory">
-              <gl-link
-                data-testid="security-view-history-link"
-                data-qa-selector="security_configuration_history_link"
-                :href="gitlabCiHistoryPath"
-                >{{ $options.i18n.configurationHistory }}</gl-link
-              >
+              <gl-link data-testid="security-view-history-link" :href="gitlabCiHistoryPath">{{
+                $options.i18n.configurationHistory
+              }}</gl-link>
             </p>
           </template>
 
diff --git a/app/assets/javascripts/security_configuration/components/auto_dev_ops_alert.vue b/app/assets/javascripts/security_configuration/components/auto_dev_ops_alert.vue
index 315f676e6597..c01df3573c57 100644
--- a/app/assets/javascripts/security_configuration/components/auto_dev_ops_alert.vue
+++ b/app/assets/javascripts/security_configuration/components/auto_dev_ops_alert.vue
@@ -28,7 +28,7 @@ export default {
     variant="info"
     :primary-button-link="autoDevopsPath"
     :primary-button-text="$options.i18n.primaryButtonText"
-    data-qa-selector="autodevops_container"
+    data-testid="autodevops-container"
     @dismiss="dismissMethod"
   >
     <gl-sprintf :message="$options.i18n.body">
diff --git a/app/assets/javascripts/security_configuration/components/feature_card.vue b/app/assets/javascripts/security_configuration/components/feature_card.vue
index d1b705fe2fcd..467b3e66dd44 100644
--- a/app/assets/javascripts/security_configuration/components/feature_card.vue
+++ b/app/assets/javascripts/security_configuration/components/feature_card.vue
@@ -122,7 +122,7 @@ export default {
         v-if="isNotSastIACTemporaryHack"
         :class="statusClasses"
         data-testid="feature-status"
-        :data-qa-selector="`${feature.type}_status`"
+        :data-qa-feature="`${feature.type}_${hasEnabledStatus}_status`"
       >
         <feature-card-badge
           v-if="hasBadge"
@@ -164,7 +164,7 @@ export default {
         :href="feature.configurationPath"
         variant="confirm"
         :category="configurationButton.category"
-        :data-qa-selector="`${feature.type}_enable_button`"
+        :data-testid="`${feature.type}_enable_button`"
         class="gl-mt-5"
       >
         {{ configurationButton.text }}
@@ -176,7 +176,7 @@ export default {
         variant="confirm"
         :category="manageViaMrButtonCategory"
         class="gl-mt-5"
-        :data-qa-selector="`${feature.type}_mr_button`"
+        :data-testid="`${feature.type}_mr_button`"
         @error="onError"
       />
 
diff --git a/app/views/protected_branches/_create_protected_branch.html.haml b/app/views/protected_branches/_create_protected_branch.html.haml
index b4765ab49c26..799f6aa6031a 100644
--- a/app/views/protected_branches/_create_protected_branch.html.haml
+++ b/app/views/protected_branches/_create_protected_branch.html.haml
@@ -3,12 +3,12 @@
     = dropdown_tag(_('Select'),
                     options: { toggle_class: 'js-allowed-to-merge wide',
                     dropdown_class: 'dropdown-menu-selectable capitalize-header', dropdown_qa_selector: 'allowed_to_merge_dropdown_content', dropdown_testid: 'allowed-to-merge-dropdown',
-                    data: { field_name: 'protected_branch[merge_access_levels_attributes][0][access_level]', input_id: 'merge_access_levels_attributes', qa_selector: 'allowed_to_merge_dropdown' }})
+                    data: { field_name: 'protected_branch[merge_access_levels_attributes][0][access_level]', input_id: 'merge_access_levels_attributes', qa_selector: 'select_allowed_to_merge_dropdown' }})
 - content_for :push_access_levels do
   .push_access_levels-container
     = dropdown_tag(_('Select'),
                     options: { toggle_class: "js-allowed-to-push js-multiselect wide",
                     dropdown_class: 'dropdown-menu-selectable capitalize-header', dropdown_qa_selector: 'allowed_to_push_dropdown_content' , dropdown_testid: 'allowed-to-push-dropdown',
-                    data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes', qa_selector: 'allowed_to_push_dropdown' }})
+                    data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes', qa_selector: 'select_allowed_to_push_dropdown' }})
 
 = render 'protected_branches/shared/create_protected_branch', protected_branch_entity: protected_branch_entity
diff --git a/ee/app/views/protected_branches/ee/_create_protected_branch.html.haml b/ee/app/views/protected_branches/ee/_create_protected_branch.html.haml
index 67e676cfdb57..9b264126a368 100644
--- a/ee/app/views/protected_branches/ee/_create_protected_branch.html.haml
+++ b/ee/app/views/protected_branches/ee/_create_protected_branch.html.haml
@@ -3,13 +3,13 @@
     = dropdown_tag(_('Select'),
                    options: { toggle_class: 'js-allowed-to-merge js-multiselect gl-w-full! gl-form-input-lg',
                    dropdown_class: 'dropdown-menu-user dropdown-menu-selectable capitalize-header', dropdown_qa_selector: 'allowed_to_merge_dropdown_content', dropdown_testid: 'allowed-to-merge-dropdown', filter: true,
-                   data: { input_id: 'merge_access_levels_attributes', default_label: 'Select', qa_selector: 'allowed_to_merge_dropdown' } })
+                   data: { input_id: 'merge_access_levels_attributes', default_label: 'Select', qa_selector: 'select_allowed_to_merge_dropdown' } })
 - content_for :push_access_levels do
   .push_access_levels-container
     = dropdown_tag(_('Select'),
                    options: { toggle_class: 'js-allowed-to-push js-multiselect gl-w-full! gl-form-input-lg',
                    dropdown_class: 'dropdown-menu-user dropdown-menu-selectable capitalize-header', dropdown_qa_selector: 'allowed_to_push_dropdown_content', dropdown_testid: 'allowed-to-push-dropdown', filter: true,
-                   data: { input_id: 'push_access_levels_attributes', default_label: 'Select', qa_selector: 'allowed_to_push_dropdown' } })
+                   data: { input_id: 'push_access_levels_attributes', default_label: 'Select', qa_selector: 'select_allowed_to_push_dropdown' } })
   .form-text.text-muted
     = s_('ProtectedBranch|You can add only groups that have this project shared. %{learn_more_link}').html_safe % { learn_more_link: link_to(_('Learn more.'), help_page_path('user/project/members/share_project_with_groups')) }
 
diff --git a/qa/qa/ee/page/project/settings/protected_branches.rb b/qa/qa/ee/page/project/settings/protected_branches.rb
index 76576ca3c3f7..832041580842 100644
--- a/qa/qa/ee/page/project/settings/protected_branches.rb
+++ b/qa/qa/ee/page/project/settings/protected_branches.rb
@@ -17,9 +17,9 @@ def self.prepended(base)
                 end
 
                 view 'ee/app/views/protected_branches/ee/_create_protected_branch.html.haml' do
-                  element :allowed_to_push_dropdown
+                  element :select_allowed_to_push_dropdown
                   element :allowed_to_push_dropdown_content
-                  element :allowed_to_merge_dropdown
+                  element :select_allowed_to_merge_dropdown
                   element :allowed_to_merge_dropdown_content
                 end
               end
@@ -36,7 +36,7 @@ def select_allowed(action, allowed)
               super
 
               # Click the select element again to close the dropdown
-              click_element(:"allowed_to_#{action}_dropdown")
+              click_element(:"select_allowed_to_#{action}_dropdown")
             end
           end
         end
diff --git a/qa/qa/page/element.rb b/qa/qa/page/element.rb
index 6bfdf98587ba..27886934e2ee 100644
--- a/qa/qa/page/element.rb
+++ b/qa/qa/page/element.rb
@@ -13,9 +13,7 @@ def initialize(name, *options)
         @attributes[:pattern] ||= selector
 
         options.each do |option|
-          if option.is_a?(String) || option.is_a?(Regexp)
-            @attributes[:pattern] = option
-          end
+          @attributes[:pattern] = option if option.is_a?(String) || option.is_a?(Regexp)
         end
       end
 
@@ -28,7 +26,7 @@ def required?
       end
 
       def selector_css
-        %Q([data-qa-selector="#{@name}"]#{additional_selectors},.#{selector})
+        %(#{qa_selector},.#{selector})
       end
 
       def expression
@@ -40,14 +38,19 @@ def expression
       end
 
       def matches?(line)
-        !!(line =~ /["']#{name}['"]|#{expression}/)
+        !!(line =~ /["']#{name}['"]|["']#{name.to_s.tr('_', '-')}['"]|#{expression}/)
       end
 
       private
 
+      def qa_selector
+        %([data-testid="#{@name}"]#{additional_selectors},[data-testid="#{@name.to_s.tr('_',
+          '-')}"]#{additional_selectors},[data-qa-selector="#{@name}"]#{additional_selectors})
+      end
+
       def additional_selectors
         @attributes.dup.delete_if { |attr| attr == :pattern || attr == :required }.map do |key, value|
-          %Q([data-qa-#{key.to_s.tr('_', '-')}="#{value}"])
+          %([data-qa-#{key.to_s.tr('_', '-')}="#{value}"])
         end.join
       end
     end
diff --git a/qa/qa/page/project/secure/configuration_form.rb b/qa/qa/page/project/secure/configuration_form.rb
index 493ec08d0239..70eff31bfa98 100644
--- a/qa/qa/page/project/secure/configuration_form.rb
+++ b/qa/qa/page/project/secure/configuration_form.rb
@@ -9,15 +9,13 @@ class ConfigurationForm < QA::Page::Base
 
           view 'app/assets/javascripts/security_configuration/components/app.vue' do
             element :security_configuration_container
-            element :security_configuration_history_link
+            element :security_view_history_link
           end
 
           view 'app/assets/javascripts/security_configuration/components/feature_card.vue' do
-            element :dependency_scanning_status, "`${feature.type}_status`" # rubocop:disable QA/ElementWithPattern
-            element :sast_status, "`${feature.type}_status`" # rubocop:disable QA/ElementWithPattern
+            element :feature_status
             element :sast_enable_button, "`${feature.type}_enable_button`" # rubocop:disable QA/ElementWithPattern
             element :dependency_scanning_mr_button, "`${feature.type}_mr_button`" # rubocop:disable QA/ElementWithPattern
-            element :license_scanning_status, "`${feature.type}_status`" # rubocop:disable QA/ElementWithPattern
           end
 
           view 'app/assets/javascripts/security_configuration/components/auto_dev_ops_alert.vue' do
@@ -25,15 +23,15 @@ class ConfigurationForm < QA::Page::Base
           end
 
           def has_security_configuration_history_link?
-            has_element?(:security_configuration_history_link)
+            has_element?(:security_view_history_link)
           end
 
           def has_no_security_configuration_history_link?
-            has_no_element?(:security_configuration_history_link)
+            has_no_element?(:security_view_history_link)
           end
 
           def click_security_configuration_history_link
-            click_element(:security_configuration_history_link)
+            click_element(:security_view_history_link)
           end
 
           def click_sast_enable_button
@@ -44,40 +42,20 @@ def click_dependency_scanning_mr_button
             click_element(:dependency_scanning_mr_button)
           end
 
-          def has_sast_status?(status_text)
-            within_element(:sast_status) do
-              has_text?(status_text)
-            end
-          end
-
-          def has_no_sast_status?(status_text)
-            within_element(:sast_status) do
-              has_no_text?(status_text)
-            end
-          end
-
-          def has_dependency_scanning_status?(status_text)
-            within_element(:dependency_scanning_status) do
-              has_text?(status_text)
-            end
+          def has_true_sast_status?
+            has_element?(:feature_status, feature: 'sast_true_status')
           end
 
-          def has_no_dependency_scanning_status?(status_text)
-            within_element(:dependency_scanning_status) do
-              has_no_text?(status_text)
-            end
+          def has_false_sast_status?
+            has_element?(:feature_status, feature: 'sast_false_status')
           end
 
-          def has_license_compliance_status?(status_text)
-            within_element(:license_scanning_status) do
-              has_text?(status_text)
-            end
+          def has_true_dependency_scanning_status?
+            has_element?(:feature_status, feature: 'dependency_scanning_true_status')
           end
 
-          def has_no_license_compliance_status?(status_text)
-            within_element(:license_scanning_status) do
-              has_no_text?(status_text)
-            end
+          def has_false_dependency_scanning_status?
+            has_element?(:feature_status, feature: 'dependency_scanning_false_status')
           end
 
           def has_auto_devops_container?
diff --git a/qa/qa/page/project/settings/protected_branches.rb b/qa/qa/page/project/settings/protected_branches.rb
index 3eddd0fd33a2..e6b13ed77a01 100644
--- a/qa/qa/page/project/settings/protected_branches.rb
+++ b/qa/qa/page/project/settings/protected_branches.rb
@@ -11,9 +11,9 @@ class ProtectedBranches < Page::Base
           end
 
           view 'app/views/protected_branches/_create_protected_branch.html.haml' do
-            element :allowed_to_push_dropdown
+            element :select_allowed_to_push_dropdown
             element :allowed_to_push_dropdown_content
-            element :allowed_to_merge_dropdown
+            element :select_allowed_to_merge_dropdown
             element :allowed_to_merge_dropdown_content
           end
 
@@ -45,7 +45,7 @@ def protect_branch
           private
 
           def select_allowed(action, allowed)
-            click_element :"allowed_to_#{action}_dropdown"
+            click_element :"select_allowed_to_#{action}_dropdown"
 
             allowed[:roles] = Resource::ProtectedBranch::Roles::NO_ONE unless allowed.key?(:roles)
 
diff --git a/qa/qa/specs/features/ee/browser_ui/13_secure/enable_scanning_from_configuration_spec.rb b/qa/qa/specs/features/ee/browser_ui/13_secure/enable_scanning_from_configuration_spec.rb
index c010425ae747..b900ef829c42 100644
--- a/qa/qa/specs/features/ee/browser_ui/13_secure/enable_scanning_from_configuration_spec.rb
+++ b/qa/qa/specs/features/ee/browser_ui/13_secure/enable_scanning_from_configuration_spec.rb
@@ -53,7 +53,7 @@ module QA
           Page::Project::Menu.perform(&:go_to_security_configuration)
 
           Page::Project::Secure::ConfigurationForm.perform do |config_form|
-            expect(config_form).to have_dependency_scanning_status('Not enabled')
+            expect(config_form).to have_false_dependency_scanning_status
             expect(config_form).to have_auto_devops_container
             expect(config_form).to have_auto_devops_container_description
             expect(config_form).to have_no_security_configuration_history_link
@@ -66,9 +66,7 @@ module QA
             new_merge_request.create_merge_request
           end
 
-          Page::MergeRequest::Show.perform do |merge_request|
-            merge_request.merge_immediately!
-          end
+          Page::MergeRequest::Show.perform(&:merge_immediately!)
 
           Flow::Pipeline.visit_latest_pipeline
 
@@ -79,8 +77,7 @@ module QA
           Page::Project::Menu.perform(&:go_to_security_configuration)
 
           Page::Project::Secure::ConfigurationForm.perform do |config_form|
-            expect(config_form).to have_dependency_scanning_status('Enabled')
-            expect(config_form).to have_no_dependency_scanning_status('Not enabled')
+            expect(config_form).to have_true_dependency_scanning_status
             expect(config_form).to have_security_configuration_history_link
             expect(config_form).to have_no_auto_devops_container
 
@@ -113,7 +110,7 @@ def sast_config_expects(current_page, sast_string_fields, sast_int_fields)
           Page::Project::Menu.perform(&:go_to_security_configuration)
 
           Page::Project::Secure::ConfigurationForm.perform do |config_form|
-            expect(config_form).to have_sast_status('Not enabled')
+            expect(config_form).to have_false_sast_status
             expect(config_form).to have_auto_devops_container
             expect(config_form).to have_auto_devops_container_description
             expect(config_form).to have_no_security_configuration_history_link
@@ -145,9 +142,7 @@ def sast_config_expects(current_page, sast_string_fields, sast_int_fields)
             new_merge_request.create_merge_request
           end
 
-          Page::MergeRequest::Show.perform do |merge_request|
-            merge_request.merge_immediately!
-          end
+          Page::MergeRequest::Show.perform(&:merge_immediately!)
 
           Flow::Pipeline.visit_latest_pipeline
 
@@ -158,8 +153,7 @@ def sast_config_expects(current_page, sast_string_fields, sast_int_fields)
           Page::Project::Menu.perform(&:go_to_security_configuration)
 
           Page::Project::Secure::ConfigurationForm.perform do |config_form|
-            expect(config_form).to have_sast_status('Enabled')
-            expect(config_form).to have_no_sast_status('Not enabled')
+            expect(config_form).to have_true_sast_status
             expect(config_form).to have_security_configuration_history_link
             expect(config_form).to have_no_auto_devops_container
 
diff --git a/qa/spec/page/element_spec.rb b/qa/spec/page/element_spec.rb
index fbf58b5e18a4..da1fd224564c 100644
--- a/qa/spec/page/element_spec.rb
+++ b/qa/spec/page/element_spec.rb
@@ -73,7 +73,7 @@
       subject { described_class.new(:something, /link_to 'something'/) }
 
       it 'has an attribute[pattern] of the pattern' do
-        expect(subject.attributes[:pattern]).to eq /link_to 'something'/
+        expect(subject.attributes[:pattern]).to eq(/link_to 'something'/)
       end
 
       it 'is not required by default' do
@@ -98,7 +98,7 @@
       subject { described_class.new(:something, /link_to 'something_else_entirely'/, required: true) }
 
       it 'has an attribute[pattern] of the passed pattern' do
-        expect(subject.attributes[:pattern]).to eq /link_to 'something_else_entirely'/
+        expect(subject.attributes[:pattern]).to eq(/link_to 'something_else_entirely'/)
       end
 
       it 'is required' do
@@ -118,6 +118,10 @@
       expect(subject.selector_css).to include(%q([data-qa-selector="my_element"]))
     end
 
+    it 'properly translates to a data-testid' do
+      expect(subject.selector_css).to include(%q([data-testid="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) }
-- 
GitLab