Optimize Capybara::Result by evaluating filters lazily when possible

History.md

@@ -18,6 +18,7 @@ Release date: Unreleased
 * Improved error messages for have_text matcher [Alex Chaffee, Thomas Walpole]
 * The `:with` option for the field selector now accepts a regular expression for matching the field value [Uwe Kubosch]
 * Support matching on aria-label attribute when finding fields/links/buttons - Issue #1528 [Thomas Walpole]
+* Optimize Capybara::Result to only apply fields as necessary in common use-case of `.all[idx]` [Thomas Walpole]
 
 #Version 2.7.1
 Release date: 2016-05-01

lib/capybara/node/finders.rb

@@ -33,14 +33,14 @@ module Capybara
         synchronize(query.wait) do
           if query.match == :smart or query.match == :prefer_exact
             result = query.resolve_for(self, true)
-            result = query.resolve_for(self, false) if result.size == 0 && !query.exact?
+            result = query.resolve_for(self, false) if result.empty? && !query.exact?
           else
             result = query.resolve_for(self)
           end
           if query.match == :one or query.match == :smart and result.size > 1
             raise Capybara::Ambiguous.new("Ambiguous match, found #{result.size} elements matching #{query.description}")
           end
-          if result.size == 0
+          if result.empty?
             raise Capybara::ElementNotFound.new("Unable to find #{query.description}")
           end
           result.first

lib/capybara/node/matchers.rb

@@ -123,8 +123,7 @@ module Capybara
         query = Capybara::Queries::SelectorQuery.new(*args)
         synchronize(query.wait) do
           result = query.resolve_for(self)
-          matches_count = Capybara::Helpers.matches_count?(result.size, query.options)
+          unless result.matches_count? && ((!result.empty?) || Capybara::Helpers.expects_none?(query.options))
-          unless matches_count && ((result.size > 0) || Capybara::Helpers.expects_none?(query.options))
             raise Capybara::ExpectationNotMet, result.failure_message
           end
         end
@@ -151,8 +150,7 @@ module Capybara
         query = Capybara::Queries::SelectorQuery.new(*args)
         synchronize(query.wait) do
           result = query.resolve_for(self)
-          matches_count = Capybara::Helpers.matches_count?(result.size, query.options)
+          if result.matches_count? && ((!result.empty?) || Capybara::Helpers.expects_none?(query.options))
-          if matches_count && ((result.size > 0) || Capybara::Helpers.expects_none?(query.options))
             raise Capybara::ExpectationNotMet, result.negative_failure_message
           end
         end

lib/capybara/queries/selector_query.rb

@@ -45,15 +45,19 @@ module Capybara
           regexp = options[:text].is_a?(Regexp) ? options[:text] : Regexp.escape(options[:text].to_s)
           return false if not node.text(visible).match(regexp)
         end
+
         case visible
           when :visible then return false unless node.visible?
           when :hidden then return false if node.visible?
         end
-        query_filters.each do |name, filter|
+
+        query_filters.all? do |name, filter|
           if options.has_key?(name)
-            return false unless filter.matches?(node, options[name])
+            filter.matches?(node, options[name])
           elsif filter.default?
-            return false unless filter.matches?(node, filter.default)
+            filter.matches?(node, filter.default)
+          else
+            true
           end
         end
       end

lib/capybara/result.rb

@@ -25,27 +25,74 @@ module Capybara
 
     def initialize(elements, query)
       @elements = elements
-      @result = elements.select { |node| query.matches_filters?(node) }
+      @result_cache = []
-      @rest = @elements - @result
+      @results_enum = lazy_select_elements { |node| query.matches_filters?(node) }
       @query = query
     end
 
