spec: refactor implicit waiting, fixes SD-1939

[ci rspec profile]

basically never use built-in implicit waiting, since it makes everything
wait (not just find_element, but attribute lookups, etc.). that means that
StaleElementProtection would automatically always take the full 15 seconds,
because that's how long we'd wait for the StaleElementReferenceError to
come back

instead roll our own, which allows those to recover almost immediately,
and prevents other unnecessary waiting. disable_implicit_wait is also
now much cheaper since it doesn't mean two ruby <-> firefox roundtrips

test plan:
1. specs should be faster, e.g.
   ./spec/selenium/quizzes/quizzes_unpublish_quiz_teacher_spec.rb:20
   now takes 10 sec instead of 25, since it always has a
   StaleElementReferenceError

Change-Id: Ice31e64a274930cdf8af4407b5632e961683cdf6
Reviewed-on: https://gerrit.instructure.com/98295
Tested-by: Jenkins
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>

Author: jenseng

spec/selenium/assignments/assignments_external_tool_spec.rb

@@ -49,9 +49,7 @@
     f('.add_item_button.ui-button').click
 
     expect(f('#assignment_external_tool_tag_attributes_url')).to have_attribute('value', @t2.url)
-    disable_implicit_wait do # this action results in an expected 404, so don't wait
-      f("#edit_assignment_form button[type='submit']").click
-    end
+    f("#edit_assignment_form button[type='submit']").click
 
     keep_trying_until do # timing issues require waiting
       expect(@course.assignments(true).last).to be_present
@@ -78,9 +76,7 @@
     expect(f('#context_external_tools_select input#external_tool_create_url')).to have_attribute('value', @t1.url)
     f('.add_item_button.ui-button').click
     expect(f('#assignment_external_tool_tag_attributes_url')).to have_attribute('value', @t1.url)
-    disable_implicit_wait do # this action results in an expected 404, so don't wait
-      f("#edit_assignment_form button[type='submit']").click
-    end
+    f("#edit_assignment_form button[type='submit']").click
 
     keep_trying_until do # timing issues require waiting
       a.reload

spec/selenium/context_modules_teacher_spec.rb

@@ -135,19 +135,19 @@ def module_with_two_items
 
       f('.ig-header-admin  .al-trigger').click
       hover_and_click('#context_modules .edit_module_link')
-      wait_for_ajax_requests
+      wait_for_animations
       expect(f('#add_context_module_form')).to be_displayed
       f('.add_completion_criterion_link').click
-      assignment_picker = fj('.assignment_picker:visible')
-
-      assignment_picker.find_element(:css, "option[value='#{content_tag_1.id}']").click
-      requirement_picker = fj('.assignment_requirement_picker:visible')
-      requirement_picker.find_element(:css, 'option[value="min_score"]').click
-      expect(driver.execute_script('return $(".points_possible_parent:visible").length')).to be > 0
-
-      assignment_picker.find_element(:css, "option[value='#{content_tag_2.id}']").click
-      requirement_picker.find_element(:css, 'option[value="min_score"]').click
-      expect(driver.execute_script('return $(".points_possible_parent:visible").length')).to eq 0
+      wait_for_animations
+      fj(".assignment_picker:visible option[value='#{content_tag_1.id}']").click
+      wait_for_js # the action above might be changing the element underneath between finding and clicking
+      fj('.assignment_requirement_picker:visible option[value="min_score"]').click
+      expect(f("body")).to contain_jqcss(".points_possible_parent:visible")
+
+      fj(".assignment_picker:visible option[value='#{content_tag_2.id}']").click
+      wait_for_js # the action above might be changing the element underneath between finding and clicking
+      fj('.assignment_requirement_picker:visible option[value="min_score"]').click
+      expect(f("body")).not_to contain_jqcss(".points_possible_parent:visible")
     end
 
     it "should add and remove completion criteria" do

spec/selenium/eportfolios_spec.rb

@@ -173,6 +173,7 @@
     f("#right-side .edit_content_link").click
     wait_for_ajaximations
     f('.add_file_link').click
+    wait_for_animations
     fj(".file_upload:visible").send_keys(fullpath)
     wait_for_ajaximations
     f(".upload_file_button").click

spec/selenium/grades/page_objects/srgb_page.rb

@@ -21,7 +21,7 @@ def submission_details_button
     end
 
     def notes_field
-      f('#student_information textarea')
+      fj('#student_information textarea:visible:not([disabled])')
     end
 
     def final_grade

spec/selenium/helpers/wiki_and_tiny_common.rb

