From 390e0d8906c066996e8bbb14aa5824643f9798f8 Mon Sep 17 00:00:00 2001 From: ddavison Date: Wed, 29 May 2019 16:47:53 -0700 Subject: Introduce data-qa-selector to supplant .qa-class In order to break away from using CSS classes as our primary method of element identification, we need to provide the ability to search for data attributes. Make Test::Sanity::Selectors now work Utilize regex to match on literal strings of the element name Suggest the data-qa-selector pattern vs the qa- Add data-qa-selector to login page to start We need a page that is heavily used in order to be confident that this functionality works. Let's start with the Login page Use appropriate HAML data tag practices --- app/views/devise/sessions/_new_base.html.haml | 6 +++--- app/views/layouts/devise.html.haml | 2 +- app/views/layouts/header/_default.html.haml | 2 +- qa/qa/page/element.rb | 4 ++-- qa/spec/page/element_spec.rb | 13 ++++++++++++- rubocop/cop/qa/element_with_pattern.rb | 2 +- 6 files changed, 20 insertions(+), 9 deletions(-) diff --git a/app/views/devise/sessions/_new_base.html.haml b/app/views/devise/sessions/_new_base.html.haml index 7dacd0b1d72..2f10f08c839 100644 --- a/app/views/devise/sessions/_new_base.html.haml +++ b/app/views/devise/sessions/_new_base.html.haml @@ -1,10 +1,10 @@ = form_for(resource, as: resource_name, url: session_path(resource_name), html: { class: 'new_user gl-show-field-errors', 'aria-live' => 'assertive'}) do |f| .form-group = f.label "Username or email", for: "user_login", class: 'label-bold' - = f.text_field :login, class: "form-control top qa-login-field", autofocus: "autofocus", autocapitalize: "off", autocorrect: "off", required: true, title: "This field is required." + = f.text_field :login, class: "form-control top", autofocus: "autofocus", autocapitalize: "off", autocorrect: "off", required: true, title: "This field is required.", data: { qa_selector: 'login_field' } .form-group = f.label :password, class: 'label-bold' - = f.password_field :password, class: "form-control bottom qa-password-field", required: true, title: "This field is required." + = f.password_field :password, class: "form-control bottom", required: true, title: "This field is required.", data: { qa_selector: 'password_field' } - if devise_mapping.rememberable? .remember-me %label{ for: "user_remember_me" } @@ -17,4 +17,4 @@ = recaptcha_tags .submit-container.move-submit-down - = f.submit "Sign in", class: "btn btn-success qa-sign-in-button" + = f.submit "Sign in", class: "btn btn-success", data: { qa_selector: 'sign_in_button' } diff --git a/app/views/layouts/devise.html.haml b/app/views/layouts/devise.html.haml index ff3410f6268..e9a4a068599 100644 --- a/app/views/layouts/devise.html.haml +++ b/app/views/layouts/devise.html.haml @@ -1,7 +1,7 @@ !!! 5 %html.devise-layout-html{ class: system_message_class } = render "layouts/head" - %body.ui-indigo.login-page.application.navless.qa-login-page{ data: { page: body_data_page } } + %body.ui-indigo.login-page.application.navless{ data: { page: body_data_page, qa_selector: 'login_page' } } = header_message .page-wrap = render "layouts/header/empty" diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index f9ee6f42e23..36ae24fc4bf 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -5,7 +5,7 @@ - else - search_path_url = search_path -%header.navbar.navbar-gitlab.qa-navbar.navbar-expand-sm.js-navbar +%header.navbar.navbar-gitlab.navbar-expand-sm.js-navbar{ data: { qa_selector: 'navbar' } } %a.sr-only.gl-accessibility{ href: "#content-body", tabindex: "1" } Skip to content .container-fluid .header-content diff --git a/qa/qa/page/element.rb b/qa/qa/page/element.rb index 7a01320901d..f9aa9cc6377 100644 --- a/qa/qa/page/element.rb +++ b/qa/qa/page/element.rb @@ -28,7 +28,7 @@ module QA end def selector_css - ".#{selector}" + "[data-qa-selector='#{@name}'],.#{selector}" end def expression @@ -40,7 +40,7 @@ module QA end def matches?(line) - !!(line =~ expression) + !!(line =~ /["']#{name}['"]|#{expression}/) end end end diff --git a/qa/spec/page/element_spec.rb b/qa/spec/page/element_spec.rb index f746fe06e40..87c9fbf74a3 100644 --- a/qa/spec/page/element_spec.rb +++ b/qa/spec/page/element_spec.rb @@ -11,7 +11,7 @@ describe QA::Page::Element do describe '#selector_css' do it 'transforms element name into QA-specific clickable css selector' do expect(described_class.new(:sign_in_button).selector_css) - .to eq '.qa-sign-in-button' + .to include('.qa-sign-in-button') end end @@ -49,6 +49,10 @@ describe QA::Page::Element do it 'does not match if QA selector is not there' do expect(subject.matches?('some_name selector')).to be false end + + it 'matches when element name is specified' do + expect(subject.matches?('data:{qa:{selector:"some_name"}}')).to be true + end end describe 'attributes' do @@ -106,4 +110,11 @@ describe QA::Page::Element do end end end + + describe 'data-qa selectors' do + subject { described_class.new(:my_element) } + it 'properly translates to a data-qa-selector' do + expect(subject.selector_css).to include("[data-qa-selector='my_element']") + end + end end diff --git a/rubocop/cop/qa/element_with_pattern.rb b/rubocop/cop/qa/element_with_pattern.rb index d14eeaaeaf3..f7debf13151 100644 --- a/rubocop/cop/qa/element_with_pattern.rb +++ b/rubocop/cop/qa/element_with_pattern.rb @@ -30,7 +30,7 @@ module RuboCop return if args.first.nil? args.first.each_node(:str) do |arg| - add_offense(arg, message: MESSAGE % "qa-#{element_name.value.to_s.tr('_', '-')}") + add_offense(arg, message: MESSAGE % "data-qa-selector=\"#{element_name.value}\"") end end -- cgit v1.2.1 From 98fc3f23db648a16096994ba3b70d461b5b1ccf0 Mon Sep 17 00:00:00 2001 From: ddavison Date: Thu, 6 Jun 2019 22:34:42 -0700 Subject: Fix Main::Menu locators Fix the locators to not use a regex Add requirement --- app/views/layouts/header/_default.html.haml | 2 +- qa/qa/page/main/menu.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 36ae24fc4bf..fc2dea25c77 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -64,7 +64,7 @@ .dropdown-menu.dropdown-menu-right = render 'layouts/header/help_dropdown' - if header_link?(:user_dropdown) - %li.nav-item.header-user.dropdown{ data: { track_label: "profile_dropdown", track_event: "click_dropdown" } } + %li.nav-item.header-user.dropdown{ data: { track_label: "profile_dropdown", track_event: "click_dropdown", qa_selector: 'user_menu' } } = link_to current_user, class: user_dropdown_class, data: { toggle: "dropdown" } do = image_tag avatar_icon_for_user(current_user, 23), width: 23, height: 23, class: "header-user-avatar qa-user-avatar" = sprite_icon('angle-down', css_class: 'caret-down') diff --git a/qa/qa/page/main/menu.rb b/qa/qa/page/main/menu.rb index 9c99e43d4c0..c98d85d7911 100644 --- a/qa/qa/page/main/menu.rb +++ b/qa/qa/page/main/menu.rb @@ -12,7 +12,7 @@ module QA view 'app/views/layouts/header/_default.html.haml' do element :navbar, required: true element :user_avatar, required: true - element :user_menu, '.dropdown-menu' # rubocop:disable QA/ElementWithPattern + element :user_menu, required: true end view 'app/views/layouts/nav/_dashboard.html.haml' do @@ -82,7 +82,7 @@ module QA private def within_top_menu - page.within('.qa-navbar') do + within_element(:navbar) do yield end end @@ -91,7 +91,7 @@ module QA within_top_menu do click_element :user_avatar - page.within('.dropdown-menu') do + within_element(:user_menu) do yield end end -- cgit v1.2.1 From 015b61b1dc73e2c8d6e6cf04b6b2c7fb896a452f Mon Sep 17 00:00:00 2001 From: ddavison Date: Tue, 9 Jul 2019 14:44:16 -0700 Subject: Change element_with_pattern cop to not use quotes Using quotes within string validations can be messy. Let's use a typical CSS selector for an unquoted attribute Update the cop spec to validate appropriate new message --- rubocop/cop/qa/element_with_pattern.rb | 2 +- spec/rubocop/cop/qa/element_with_pattern_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rubocop/cop/qa/element_with_pattern.rb b/rubocop/cop/qa/element_with_pattern.rb index f7debf13151..d48f4725488 100644 --- a/rubocop/cop/qa/element_with_pattern.rb +++ b/rubocop/cop/qa/element_with_pattern.rb @@ -30,7 +30,7 @@ module RuboCop return if args.first.nil? args.first.each_node(:str) do |arg| - add_offense(arg, message: MESSAGE % "data-qa-selector=\"#{element_name.value}\"") + add_offense(arg, message: MESSAGE % "data-qa-selector=#{element_name.value}") end end diff --git a/spec/rubocop/cop/qa/element_with_pattern_spec.rb b/spec/rubocop/cop/qa/element_with_pattern_spec.rb index ef20d9a1f26..fee390caa9f 100644 --- a/spec/rubocop/cop/qa/element_with_pattern_spec.rb +++ b/spec/rubocop/cop/qa/element_with_pattern_spec.rb @@ -23,9 +23,9 @@ describe RuboCop::Cop::QA::ElementWithPattern do expect_offense(<<-RUBY) view 'app/views/shared/groups/_search_form.html.haml' do element :groups_filter, 'search_field_tag :filter' - ^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use a pattern for element, create a corresponding `qa-groups-filter` instead. + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use a pattern for element, create a corresponding `data-qa-selector=groups_filter` instead. element :groups_filter_placeholder, /Search by name/ - ^^^^^^^^^^^^^^ Don't use a pattern for element, create a corresponding `qa-groups-filter-placeholder` instead. + ^^^^^^^^^^^^^^ Don't use a pattern for element, create a corresponding `data-qa-selector=groups_filter_placeholder` instead. end RUBY end -- cgit v1.2.1 From 3281e6db23d8f1ea6757af6eb71e87ed7a93c130 Mon Sep 17 00:00:00 2001 From: ddavison Date: Wed, 10 Jul 2019 16:20:31 -0700 Subject: Treat element#selector_css string appropriately Proper escaping should be used for page/base.rb#scroll_to as it is a single quoted JS string --- qa/qa/page/element.rb | 2 +- qa/spec/page/element_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qa/qa/page/element.rb b/qa/qa/page/element.rb index f9aa9cc6377..9e6fd2fdd4f 100644 --- a/qa/qa/page/element.rb +++ b/qa/qa/page/element.rb @@ -28,7 +28,7 @@ module QA end def selector_css - "[data-qa-selector='#{@name}'],.#{selector}" + %Q([data-qa-selector="#{@name}"],.#{selector}) end def expression diff --git a/qa/spec/page/element_spec.rb b/qa/spec/page/element_spec.rb index 87c9fbf74a3..20d4a00c020 100644 --- a/qa/spec/page/element_spec.rb +++ b/qa/spec/page/element_spec.rb @@ -114,7 +114,7 @@ describe QA::Page::Element do describe 'data-qa selectors' do subject { described_class.new(:my_element) } it 'properly translates to a data-qa-selector' do - expect(subject.selector_css).to include("[data-qa-selector='my_element']") + expect(subject.selector_css).to include(%q([data-qa-selector="my_element"])) end end end -- cgit v1.2.1