refactor some complex methods

lib/capybara/queries/selector_query.rb

@@ -61,8 +61,7 @@ module Capybara
         return true if (@resolved_node&.== node) && options[:allow_self]
 
         @applied_filters ||= :system
-        return false unless matches_id_filter?(node) && matches_class_filter?(node)
-        return false unless matches_text_filter?(node) && matches_exact_text_filter?(node) && matches_visible_filter?(node)
+        return false unless matches_system_filters?(node)
 
         @applied_filters = :node
         matches_node_filters?(node) && matches_filter_block?(node)
@@ -292,6 +291,14 @@ module Capybara
         node.is_a?(::Capybara::Node::Simple) && node.path == '/'
       end
 
+      def matches_system_filters?(node)
+        matches_id_filter?(node) &&
+          matches_class_filter?(node) &&
+          matches_text_filter?(node) &&
+          matches_exact_text_filter?(node) &&
+          matches_visible_filter?(node)
+      end
+
       def matches_id_filter?(node)
         return true unless use_default_id_filter? && options[:id].is_a?(Regexp)
 

lib/capybara/rack_test/browser.rb

@@ -57,25 +57,26 @@ class Capybara::RackTest::Browser
   end
 
   def process(method, path, attributes = {}, env = {})
-    new_uri = URI.parse(path)
-    method.downcase! unless method.is_a? Symbol
-    if path.empty?
-      new_uri.path = request_path
-    else
-      new_uri.path = request_path if path.start_with?('?')
-      new_uri.path = '/' if new_uri.path.empty?
-      new_uri.path = request_path.sub(%r{/[^/]*$}, '/') + new_uri.path unless new_uri.path.start_with?('/')
-    end
-    new_uri.scheme ||= @current_scheme
-    new_uri.host ||= @current_host
-    new_uri.port ||= @current_port unless new_uri.default_port == @current_port
-
+    method = method.downcase
+    new_uri = build_uri(path)
     @current_scheme, @current_host, @current_port = new_uri.select(:scheme, :host, :port)
 
     reset_cache!
     send(method, new_uri.to_s, attributes, env.merge(options[:headers] || {}))
   end
 
+  def build_uri(path)
+    URI.parse(path).tap do |uri|
+      uri.path = request_path if path.empty? || path.start_with?('?')
+      uri.path = '/' if uri.path.empty?
+      uri.path = request_path.sub(%r{/[^/]*$}, '/') + uri.path unless uri.path.start_with?('/')
+
+      uri.scheme ||= @current_scheme
+      uri.host ||= @current_host
+      uri.port ||= @current_port unless uri.default_port == @current_port
+    end
+  end
+
   def current_url
     last_request.url
   rescue Rack::Test::Error

lib/capybara/result.rb

@@ -73,28 +73,25 @@ module Capybara
     end
 
     def compare_count
+      count, min, max, between = @query.options.values_at(:count, :minimum, :maximum, :between)
+
       # Only check filters for as many elements as necessary to determine result
-      if (count = @query.options[:count])
-        count = Integer(count)
+      if count && (count = Integer(count))
         return load_up_to(count + 1) <=> count
       end
 
-      if (min = @query.options[:minimum])
-        min = Integer(min)
+      if min && (min = Integer(min))
         return -1 if load_up_to(min) < min
       end
 
-      if (max = @query.options[:maximum])
-        max = Integer(max)
+      if max && (max = Integer(max))
         return 1 if load_up_to(max + 1) > max
       end
 
-      if (between = @query.options[:between])
+      if between
         min, max = between.minmax
         size = load_up_to(max + 1)
-        return 0 if between.include? size
-
-        return size <=> min
+        return between.include?(size) ? 0 : size <=> min
       end
 
       0

lib/capybara/selenium/driver.rb

@@ -107,33 +107,17 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
     navigated = false
     timer = Capybara::Helpers.timer(expire_in: 10)
     begin
-      unless navigated
-        # Only trigger a navigation if we haven't done it already, otherwise it
-        # can trigger an endless series of unload modals
-        clear_browser_state
-        @browser.navigate.to('about:blank')
-      end
+      # Only trigger a navigation if we haven't done it already, otherwise it
+      # can trigger an endless series of unload modals
+      reset_browser_state unless navigated
       navigated = true
-
       # Ensure the page is empty and trigger an UnhandledAlertError for any modals that appear during unload
-      until find_xpath('/html/body/*').empty?
-        raise Capybara::ExpectationNotMet, 'Timed out waiting for Selenium session reset' if timer.expired?
-
-        sleep 0.05
-      end
+      wait_for_empty_page(timer)
     rescue Selenium::WebDriver::Error::UnhandledAlertError, Selenium::WebDriver::Error::UnexpectedAlertOpenError
       # This error is thrown if an unhandled alert is on the page
       # Firefox appears to automatically dismiss this alert, chrome does not
       # We'll try to accept it
-      begin
-        @browser.switch_to.alert.accept
-        sleep 0.25 # allow time for the modal to be handled
-      rescue modal_error
-        # The alert is now gone.
-        # If navigation has not occurred attempt again and accept alert
-        # since FF may have dismissed the alert at first attempt.
-        navigate_with_accept('about:blank') if current_url != 'about:blank'
-      end
+      accept_unhandled_reset_alert
       # try cleaning up the browser again
       retry
     end
@@ -382,6 +366,29 @@ private
       exit @exit_status if @exit_status # Force exit with stored status
     end
   end
+
+  def reset_browser_state
+    clear_browser_state
+    @browser.navigate.to('about:blank')
+  end
+
+  def wait_for_empty_page(timer)
+    until find_xpath('/html/body/*').empty?
+      raise Capybara::ExpectationNotMet, 'Timed out waiting for Selenium session reset' if timer.expired?
+
+      sleep 0.05
+    end
+  end
+
+  def accept_unhandled_reset_alert
+    @browser.switch_to.alert.accept
+    sleep 0.25 # allow time for the modal to be handled
+  rescue modal_error
+    # The alert is now gone.
+    # If navigation has not occurred attempt again and accept alert
+    # since FF may have dismissed the alert at first attempt.
+    navigate_with_accept('about:blank') if current_url != 'about:blank'
+  end
 end
 
 require 'capybara/selenium/driver_specializations/chrome_driver'

lib/capybara/selenium/nodes/marionette_node.rb

@@ -36,16 +36,11 @@ class Capybara::Selenium::MarionetteNode < Capybara::Selenium::Node
     return super if browser_version >= 62.0
 
     # Workaround lack of support for multiple upload by uploading one at a time
-    path_names = value.to_s.empty? ? [] : value
-    Array(path_names).each do |path|
-      unless driver.browser.respond_to?(:upload)
-        if (fd = bridge.file_detector)
-          local_file = fd.call([path])
-          path = upload(local_file) if local_file
-        end
-      end
-      native.send_keys(path)
+    path_names = value.to_s.empty? ? [] : Array(value)
+    if (fd = bridge.file_detector) && !driver.browser.respond_to?(:upload)
+      path_names.map! { |path| upload(fd.call([path])) || path }
     end
+    path_names.each { |path| native.send_keys(path) }
   end
 
   def send_keys(*args)
@@ -101,6 +96,8 @@ private
   end
 
   def upload(local_file)
+    return nil unless local_file
+
     unless File.file?(local_file)
       raise ArgumentError, "You may only upload files: #{local_file.inspect}"
     end