@@ -148,6 +148,7 @@ def add_image_to_rce
     clear_wiki_rce
     f('#editor_tabs .ui-tabs-nav li:nth-child(3) a').click
     f('.upload_new_image_link').click
+    wait_for_animations
     wiki_page_tools_upload_file('#sidebar_upload_image_form', :image)
     in_frame wiki_page_body_ifr_id do
       expect(f('#tinymce img')).to be_displayed

spec/selenium/test_setup/common_helper_methods/custom_selenium_actions.rb

@@ -62,7 +62,7 @@ def fln(link_text, scope = nil)
   #   expect(f('#content')).not_to contain_jqcss('.gone:visible')
   def fj(selector, scope = nil)
     stale_element_protection do
-      wait_for(method: :fj, timeout: driver.manage.timeouts.implicit_wait) do
+      wait_for(method: :fj) do
         find_with_jquery selector, scope
       end or raise Selenium::WebDriver::Error::NoSuchElementError
     end
@@ -73,11 +73,14 @@ def fj(selector, scope = nil)
   # like other selenium methods, this will wait until it finds elements on
   # the page, and will eventually raise if none are found
   def ff(selector, scope = nil)
-    result = reloadable_collection do
-      (scope || driver).find_elements :css, selector
+    reloadable_collection do
+      result = nil
+      wait_for(method: :ff) do
+        result = disable_implicit_wait { (scope || driver).find_elements(:css, selector) }
+        result.present?
+      end or raise Selenium::WebDriver::Error::NoSuchElementError
+      result
     end
-    raise Selenium::WebDriver::Error::NoSuchElementError unless result.present?
-    result
   end
 
   # same as `fj`, but returns all matching elements
@@ -87,7 +90,7 @@ def ff(selector, scope = nil)
   def ffj(selector, scope = nil)
     reloadable_collection do
       result = nil
-      wait_for(method: :ffj, timeout: driver.manage.timeouts.implicit_wait) do
+      wait_for(method: :ffj) do
         result = find_all_with_jquery(selector, scope)
         result.present?
       end or raise Selenium::WebDriver::Error::NoSuchElementError

spec/selenium/test_setup/common_helper_methods/custom_validators.rb

@@ -49,24 +49,20 @@ def check_element_attrs(element, attrs)
 
   def expect_flash_message(type = :warning, message = nil)
     message = Regexp.new(Regexp.escape(message)) if message.is_a?(String)
-    disable_implicit_wait do
-      wait_for method: :expect_flash_message, ignore: [Selenium::WebDriver::Error::StaleElementReferenceError] do
-        messages = driver.find_elements :css, "#flash_message_holder .ic-flash-#{type}"
-        text = messages.map(&:text).join('\n')
-        message ? !!text.match(message) : messages.present?
-      end or raise(RSpec::Expectations::ExpectationNotMetError, "expected flash #{type} message#{message ? " " + message.inspect : ""}, none found")
-    end
+    wait_for method: :expect_flash_message, ignore: [Selenium::WebDriver::Error::StaleElementReferenceError] do
+      messages = disable_implicit_wait { driver.find_elements :css, "#flash_message_holder .ic-flash-#{type}" }
+      text = messages.map(&:text).join('\n')
+      message ? !!text.match(message) : messages.present?
+    end or raise(RSpec::Expectations::ExpectationNotMetError, "expected flash #{type} message#{message ? " " + message.inspect : ""}, none found")
   end
 
   def expect_no_flash_message(type = :warning, message = nil)
     message = Regexp.new(Regexp.escape(message)) if message.is_a?(String)
-    disable_implicit_wait do
-      wait_for method: :expect_no_flash_message, ignore: [Selenium::WebDriver::Error::StaleElementReferenceError] do
-        messages = driver.find_elements :css, "#flash_message_holder .ic-flash-#{type}"
-        text = messages.map(&:text).join('\n')
-        message ? !text.match(message) : messages.empty?
-      end or raise(RSpec::Expectations::ExpectationNotMetError, "expected no flash #{type} message#{message ? " " + message.inspect : ""}, one was found")
-    end
+    wait_for method: :expect_no_flash_message, ignore: [Selenium::WebDriver::Error::StaleElementReferenceError] do
+      messages = disable_implicit_wait { driver.find_elements :css, "#flash_message_holder .ic-flash-#{type}" }
+      text = messages.map(&:text).join('\n')
+      message ? !text.match(message) : messages.empty?
+    end or raise(RSpec::Expectations::ExpectationNotMetError, "expected no flash #{type} message#{message ? " " + message.inspect : ""}, one was found")
   end
 
   def assert_flash_notice_message(okay_message_regex)
