From c6d4004a48638d35eeffbbdfaadfa44bdd02d788 Mon Sep 17 00:00:00 2001 From: Thomas Walpole Date: Fri, 5 Jul 2019 14:12:38 -0700 Subject: [PATCH] Optionally allow `all` results to be reloaded when stale --- lib/capybara/node/element.rb | 17 +++++++------- lib/capybara/node/finders.rb | 5 +++- lib/capybara/node/simple.rb | 2 +- lib/capybara/result.rb | 18 +++++++++++++-- lib/capybara/spec/session/all_spec.rb | 33 +++++++++++++++++++++++++++ lib/capybara/spec/views/form.erb | 2 +- 6 files changed, 64 insertions(+), 13 deletions(-) diff --git a/lib/capybara/node/element.rb b/lib/capybara/node/element.rb index 958f23e279..d7087144d7 100644 --- a/lib/capybara/node/element.rb +++ b/lib/capybara/node/element.rb @@ -27,9 +27,11 @@ def initialize(session, base, query_scope, query) @query_scope = query_scope @query = query @allow_reload = false + @query_idx = nil end - def allow_reload! + def allow_reload!(idx = nil) + @query_idx = idx @allow_reload = true end @@ -539,14 +541,13 @@ def flash # @api private def reload - if @allow_reload - begin - reloaded = @query.resolve_for(query_scope.reload)&.first + return self unless @allow_reload - @base = reloaded.base if reloaded - rescue StandardError => e - raise e unless catch_error?(e) - end + begin + reloaded = @query.resolve_for(query_scope.reload)[@query_idx.to_i] + @base = reloaded.base if reloaded + rescue StandardError => e + raise e unless catch_error?(e) end self end diff --git a/lib/capybara/node/finders.rb b/lib/capybara/node/finders.rb index d3989f2aa4..b6431c1597 100644 --- a/lib/capybara/node/finders.rb +++ b/lib/capybara/node/finders.rb @@ -235,13 +235,15 @@ def find_by_id(id, **options, &optional_filter_block) # @option options [Integer] maximum Maximum number of matches that are expected to be found # @option options [Integer] minimum Minimum number of matches that are expected to be found # @option options [Range] between Number of matches found must be within the given range + # @option options [Boolean] allow_reload When `true` allows elements to be reloaded if they become stale. This is an advanced behavior and should only be used + # if you fully understand the potential ramifications. The results can be confusing on dynamic pages. # @overload all([kind = Capybara.default_selector], locator = nil, **options) # @overload all([kind = Capybara.default_selector], locator = nil, **options, &filter_block) # @yieldparam element [Capybara::Node::Element] The element being considered for inclusion in the results # @yieldreturn [Boolean] Should the element be considered in the results? # @return [Capybara::Result] A collection of found elements # @raise [Capybara::ExpectationNotMet] The number of elements found doesn't match the specified conditions - def all(*args, **options, &optional_filter_block) + def all(*args, allow_reload: nil, **options, &optional_filter_block) minimum_specified = options_include_minimum?(options) options = { minimum: 1 }.merge(options) unless minimum_specified options[:session_options] = session_options @@ -250,6 +252,7 @@ def all(*args, **options, &optional_filter_block) begin synchronize(query.wait) do result = query.resolve_for(self) + result.allow_reload! if allow_reload raise Capybara::ExpectationNotMet, result.failure_message unless result.matches_count? result diff --git a/lib/capybara/node/simple.rb b/lib/capybara/node/simple.rb index cd55a412bf..b5208c93a3 100644 --- a/lib/capybara/node/simple.rb +++ b/lib/capybara/node/simple.rb @@ -150,7 +150,7 @@ def synchronize(_seconds = nil) yield # simple nodes don't need to wait end - def allow_reload! + def allow_reload!(*) # no op end diff --git a/lib/capybara/result.rb b/lib/capybara/result.rb index ff63a62f4d..9160b22fb6 100644 --- a/lib/capybara/result.rb +++ b/lib/capybara/result.rb @@ -31,6 +31,7 @@ def initialize(elements, query) @filter_errors = [] @results_enum = lazy_select_elements { |node| query.matches_filters?(node, @filter_errors) } @query = query + @allow_reload = false end def_delegators :full_results, :size, :length, :last, :values_at, :inspect, :sample @@ -43,7 +44,7 @@ def each(&block) @result_cache.each(&block) loop do next_result = @results_enum.next - @result_cache << next_result + add_to_cache(next_result) yield next_result end self @@ -130,13 +131,26 @@ def unfiltered_size @elements.length end + ## + # @api private + # + def allow_reload! + @allow_reload = true + self + end + private + def add_to_cache(elem) + elem.allow_reload!(@result_cache.size) if @allow_reload + @result_cache << elem + end + def load_up_to(num) loop do break if @result_cache.size >= num - @result_cache << @results_enum.next + add_to_cache(@results_enum.next) end @result_cache.size end diff --git a/lib/capybara/spec/session/all_spec.rb b/lib/capybara/spec/session/all_spec.rb index 851952ed0d..2096dd6cab 100644 --- a/lib/capybara/spec/session/all_spec.rb +++ b/lib/capybara/spec/session/all_spec.rb @@ -40,6 +40,39 @@ expect { @session.all('//p', schmoo: 'foo') }.to raise_error(ArgumentError) end + it 'should not reload by default', requires: [:driver] do + paras = @session.all(:css, 'p', minimum: 3) + expect { paras[0].text }.not_to raise_error + @session.refresh + expect { paras[0].text }.to raise_error do |err| + expect(err).to be_an_invalid_element_error(@session) + end + end + + context 'with allow_reload' do + it 'should reload if true' do + paras = @session.all(:css, 'p', allow_reload: true, minimum: 3) + expect { paras[0].text }.not_to raise_error + @session.refresh + sleep 1 # Ensure page has started to reload + expect(paras[0]).to have_text('Lorem ipsum dolor') + expect(paras[1]).to have_text('Duis aute irure dolor') + end + + it 'should not reload if false', requires: [:driver] do + paras = @session.all(:css, 'p', allow_reload: false, minimum: 3) + expect { paras[0].text }.not_to raise_error + @session.refresh + sleep 1 # Ensure page has started to reload + expect { paras[0].text }.to raise_error do |err| + expect(err).to be_an_invalid_element_error(@session) + end + expect { paras[2].text }.to raise_error do |err| + expect(err).to be_an_invalid_element_error(@session) + end + end + end + context 'with css selectors' do it 'should find all elements using the given selector' do expect(@session.all(:css, 'h1').first.text).to eq('This is a test') diff --git a/lib/capybara/spec/views/form.erb b/lib/capybara/spec/views/form.erb index 03369d5b19..fae287b69a 100644 --- a/lib/capybara/spec/views/form.erb +++ b/lib/capybara/spec/views/form.erb @@ -11,7 +11,7 @@ -

+