diff --git a/qa/qa.rb b/qa/qa.rb index f580691f952..944dcc31917 100644 --- a/qa/qa.rb +++ b/qa/qa.rb @@ -130,6 +130,7 @@ module QA autoload :View, 'qa/page/view' autoload :Element, 'qa/page/element' autoload :Validator, 'qa/page/validator' + autoload :Validatable, 'qa/page/validatable' module Main autoload :Login, 'qa/page/main/login' diff --git a/qa/qa/ce/strategy.rb b/qa/qa/ce/strategy.rb index d7748a976f0..7e2d02424fe 100644 --- a/qa/qa/ce/strategy.rb +++ b/qa/qa/ce/strategy.rb @@ -13,7 +13,6 @@ module QA # The login page could take some time to load the first time it is visited. # We visit the login page and wait for it to properly load only once before the tests. QA::Runtime::Browser.visit(:gitlab, QA::Page::Main::Login) - QA::Page::Main::Login.perform(&:assert_page_loaded) end end end diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index c395e5f6011..389f4e0032e 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -8,6 +8,7 @@ module QA prepend Support::Page::Logging if Runtime::Env.debug? include Capybara::DSL include Scenario::Actable + extend Validatable extend SingleForwardable ElementNotFound = Class.new(RuntimeError) @@ -93,8 +94,10 @@ module QA find_element(name).set(false) end - def click_element(name) + # replace with (..., page = self.class) + def click_element(name, page = nil) find_element(name).click + page.validate_elements_present! if page end def fill_element(name, content) diff --git a/qa/qa/page/element.rb b/qa/qa/page/element.rb index d92e71467fe..7a01320901d 100644 --- a/qa/qa/page/element.rb +++ b/qa/qa/page/element.rb @@ -1,28 +1,41 @@ # frozen_string_literal: true +require 'active_support/core_ext/array/extract_options' + module QA module Page class Element - attr_reader :name + attr_reader :name, :attributes - def initialize(name, pattern = nil) + def initialize(name, *options) @name = name - @pattern = pattern || selector + @attributes = options.extract_options! + @attributes[:pattern] ||= selector + + options.each do |option| + if option.is_a?(String) || option.is_a?(Regexp) + @attributes[:pattern] = option + end + end end def selector "qa-#{@name.to_s.tr('_', '-')}" end + def required? + !!@attributes[:required] + end + def selector_css ".#{selector}" end def expression - if @pattern.is_a?(String) - @_regexp ||= Regexp.new(Regexp.escape(@pattern)) + if @attributes[:pattern].is_a?(String) + @_regexp ||= Regexp.new(Regexp.escape(@attributes[:pattern])) else - @pattern + @attributes[:pattern] end end diff --git a/qa/qa/page/main/login.rb b/qa/qa/page/main/login.rb index 99b3d1b83d3..8970eeb6678 100644 --- a/qa/qa/page/main/login.rb +++ b/qa/qa/page/main/login.rb @@ -39,19 +39,7 @@ module QA end view 'app/views/layouts/devise.html.haml' do - element :login_page - end - - def assert_page_loaded - unless page_loaded? - raise QA::Runtime::Browser::NotRespondingError, "Login page did not load at #{QA::Page::Main::Login.perform(&:current_url)}" - end - end - - def page_loaded? - wait(max: 60) do - has_element?(:login_page) - end + element :login_page, required: true end def sign_in_using_credentials(user = nil) @@ -159,7 +147,7 @@ module QA fill_element :login_field, user.username fill_element :password_field, user.password - click_element :sign_in_button + click_element :sign_in_button, Page::Main::Menu end def set_initial_password_if_present diff --git a/qa/qa/page/main/menu.rb b/qa/qa/page/main/menu.rb index e98d531c86e..5eb24d2d2ba 100644 --- a/qa/qa/page/main/menu.rb +++ b/qa/qa/page/main/menu.rb @@ -10,15 +10,15 @@ module QA end view 'app/views/layouts/header/_default.html.haml' do - element :navbar - element :user_avatar + element :navbar, required: true + element :user_avatar, required: true element :user_menu, '.dropdown-menu' # rubocop:disable QA/ElementWithPattern end view 'app/views/layouts/nav/_dashboard.html.haml' do element :admin_area_link - element :projects_dropdown - element :groups_dropdown + element :projects_dropdown, required: true + element :groups_dropdown, required: true element :snippets_link end diff --git a/qa/qa/page/validatable.rb b/qa/qa/page/validatable.rb new file mode 100644 index 00000000000..8467d261285 --- /dev/null +++ b/qa/qa/page/validatable.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module QA + module Page + module Validatable + PageValidationError = Class.new(StandardError) + + def validate_elements_present! + base_page = self.new + + elements.each do |element| + next unless element.required? + + # TODO: this wait needs to be replaced by the wait class + unless base_page.has_element?(element.name, wait: 60) + raise Validatable::PageValidationError, "#{element.name} did not appear on #{self.name} as expected" + end + end + end + end + end +end diff --git a/qa/qa/page/view.rb b/qa/qa/page/view.rb index 96f3917a8ab..613059b2d32 100644 --- a/qa/qa/page/view.rb +++ b/qa/qa/page/view.rb @@ -50,8 +50,8 @@ module QA @elements = [] end - def element(name, pattern = nil) - @elements.push(Page::Element.new(name, pattern)) + def element(name, *args) + @elements.push(Page::Element.new(name, *args)) end end end diff --git a/qa/qa/runtime/browser.rb b/qa/qa/runtime/browser.rb index 174a52bd376..3bf4b3bbbfb 100644 --- a/qa/qa/runtime/browser.rb +++ b/qa/qa/runtime/browser.rb @@ -33,6 +33,7 @@ module QA def self.visit(address, page = nil, &block) new.visit(address, page, &block) + page.validate_elements_present! end def self.configure! diff --git a/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb index b1d641b507f..67610b62ed7 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb @@ -4,16 +4,14 @@ module QA context 'Manage', :orchestrated, :mattermost do describe 'Mattermost login' do it 'user logs into Mattermost using GitLab OAuth' do - Runtime::Browser.visit(:gitlab, Page::Main::Login) do - Page::Main::Login.act { sign_in_using_credentials } + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.perform(&:sign_in_using_credentials) - Runtime::Browser.visit(:mattermost, Page::Mattermost::Login) do - Page::Mattermost::Login.act { sign_in_using_oauth } + Runtime::Browser.visit(:mattermost, Page::Mattermost::Login) + Page::Mattermost::Login.perform(&:sign_in_using_oauth) - Page::Mattermost::Main.perform do |page| - expect(page).to have_content(/(Welcome to: Mattermost|Logout GitLab Mattermost)/) - end - end + Page::Mattermost::Main.perform do |page| + expect(page).to have_content(/(Welcome to: Mattermost|Logout GitLab Mattermost)/) end end end diff --git a/qa/qa/support/page/logging.rb b/qa/qa/support/page/logging.rb index ff505fdbddd..3fe567d7757 100644 --- a/qa/qa/support/page/logging.rb +++ b/qa/qa/support/page/logging.rb @@ -56,8 +56,11 @@ module QA elements end - def click_element(name) - log("clicking :#{name}") + def click_element(name, page = nil) + msg = ["clicking :#{name}"] + msg << ", expecting to be at #{page.class}" if page + + log(msg.compact.join(' ')) super end diff --git a/qa/spec/page/element_spec.rb b/qa/spec/page/element_spec.rb index d5d6dff69da..f746fe06e40 100644 --- a/qa/spec/page/element_spec.rb +++ b/qa/spec/page/element_spec.rb @@ -50,4 +50,60 @@ describe QA::Page::Element do expect(subject.matches?('some_name selector')).to be false end end + + describe 'attributes' do + context 'element with no args' do + subject { described_class.new(:something) } + + it 'defaults pattern to #selector' do + expect(subject.attributes[:pattern]).to eq 'qa-something' + expect(subject.attributes[:pattern]).to eq subject.selector + end + + it 'is not required by default' do + expect(subject.required?).to be false + end + end + + context 'element with a pattern' do + 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'/ + end + + it 'is not required by default' do + expect(subject.required?).to be false + end + end + + context 'element with requirement; no pattern' do + subject { described_class.new(:something, required: true) } + + it 'has an attribute[pattern] of the selector' do + expect(subject.attributes[:pattern]).to eq 'qa-something' + expect(subject.attributes[:pattern]).to eq subject.selector + end + + it 'is required' do + expect(subject.required?).to be true + end + end + + context 'element with requirement and pattern' do + 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'/ + end + + it 'is required' do + expect(subject.required?).to be true + end + + it 'has a selector of the name' do + expect(subject.selector).to eq 'qa-something' + end + end + end end diff --git a/rubocop/cop/qa/element_with_pattern.rb b/rubocop/cop/qa/element_with_pattern.rb index 9d80946f1ba..d14eeaaeaf3 100644 --- a/rubocop/cop/qa/element_with_pattern.rb +++ b/rubocop/cop/qa/element_with_pattern.rb @@ -1,18 +1,21 @@ +# frozen_string_literal: true + require_relative '../../qa_helpers' module RuboCop module Cop module QA - # This cop checks for the usage of factories in migration specs + # This cop checks for the usage of patterns in QA elements # # @example # # # bad - # let(:user) { create(:user) } + # element :some_element, "link_to 'something'" + # element :some_element, /link_to 'something'/ # # # good - # let(:users) { table(:users) } - # let(:user) { users.create!(name: 'User 1', username: 'user1') } + # element :some_element + # element :some_element, required: true class ElementWithPattern < RuboCop::Cop::Cop include QAHelpers @@ -22,10 +25,13 @@ module RuboCop return unless in_qa_file?(node) return unless method_name(node).to_s == 'element' - element_name, pattern = node.arguments - return unless pattern + element_name, *args = node.arguments - add_offense(node, location: pattern.source_range, message: MESSAGE % "qa-#{element_name.value.to_s.tr('_', '-')}") + return if args.first.nil? + + args.first.each_node(:str) do |arg| + add_offense(arg, message: MESSAGE % "qa-#{element_name.value.to_s.tr('_', '-')}") + end end private diff --git a/spec/rubocop/cop/qa/element_with_pattern_spec.rb b/spec/rubocop/cop/qa/element_with_pattern_spec.rb index c5beb40f9fd..ef20d9a1f26 100644 --- a/spec/rubocop/cop/qa/element_with_pattern_spec.rb +++ b/spec/rubocop/cop/qa/element_with_pattern_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' @@ -23,7 +25,7 @@ describe RuboCop::Cop::QA::ElementWithPattern do element :groups_filter, 'search_field_tag :filter' ^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use a pattern for element, create a corresponding `qa-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 `qa-groups-filter-placeholder` instead. end RUBY end @@ -35,6 +37,13 @@ describe RuboCop::Cop::QA::ElementWithPattern do element :groups_filter_placeholder end RUBY + + expect_no_offenses(<<-RUBY) + view 'app/views/shared/groups/_search_form.html.haml' do + element :groups_filter, required: true + element :groups_filter_placeholder, required: false + end + RUBY end end