Merge pull request #2068 from teamcapybara/clearer_selector_descripti…

…on_errors

Only include node filters in error descriptions if at least one was applied

features/step_definitions/capybara_steps.rb

@@ -21,5 +21,5 @@
 end
 
 Then(/^the failing exception should be nice$/) do
-  expect(@error_message).to match(/expected to find visible css \"h1#doesnotexist\"/)
+  expect(@error_message).to match(/expected to find css \"h1#doesnotexist\"/)
 end

lib/capybara/node/finders.rb

@@ -290,8 +290,8 @@ def synced_resolve(query)
             result = query.resolve_for(self)
           end
 
-          raise Capybara::Ambiguous, "Ambiguous match, found #{result.size} elements matching #{query.description}" if ambiguous?(query, result)
-          raise Capybara::ElementNotFound, "Unable to find #{query.description}" if result.empty?
+          raise Capybara::Ambiguous, "Ambiguous match, found #{result.size} elements matching #{query.applied_description}" if ambiguous?(query, result)
+          raise Capybara::ElementNotFound, "Unable to find #{query.applied_description}" if result.empty?
 
           result.first
         end.tap(&:allow_reload!)

lib/capybara/queries/ancestor_query.rb

@@ -12,7 +12,7 @@ def resolve_for(node, exact = nil)
         end
       end
 
-      def description
+      def description(applied = false)
         child_query = @child_node&.instance_variable_get(:@query)
         desc = super
         desc += " that is an ancestor of #{child_query.description}" if child_query

lib/capybara/queries/selector_query.rb

@@ -30,22 +30,31 @@ def initialize(*args, session_options:, **options, &filter_block)
       def name; selector.name; end
       def label; selector.label || selector.name; end
 
-      def description
+      def description(applied = false)
         @description = +""
-        @description << "visible " if visible == :visible
-        @description << "non-visible " if visible == :hidden
+        if !applied || @applied_filters
+          @description << "visible " if visible == :visible
+          @description << "non-visible " if visible == :hidden
+        end
         @description << "#{label} #{locator.inspect}"
-        @description << " with#{' exact' if exact_text == true} text #{options[:text].inspect}" if options[:text]
-        @description << " with exact text #{exact_text}" if exact_text.is_a?(String)
+        if !applied || @applied_filters
+          @description << " with#{' exact' if exact_text == true} text #{options[:text].inspect}" if options[:text]
+          @description << " with exact text #{exact_text}" if exact_text.is_a?(String)
+        end
         @description << " with id #{options[:id]}" if options[:id]
         @description << " with classes [#{Array(options[:class]).join(',')}]" if options[:class]
-        @description << selector.description(options)
-        @description << " that also matches the custom filter block" if @filter_block
+        @description << selector.description(skip_node_filters: applied && (@applied_filters != :node), **options)
+        @description << " that also matches the custom filter block" if @filter_block && (!applied || (@applied_filters == :node))
         @description << " within #{@resolved_node.inspect}" if describe_within?
         @description
       end
 
+      def applied_description
+        description(true)
+      end
+
       def matches_filters?(node)
+        @applied_filters ||= :system
         return false if options[:text] && !matches_text_filter(node, options[:text])
         return false if exact_text.is_a?(String) && !matches_exact_text_filter(node, exact_text)
 
@@ -54,6 +63,7 @@ def matches_filters?(node)
         when :hidden then return false if node.visible?
         end
 
+        @applied_filters = :node
         matches_node_filters?(node) && matches_filter_block?(node)
       rescue *(node.respond_to?(:session) ? node.session.driver.invalid_element_errors : [])
         false
@@ -88,6 +98,7 @@ def css
 
       # @api private
       def resolve_for(node, exact = nil)
+        @applied_filters = false
         @resolved_node = node
         node.synchronize do
           children = if selector.format == :css
@@ -110,6 +121,14 @@ def supports_exact?
         @expression.respond_to? :to_xpath
       end
 
+      def failure_message
+        +"expected to find #{applied_description}" << count_message
+      end
+
+      def negative_failure_message
+        +"expected not to find #{applied_description}" << count_message
+      end
+
     private
 
       def find_selector(locator)

lib/capybara/queries/sibling_query.rb

@@ -14,7 +14,7 @@ def resolve_for(node, exact = nil)
         end
       end
 
-      def description
+      def description(applied = false)
         desc = super
         sibling_query = @sibling_node&.instance_variable_get(:@query)
         desc += " that is a sibling of #{sibling_query.description}" if sibling_query

lib/capybara/result.rb

@@ -138,6 +138,10 @@ def negative_failure_message
       failure_message.sub(/(to find)/, 'not \1')
     end
 
+    def unfiltered_size
+      @elements.length
+    end
+
   private
 
     def full_results

lib/capybara/selector.rb

@@ -10,7 +10,7 @@
   expression_filter(:name) { |xpath, val| xpath[XPath.attr(:name) == val] }
   expression_filter(:placeholder) { |xpath, val| xpath[XPath.attr(:placeholder) == val] }
 
-  describe do |checked: nil, unchecked: nil, disabled: nil, multiple: nil, **_options|
+  describe(node_filters: true) do |checked: nil, unchecked: nil, disabled: nil, multiple: nil, **|
     desc, states = +"", []
     states << 'checked' if checked || (unchecked == false)
     states << 'not checked' if unchecked || (checked == false)
@@ -58,17 +58,21 @@
   node_filter(:with) do |node, with|
     with.is_a?(Regexp) ? node.value =~ with : node.value == with.to_s
   end
+
   describe do |type: nil, **options|
     desc = +""
     (expression_filters.keys - [:type]).each { |ef| desc << " with #{ef} #{options[ef]}" if options.key?(ef) }
     desc << " of type #{type.inspect}" if type
-    desc << " with value #{options[:with].to_s.inspect}" if options.key?(:with)
     desc
   end
+
+  describe_node_filters do |**options|
+    " with value #{options[:with].to_s.inspect}" if options.key?(:with)
+  end
 end
 
 Capybara.add_selector(:fieldset) do
-  xpath(:legend) do |locator, legend: nil, **_options|
+  xpath(:legend) 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(Capybara.test_id) == locator if Capybara.test_id
     xpath = XPath.descendant(:fieldset)
@@ -81,7 +85,7 @@
 end
 
 Capybara.add_selector(:link) do
-  xpath(:title, :alt) do |locator, href: true, enable_aria_label: false, alt: nil, title: nil, **_options|
+  xpath(:title, :alt) do |locator, href: true, enable_aria_label: false, alt: nil, title: nil, **|
     xpath = XPath.descendant(:a)
     xpath = xpath[
               case href
@@ -128,8 +132,13 @@
 
   describe do |**options|
     desc = +""
-    desc << " with href #{options[:href].inspect}" if options[:href]
+    desc << " with href #{options[:href].inspect}" if options[:href] && !options[:href].is_a?(Regexp)
     desc << " with no href attribute" if options.fetch(:href, true).nil?
+    desc
+  end
+
+  describe_node_filters do |href: nil, **|
+    " with href matching #{href.inspect}" if href.is_a? Regexp
   end
 end
 
@@ -163,11 +172,9 @@
 
   node_filter(:disabled, :boolean, default: false, skip_if: :all) { |node, value| !(value ^ node.disabled?) }
 
-  describe do |disabled: nil, **options|
-    desc = +""
-    desc << " that is disabled" if disabled == true
-    desc << describe_all_expression_filters(options)
-    desc
+  describe_expression_filters
+  describe_node_filters do |disabled: nil, **|
+    " that is disabled" if disabled == true
   end
 end
 
@@ -179,7 +186,9 @@
 
   node_filter(:disabled, :boolean, default: false, skip_if: :all) { |node, value| node.tag_name == "a" || !(value ^ node.disabled?) }
 
-  describe { |disabled: nil, **_options| " that is disabled" if disabled == true }
+  describe_node_filters do |disabled: nil, **|
+    " that is disabled" if disabled == true
+  end
 end
 
 Capybara.add_selector(:fillable_field) do
@@ -205,11 +214,9 @@
     with.is_a?(Regexp) ? node.value =~ with : node.value == with.to_s
   end
 
-  describe do |options|
-    desc = +""
-    desc << describe_all_expression_filters(options)
-    desc << " with value #{options[:with].to_s.inspect}" if options.key?(:with)
-    desc
+  describe_expression_filters
+  describe_node_filters do |**options|
+    " with value #{options[:with].to_s.inspect}" if options.key?(:with)
   end
 end
 
@@ -225,11 +232,9 @@
 
   node_filter(:option) { |node, value| node.value == value.to_s }
 
-  describe do |option: nil, **options|
-    desc = +""
-    desc << " with value #{option.inspect}" if option
-    desc << describe_all_expression_filters(options)
-    desc
+  describe_expression_filters
+  describe_node_filters do |option: nil, **|
+    " with value #{option.inspect}" if option
   end
 end
 
@@ -243,11 +248,9 @@
 
   node_filter(:option) { |node, value| node.value == value.to_s }
 
-  describe do |option: nil, **options|
-    desc = +""
-    desc << " with value #{option.inspect}" if option
-    desc << describe_all_expression_filters(options)
-    desc
+  describe_expression_filters
+  describe_node_filters do |option: nil, **|
+    " with value #{option.inspect}" if option
   end
 end
 
@@ -286,13 +289,18 @@
     (Array(selected) - actual).empty?
   end
 
-  describe do |options: nil, with_options: nil, selected: nil, with_selected: nil, **opts|
+  describe do |with_options: nil, **opts|
     desc = +""
-    desc << " with options #{options.inspect}" if options
     desc << " with at least options #{with_options.inspect}" if with_options
+    desc << describe_all_expression_filters(opts)
+    desc
+  end
+
+  describe_node_filters do |options: nil, selected: nil, with_selected: nil, **|
+    desc = +""
+    desc << " with options #{options.inspect}" if options
     desc << " with #{selected.inspect} selected" if selected
     desc << " with at least #{with_selected.inspect} selected" if with_selected
-    desc << describe_all_expression_filters(opts)
     desc
   end
 end
@@ -318,13 +326,16 @@
     end
   end
 
-  describe do |options: nil, with_options: nil, **opts|
+  describe do |with_options: nil, **opts|
     desc = +""
-    desc << " with options #{options.inspect}" if options
     desc << " with at least options #{with_options.inspect}" if with_options
     desc << describe_all_expression_filters(opts)
     desc
   end
+
+  describe_node_filters do |options: nil, **|
+    " with options #{options.inspect}" if options
+  end
 end
 
 Capybara.add_selector(:option) do
@@ -337,7 +348,7 @@
   node_filter(:disabled, :boolean) { |node, value| !(value ^ node.disabled?) }
   node_filter(:selected, :boolean) { |node, value| !(value ^ node.selected?) }
 
-  describe do |**options|
+  describe_node_filters do |**options|
     desc = +""
     desc << " that is#{' not' unless options[:disabled]} disabled" if options.key?(:disabled)
     desc << " that is#{' not' unless options[:selected]} selected" if options.key?(:selected)
@@ -357,10 +368,8 @@
 
   node_filter(:disabled, :boolean) { |node, value| !(value ^ node.disabled?) }
 
-  describe do |**options|
-    desc = +""
-    desc << " that is#{' not' unless options[:disabled]} disabled" if options.key?(:disabled)
-    desc
+  describe_node_filters do |**options|
+    " that is#{' not' unless options[:disabled]} disabled" if options.key?(:disabled)
   end
 end
 
@@ -373,11 +382,7 @@
 
   filter_set(:_field, %i[disabled multiple name])
 
-  describe do |**options|
-    desc = +""
-    desc << describe_all_expression_filters(options)
-    desc
-  end
+  describe_expression_filters
 end
 
 Capybara.add_selector(:label) do
@@ -411,15 +416,13 @@
     end
   end
 
-  describe do |**options|
-    desc = +""
-    desc << " for #{options[:for]}" if options[:for]
-    desc
+  describe_node_filters do |**options|
+    " for #{options[:for]}" if options[:for]
   end
 end
 
 Capybara.add_selector(:table) do
-  xpath(:caption) do |locator, caption: nil, **_options|
+  xpath(:caption) do |locator, caption: nil, **|
     xpath = XPath.descendant(:table)
     unless locator.nil?
       locator_matchers = (XPath.attr(:id) == locator.to_s) | XPath.descendant(:caption).is(locator.to_s)
@@ -430,10 +433,8 @@
     xpath
   end
 
-  describe do |caption: nil, **_options|
-    desc = +""
-    desc << " with caption #{caption}" if caption
-    desc
+  describe do |caption: nil, **|
+    " with caption \"#{caption}\"" if caption
   end
 end
 
@@ -449,15 +450,13 @@
     xpath
   end
 
-  describe do |name: nil, **_options|
-    desc = +""
-    desc << " with name #{name}" if name
-    desc
+  describe do |name: nil, **|
+    " with name #{name}" if name
   end
 end
 
 Capybara.add_selector(:element) do
-  xpath do |locator, **_options|
+  xpath do |locator, **|
     XPath.descendant((locator || '@').to_sym)
   end
 
@@ -476,10 +475,6 @@
     val.is_a?(Regexp) ? node[name] =~ val : true
   end
 
-  describe do |**options|
-    desc = +""
-    desc << describe_all_expression_filters(options)
-    desc
-  end
+  describe_expression_filters
 end
 # rubocop:enable Metrics/BlockLength