Support locator type checking and add a warning when locator looks to be CSS/XPath but shouldn't be (#2143)

* Support locator type checking and add a warning when locator looks to be CSS/XPath but shouldn't be

* Raise error if an element is passed to the :from option of select/unselect
This commit is contained in:
Thomas Walpole 2018-12-14 11:53:09 -08:00 committed by GitHub
parent 06caef7e27
commit 7fca910080
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 90 additions and 27 deletions

View File

@ -165,8 +165,8 @@ module Capybara
# @param [Symbol] name The name of the selector to add
# @yield A block executed in the context of the new {Capybara::Selector}
#
def add_selector(name, &block)
Capybara::Selector.add(name, &block)
def add_selector(name, **options, &block)
Capybara::Selector.add(name, **options, &block)
end
##

View File

@ -195,6 +195,8 @@ module Capybara
#
# @return [Capybara::Node::Element] The option element selected
def select(value = nil, from: nil, **options)
raise ArgumentError, 'The :from option does not take an element' if from.is_a? Capybara::Node::Element
el = from ? find_select_or_datalist_input(from, options) : self
if el.respond_to?(:tag_name) && (el.tag_name == 'input')
@ -221,6 +223,8 @@ module Capybara
#
# @return [Capybara::Node::Element] The option element unselected
def unselect(value = nil, from: nil, **options)
raise ArgumentError, 'The :from option does not take an element' if from.is_a? Capybara::Node::Element
scope = from ? find(:select, from, options) : self
scope.find(:option, value, options).unselect_option
end

View File

@ -62,6 +62,12 @@ module Capybara
desc << selector.description(node_filters: show_for[:node], **options)
desc << ' that also matches the custom filter block' if @filter_block && show_for[:node]
desc << " within #{@resolved_node.inspect}" if describe_within?
if locator.is_a?(String) && locator.start_with?('#', './/', '//')
unless selector.raw_locator?
desc << "\nNote: It appears you may be passing a CSS selector or XPath expression rather than a locator. " \
"Please see the documentation for acceptable locator values.\n\n"
end
end
desc
end

View File