-    def_delegators :@result, :each, :[], :at, :size, :count, :length,
+    def_delegators :full_results, :size, :length, :last, :values_at, :inspect, :sample
-                   :first, :last, :values_at, :empty?, :inspect, :sample, :index
+
+    alias :index :find_index
+
+    def each(&block)
+      @result_cache.each(&block)
+      loop do
+        next_result = @results_enum.next
+        @result_cache << next_result
+        block.call(next_result)
+      end
+      self
+    end
+
+    def [](*args)
+      if (args.size == 1) && ((idx = args[0]).is_a? Integer) && (idx > 0)
+        @result_cache << @results_enum.next while @result_cache.size <= idx
+        @result_cache[idx]
+      else
+        full_results[*args]
+      end
+    rescue StopIteration
+      return nil
+    end
+    alias :at :[]
+
+    def empty?
+      !any?
+    end
 
     def matches_count?
-      Capybara::Helpers.matches_count?(@result.size, @query.options)
+      return Integer(@query.options[:count]) == count if @query.options[:count]
+
+      return false if @query.options[:between] && !(@query.options[:between] === count)
+
+      if @query.options[:minimum]
+        begin
+          @result_cache << @results_enum.next while @result_cache.size < Integer(@query.options[:minimum])
+        rescue StopIteration
+          return false
+        end
+      end
+
+      if @query.options[:maximum]
+        begin
+          @result_cache << @results_enum.next while @result_cache.size <= Integer(@query.options[:maximum])
+          return false
+        rescue StopIteration
+        end
+      end
+
+      return true
     end
 
     def failure_message
       message = Capybara::Helpers.failure_message(@query.description, @query.options)
       if count > 0
-        message << ", found #{count} #{Capybara::Helpers.declension("match", "matches", count)}: " << @result.map(&:text).map(&:inspect).join(", ")
+        message << ", found #{count} #{Capybara::Helpers.declension("match", "matches", count)}: " << full_results.map(&:text).map(&:inspect).join(", ")
       else
         message << " but there were no matches"
       end
-      unless @rest.empty?
+      unless rest.empty?
-        elements = @rest.map(&:text).map(&:inspect).join(", ")
+        elements = rest.map(&:text).map(&:inspect).join(", ")
         message << ". Also found " << elements << ", which matched the selector but not all filters."
       end
       message
@@ -54,5 +101,30 @@ module Capybara
     def negative_failure_message
       failure_message.sub(/(to find)/, 'not \1')
     end
+
+    private
+
+    def full_results
+      loop do
+        @result_cache << @results_enum.next
+      end
+      @result_cache
+    end
+
+    def rest
+      @rest ||= @elements - full_results
+    end
+
+    def lazy_select_elements(&block)
+      if @elements.respond_to? :lazy  #Ruby 2.0+
+        @elements.lazy.select &block
+      else
+        Enumerator.new do |yielder|
+          @elements.each do |val|
+            yielder.yield(val) if block.call(val)
+          end
+        end
+      end
+    end
   end
 end

lib/capybara/selector.rb

@@ -158,6 +158,7 @@ Capybara.add_selector(:field) do
     with.is_a?(Regexp) ? node.value =~ with : node.value == with.to_s
   end
   filter(:type) do |node, type|
+    type = type.to_s
     if ['textarea', 'select'].include?(type)
       node.tag_name == type
     else

spec/result_spec.rb

@@ -63,4 +63,29 @@ RSpec.describe Capybara::Result do
       el.text == 'Gamma'
     end).to eq(2)
   end
+
+  it 'supports all modes of []' do
+    expect(result[1].text).to eq 'Beta'
+    expect(result[0,2].map &:text).to eq ['Alpha', 'Beta']
+    expect(result[1..3].map &:text).to eq ['Beta', 'Gamma', 'Delta']
+    expect(result[-1].text).to eq 'Delta'
+  end
+
+  #Not a great test but it indirectly tests what is needed
+  it "should evaluate filters lazily" do
+    #Not processed until accessed
+    expect(result.instance_variable_get('@result_cache').size).to be 0
+
+    #Only one retrieved when needed
+    result.first
+    expect(result.instance_variable_get('@result_cache').size).to be 1
+
+    #works for indexed access
+    result[2]
+    expect(result.instance_variable_get('@result_cache').size).to be 3
+
+    #All cached when converted to array
+    result.to_a
+    expect(result.instance_variable_get('@result_cache').size).to eq 4
+  end
 end