@@ -93,7 +89,7 @@ def assert_error_box(selector)
   def expect_new_page_load(accept_alert = false)
     driver.execute_script("window.INST = window.INST || {}; INST.still_on_old_page = true;")
     yield
-    wait_for do
+    wait_for method: :expect_new_page_load do
       begin
         driver.execute_script("return window.INST && INST.still_on_old_page !== true;")
       rescue Selenium::WebDriver::Error::UnhandledAlertError, Selenium::WebDriver::Error::UnknownError

spec/selenium/test_setup/common_helper_methods/custom_wait_methods.rb

@@ -142,7 +142,7 @@ def wait_for_tiny(element)
     tiny_frame = nil
     keep_trying_until {
       begin
-        tiny_frame = parent.find_element(:css, 'iframe')
+        tiny_frame = disable_implicit_wait { parent.find_element(:css, 'iframe') }
       rescue => e
         puts "#{e.inspect}"
         false
@@ -152,32 +152,27 @@ def wait_for_tiny(element)
   end
 
   def disable_implicit_wait
-    driver.manage.timeouts.implicit_wait = 0
-    yield
-  ensure
-    driver.manage.timeouts.implicit_wait = SeleniumDriverSetup::IMPLICIT_WAIT_TIMEOUT
+    ::SeleniumExtensions::FinderWaiting.disable do
+      yield
+    end
   end
 
   # little wrapper around Selenium::WebDriver::Wait, notably it:
   # * is less verbose
   # * returns false (rather than raising) if the block never returns true
   # * doesn't rescue :allthethings: like keep_trying_until
   # * prevents nested waiting, cuz that's terrible
-  def wait_for(timeout: SeleniumDriverSetup::IMPLICIT_WAIT_TIMEOUT, method: nil, ignore: nil)
-    return yield if timeout == 0
-    driver.prevent_nested_waiting(method) do
-      Selenium::WebDriver::Wait.new(timeout: timeout, ignore: ignore).until do
-        yield
-      end
-    end
-  rescue Selenium::WebDriver::Error::TimeOutError
-    false
+  def wait_for(*args, &block)
+    ::SeleniumExtensions::FinderWaiting.wait_for(*args, &block)
   end
 
   def wait_for_no_such_element(method: nil)
     wait_for(method: method, ignore: []) do
-      yield
-      false
+      # so find_element calls return ASAP
+      disable_implicit_wait do
+        yield
+        false
+      end
     end
   rescue Selenium::WebDriver::Error::NoSuchElementError
     true

spec/selenium/test_setup/custom_selenium_rspec_matchers.rb

@@ -150,9 +150,7 @@
   end
 
   match_when_negated do |element|
-    disable_implicit_wait do # so find_element calls return ASAP
-      wait_for_no_such_element(method: :contain_css) { f(selector, element) }
-    end
+    wait_for_no_such_element(method: :contain_css) { f(selector, element) }
   end
 end
 
@@ -193,9 +191,7 @@
   end
 
   match_when_negated do |element|
-    disable_implicit_wait do # so find_element calls return ASAP
-      wait_for_no_such_element(method: :contain_link) { fln(text, element) }
-    end
+    wait_for_no_such_element(method: :contain_link) { fln(text, element) }
   end
 end
 
@@ -242,10 +238,8 @@ def supports_block_expectations?
 
   match do |actual|
     raise "The `become` matcher expects a block, e.g. `expect { actual }.to become(value)`, NOT `expect(actual).to become(value)`" unless actual.is_a? Proc
-    disable_implicit_wait do
-      wait_for(method: :become) do
-        actual.call == expected
-      end
+    wait_for(method: :become) do
+      disable_implicit_wait { actual.call == expected }
     end
   end
 end

spec/selenium/test_setup/selenium_driver_setup.rb

@@ -118,7 +118,8 @@ def start_driver
 
       focus_viewport if run_headless?
 
-      @driver.manage.timeouts.implicit_wait = IMPLICIT_WAIT_TIMEOUT
+      @driver.manage.timeouts.implicit_wait = 0 # nothing should wait by default
+      SeleniumExtensions::FinderWaiting.timeout = IMPLICIT_WAIT_TIMEOUT # except finding elements
       @driver.manage.timeouts.script_timeout = 60
 
       puts "Browser: #{browser_name} - #{browser_version}"