@ -25,20 +25,20 @@ end
# rubocop:disable Metrics/BlockLength
Capybara.add_selector(:xpath) do
Capybara.add_selector(:xpath, locator_type: [:to_xpath, String], raw_locator: true) do
xpath { |xpath| xpath }
end
Capybara.add_selector(:css) do
Capybara.add_selector(:css, locator_type: [String, Symbol], raw_locator: true) do
css { |css| css }
end
Capybara.add_selector(:id) do
Capybara.add_selector(:id, locator_type: [String, Symbol, Regexp]) do
xpath { |id| builder(XPath.descendant).add_attribute_conditions(id: id) }
locator_filter { |node, id| id.is_a?(Regexp) ? node[:id] =~ id : true }
end
Capybara.add_selector(:field) do
Capybara.add_selector(:field, locator_type: [String, Symbol]) do
visible { |options| :hidden if options[:type].to_s == 'hidden' }
xpath do |locator, **options|
invalid_types = %w[submit image]
@ -78,7 +78,7 @@ Capybara.add_selector(:field) do
end
end
Capybara.add_selector(:fieldset) do
Capybara.add_selector(:fieldset, locator_type: [String, Symbol]) do
xpath do |locator, legend: nil, **|
locator_matchers = (XPath.attr(:id) == locator.to_s) | XPath.child(:legend)[XPath.string.n.is(locator.to_s)]
locator_matchers |= XPath.attr(test_id) == locator.to_s if test_id
@ -90,7 +90,7 @@ Capybara.add_selector(:fieldset) do
node_filter(:disabled, :boolean) { |node, value| !(value ^ node.disabled?) }
end
Capybara.add_selector(:link) do
Capybara.add_selector(:link, locator_type: [String, Symbol]) do
xpath do |locator, href: true, alt: nil, title: nil, **|
xpath = builder(XPath.descendant(:a)).add_attribute_conditions(href: href)
@ -132,7 +132,7 @@ Capybara.add_selector(:link) do
end
end
Capybara.add_selector(:button) do
Capybara.add_selector(:button, locator_type: [String, Symbol]) do
xpath(:value, :title, :type) do |locator, **options|
input_btn_xpath = XPath.descendant(:input)[XPath.attr(:type).one_of('submit', 'reset', 'image', 'button')]
btn_xpath = XPath.descendant(:button)
@ -166,7 +166,7 @@ Capybara.add_selector(:button) do
end
end
Capybara.add_selector(:link_or_button) do
Capybara.add_selector(:link_or_button, locator_type: [String, Symbol]) do
label 'link or button'
xpath do |locator, **options|
self.class.all.values_at(:link, :button).map do |selector|
@ -181,9 +181,8 @@ Capybara.add_selector(:link_or_button) do
end
end
Capybara.add_selector(:fillable_field) do
Capybara.add_selector(:fillable_field, locator_type: [String, Symbol]) do
label 'field'
xpath do |locator, allow_self: nil, **options|
xpath = XPath.axis(allow_self ? :"descendant-or-self" : :descendant, :input, :textarea)[
!XPath.attr(:type).one_of('submit', 'image', 'radio', 'checkbox', 'hidden', 'file')
@ -215,9 +214,8 @@ Capybara.add_selector(:fillable_field) do
end
end
Capybara.add_selector(:radio_button) do
Capybara.add_selector(:radio_button, locator_type: [String, Symbol]) do
label 'radio button'
xpath do |locator, allow_self: nil, **options|
xpath = XPath.axis(allow_self ? :"descendant-or-self" : :descendant, :input)[
XPath.attr(:type) == 'radio'
@ -240,7 +238,7 @@ Capybara.add_selector(:radio_button) do
end
end
Capybara.add_selector(:checkbox) do
Capybara.add_selector(:checkbox, locator_type: [String, Symbol]) do
xpath do |locator, allow_self: nil, **options|
xpath = XPath.axis(allow_self ? :"descendant-or-self" : :descendant, :input)[
XPath.attr(:type) == 'checkbox'
@ -263,7 +261,7 @@ Capybara.add_selector(:checkbox) do
end
end
Capybara.add_selector(:select) do
Capybara.add_selector(:select, locator_type: [String, Symbol]) do
label 'select box'
xpath do |locator, **options|
@ -320,7 +318,7 @@ Capybara.add_selector(:select) do
end
end
Capybara.add_selector(:datalist_input) do
Capybara.add_selector(:datalist_input, locator_type: [String, Symbol]) do
label 'input box with datalist completion'
xpath do |locator, **options|
@ -355,7 +353,7 @@ Capybara.add_selector(:datalist_input) do
end
end
Capybara.add_selector(:option) do
Capybara.add_selector(:option, locator_type: [String, Symbol]) do
xpath do |locator|
xpath = XPath.descendant(:option)
xpath = xpath[XPath.string.n.is(locator.to_s)] unless locator.nil?
@ -373,7 +371,7 @@ Capybara.add_selector(:option) do
end
end
Capybara.add_selector(:datalist_option) do
Capybara.add_selector(:datalist_option, locator_type: [String, Symbol]) do
label 'datalist option'
visible(:all)
@ -390,7 +388,7 @@ Capybara.add_selector(:datalist_option) do
end
end
Capybara.add_selector(:file_field) do
Capybara.add_selector(:file_field, locator_type: [String, Symbol]) do
label 'file field'
xpath do |locator, allow_self: nil, **options|
xpath = XPath.axis(allow_self ? :"descendant-or-self" : :descendant, :input)[
@ -404,7 +402,7 @@ Capybara.add_selector(:file_field) do
describe_expression_filters
end
Capybara.add_selector(:label) do
Capybara.add_selector(:label, locator_type: [String, Symbol]) do
label 'label'
xpath(:for) do |locator, options|
xpath = XPath.descendant(:label)
@ -442,7 +440,7 @@ Capybara.add_selector(:label) do
end
end
Capybara.add_selector(:table) do
Capybara.add_selector(:table, locator_type: [String, Symbol]) do
xpath do |locator, caption: nil, **|
xpath = XPath.descendant(:table)
unless locator.nil?
@ -459,7 +457,7 @@ Capybara.add_selector(:table) do
end
end
Capybara.add_selector(:frame) do
Capybara.add_selector(:frame, locator_type: [String, Symbol]) do
xpath do |locator, name: nil, **|
xpath = XPath.descendant(:iframe).union(XPath.descendant(:frame))
unless locator.nil?
@ -475,7 +473,7 @@ Capybara.add_selector(:frame) do
end
end
Capybara.add_selector(:element) do
Capybara.add_selector(:element, locator_type: [String, Symbol]) do
xpath do |locator, **|
XPath.descendant.where(locator ? XPath.local_name == locator.to_s : nil)
end

View File

@ -184,8 +184,8 @@ module Capybara
all.fetch(name.to_sym) { |sel_type| raise ArgumentError, "Unknown selector type (:#{sel_type})" }
end
def add(name, &block)
all[name.to_sym] = Capybara::Selector.new(name.to_sym, &block)
def add(name, **options, &block)
all[name.to_sym] = Capybara::Selector.new(name.to_sym, **options, &block)
end
def update(name, &block)
@ -201,7 +201,7 @@ module Capybara
end
end
def initialize(name, &block)
def initialize(name, locator_type: nil, raw_locator: false, &block)
@name = name
@filter_set = FilterSet.add(name) {}
@match = nil
@ -212,6 +212,8 @@ module Capybara
@expression_filters = {}
@locator_filter = nil
@default_visibility = nil
@locator_type = locator_type
@raw_locator = raw_locator
@config = {
enable_aria_label: false,
test_id: nil
@ -312,6 +314,8 @@ module Capybara
else
warn 'Selector has no format'
end
ensure
warn "Locator #{locator.inspect} must #{locator_description}. This will raise an error in a future version of Capybara." unless locator_valid?(locator)
end
##
@ -439,8 +443,37 @@ module Capybara
Thread.current["capybara_#{object_id}_errors"] = nil
end
# @api private
def raw_locator?
!!@raw_locator
end
private
def locator_types
return nil unless @locator_type
Array(@locator_type)
end
def locator_valid?(locator)
return true unless locator && locator_types
locator_types&.any? do |type_or_method|
type_or_method.is_a?(Symbol) ? locator.respond_to?(type_or_method) : type_or_method === locator # rubocop:disable Style/CaseEquality
end
end
def locator_description
locator_types.group_by { |lt| lt.is_a? Symbol }.map do |symbol, types_or_methods|
if symbol
"respond to #{types_or_methods.join(' or ')}"
else
"be an instance of #{types_or_methods.join(' or ')}"
end
end.join(' or ')
end
def errors
Thread.current["capybara_#{object_id}_errors"] || []
end

View File

@ -242,4 +242,10 @@ Capybara::SpecHelper.spec '#fill_in' do
el = @session.find(:fillable_field, 'form_first_name')
expect(@session.fill_in('form_first_name', with: 'Harry')).to eq el
end
it 'should warn if passed what looks like a CSS id selector' do
expect do
@session.fill_in('#form_first_name', with: 'Harry')
end.to raise_error(/you may be passing a CSS selector or XPath expression rather than a locator/)
end
end

View File

@ -92,6 +92,12 @@ Capybara::SpecHelper.spec '#find' do
expect(@session.find(:css, '#\31 escape\.me').text).to eq('needs escaping')
expect(@session.find(:css, '.\32 escape').text).to eq('needs escaping')
end
it 'should not warn about locator' do
expect { @session.find(:css, '#not_on_page') }.to raise_error Capybara::ElementNotFound do |e|
expect(e.message).not_to match(/you may be passing a CSS selector or XPath expression/)
end
end
end
context 'with xpath selectors' do
@ -99,6 +105,11 @@ Capybara::SpecHelper.spec '#find' do
expect(@session.find(:xpath, '//h1').text).to eq('This is a test')
expect(@session.find(:xpath, "//input[@id='test_field']").value).to eq('monkey')
end
it 'should warn if passed a non-valid locator type', :focus_ do
expect_any_instance_of(Kernel).to receive(:warn).with(/must respond to to_xpath or be an instance of String/)
expect { @session.find(:xpath, 123) }.to raise_error # rubocop:disable RSpec/UnspecifiedException
end
end
context 'with custom selector' do

View File

@ -226,4 +226,9 @@ Capybara::SpecHelper.spec '#select' do
end
end
end
it 'should error when not passed a locator for :from option' do
select = @session.find(:select, 'Title')
expect { @session.select('Mr', from: select) }.to raise_error(ArgumentError, /does not take an element/)
end
end