Implement dynamic validation on QA Pages

Elements now have the ability to be required on pages or not
Currently using the default wait mechanism
Altered the ElementWithPattern Cop to fit new splat for init

13 files changed, 139 insertions, 38 deletions

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..7cc7f1a128e
--- /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: 10)
+            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 a5218fc9ab1..8023bee0e6d 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/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
+
+          return if args.first.nil?
 
-          add_offense(node, location: pattern.source_range, message: MESSAGE % "qa-#{element_name.value.to_s.tr('_', '-')}")
+          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