code cleanup

.codeclimate.yml

@@ -0,0 +1,33 @@
+---
+engines:
+  bundler-audit:
+    enabled: false
+  csslint:
+    enabled: false
+  duplication:
+    enabled: true
+    config:
+      languages:
+        ruby:
+          mass_threshold: 25
+  eslint:
+    enabled: false
+  fixme:
+    enabled: true
+  rubocop:
+    enabled: true
+ratings:
+  paths:
+  - Gemfile.lock
+  - "**.css"
+  - "**.inc"
+  - "**.js"
+  - "**.jsx"
+  - "**.module"
+  - "**.php"
+  - "**.py"
+  - "**.rb"
+exclude_paths:
+- features/
+- spec/
+- lib/capybara/spec/

lib/capybara.rb

@@ -332,7 +332,7 @@ module Capybara
     #
     def reset_sessions!
       #reset in reverse so sessions that started servers are reset last
-      session_pool.reverse_each { |mode, session| session.reset! }
+      session_pool.reverse_each { |_mode, session| session.reset! }
     end
     alias_method :reset!, :reset_sessions!
 
@@ -470,7 +470,7 @@ module Capybara
   require 'capybara/selenium/driver'
 end
 
-Capybara.register_server :default do |app, port, host|
+Capybara.register_server :default do |app, port, _host|
   Capybara.run_default_server(app, port)
 end
 

lib/capybara/node/document_matchers.rb

@@ -15,13 +15,7 @@ module Capybara
       # @return [true]
       #
       def assert_title(title, options = {})
-        query = Capybara::Queries::TitleQuery.new(title, options)
+        _verify_title(title,options) { |query| raise Capybara::ExpectationNotMet, query.failure_message unless query.resolves_for?(self) }
-        synchronize(query.wait) do
-          unless query.resolves_for?(self)
-            raise Capybara::ExpectationNotMet, query.failure_message
-          end
-        end
-        return true
       end
 
       ##
@@ -32,13 +26,7 @@ module Capybara
       # @return [true]
       #
       def assert_no_title(title, options = {})
-        query = Capybara::Queries::TitleQuery.new(title, options)
+        _verify_title(title,options) { |query| raise Capybara::ExpectationNotMet, query.negative_failure_message if query.resolves_for?(self) }
-        synchronize(query.wait) do
-          if query.resolves_for?(self)
-            raise Capybara::ExpectationNotMet, query.negative_failure_message
-          end
-        end
-        return true
       end
 
       ##
@@ -64,6 +52,17 @@ module Capybara
       rescue Capybara::ExpectationNotMet
         return false
       end
+
+      private
+
+      def _verify_title(title, options)
+        query = Capybara::Queries::TitleQuery.new(title, options)
+        synchronize(query.wait) do
+          yield(query)
+        end
+        return true
+      end
+
     end
   end
 end

lib/capybara/node/matchers.rb

@@ -90,7 +90,7 @@ module Capybara
       # @raise [Capybara::ExpectationNotMet]      If the selector does not exist
       #
       def assert_selector(*args, &optional_filter_block)
-        verify_selector_result(args, optional_filter_block) do |result, query|
+        _verify_selector_result(args, optional_filter_block) do |result, query|
           unless result.matches_count? && ((!result.empty?) || query.expects_none?)
             raise Capybara::ExpectationNotMet, result.failure_message
           end
@@ -114,7 +114,7 @@ module Capybara
       # @raise [Capybara::ExpectationNotMet]      If the selector exists
       #
       def assert_no_selector(*args, &optional_filter_block)
-        verify_selector_result(args, optional_filter_block) do |result, query|
+        _verify_selector_result(args, optional_filter_block) do |result, query|
           if result.matches_count? && ((!result.empty?) || query.expects_none?)
             raise Capybara::ExpectationNotMet, result.negative_failure_message
           end
@@ -449,13 +449,13 @@ module Capybara
       # @raise [Capybara::ExpectationNotMet]      If the selector does not match
       #
       def assert_matches_selector(*args, &optional_filter_block)
-        verify_match_result(args, optional_filter_block) do |result|
+        _verify_match_result(args, optional_filter_block) do |result|
           raise Capybara::ExpectationNotMet, "Item does not match the provided selector" unless result.include? self
         end
       end
 
       def assert_not_matches_selector(*args, &optional_filter_block)
-        verify_match_result(args, optional_filter_block) do |result|
+        _verify_match_result(args, optional_filter_block) do |result|
           raise Capybara::ExpectationNotMet, 'Item matched the provided selector' if result.include? self
         end
       end
@@ -557,7 +557,7 @@ module Capybara
       # @return [true]
       #
       def assert_text(*args)
-        verify_text(args) do |count, query|
+        _verify_text(args) do |count, query|
           unless query.matches_count?(count) && ((count > 0) || query.expects_none?)
             raise Capybara::ExpectationNotMet, query.failure_message
           end
@@ -573,7 +573,7 @@ module Capybara
       # @return [true]
       #
       def assert_no_text(*args)
-        verify_text(args) do |count, query|
+        _verify_text(args) do |count, query|
           if query.matches_count?(count) && ((count > 0) || query.expects_none?)
             raise Capybara::ExpectationNotMet, query.negative_failure_message
           end
@@ -625,7 +625,7 @@ module Capybara
 
     private
 
-      def verify_selector_result(query_args, optional_filter_block, &result_block)
+      def _verify_selector_result(query_args, optional_filter_block, &result_block)
         query = Capybara::Queries::SelectorQuery.new(*query_args, &optional_filter_block)
         synchronize(query.wait) do
           result = query.resolve_for(self)
@@ -634,7 +634,7 @@ module Capybara
         return true
       end
 
-      def verify_match_result(query_args, optional_filter_block, &result_block)
+      def _verify_match_result(query_args, optional_filter_block, &result_block)
         query = Capybara::Queries::MatchQuery.new(*query_args, &optional_filter_block)
         synchronize(query.wait) do
           result = query.resolve_for(self.query_scope)
@@ -643,7 +643,7 @@ module Capybara
         return true
       end
 
-      def verify_text(query_args)
+      def _verify_text(query_args)
         query = Capybara::Queries::TextQuery.new(*query_args)
         synchronize(query.wait) do
           count = query.resolve_for(self)

lib/capybara/rack_test/node.rb

@@ -152,7 +152,7 @@ private
     end
   end
 
-  def set_radio(value)
+  def set_radio(_value)
     other_radios_xpath = XPath.generate { |x| x.anywhere(:input)[x.attr(:name).equals(self[:name])] }.to_s
     driver.dom.xpath(other_radios_xpath).each { |node| node.remove_attribute("checked") }
     native['checked'] = 'checked'

lib/capybara/selector.rb

@@ -87,7 +87,7 @@ Capybara.add_selector(:field) do
   end
   describe do |options|
     desc = String.new
-    (expression_filters - [:type]).each { |ef| desc << " with #{ef.to_s} #{options[ef]}" if options.has_key?(ef) }
+    (expression_filters - [:type]).each { |ef| desc << " with #{ef} #{options[ef]}" if options.has_key?(ef) }
     desc << " of type #{options[:type].inspect}" if options[:type]
     desc << " with value #{options[:with].to_s.inspect}" if options.has_key?(:with)
     desc
@@ -196,7 +196,7 @@ Capybara.add_selector(:button) do
   describe do |options|
     desc = String.new
     desc << " that is disabled" if options[:disabled] == true
-    expression_filters.each { |ef| desc << " with #{ef.to_s} #{options[ef]}" if options.has_key?(ef) }
+    desc << describe_all_expression_filters(options)
     desc
   end
 end
@@ -244,7 +244,7 @@ Capybara.add_selector(:fillable_field) do
 
   describe do |options|
     desc = String.new
-    expression_filters.each { |ef| desc << " with #{ef.to_s} #{options[ef]}" if options.has_key?(ef) }
+    desc << describe_all_expression_filters(options)
     desc << " with value #{options[:with].to_s.inspect}" if options.has_key?(:with)
     desc
   end
@@ -277,7 +277,7 @@ Capybara.add_selector(:radio_button) do
   describe do |options|
     desc = String.new
     desc << " with value #{options[:option].inspect}" if options[:option]
-    expression_filters.each { |ef| desc << " with #{ef.to_s} #{options[ef]}" if options.has_key?(ef) }
+    desc << describe_all_expression_filters(options)
     desc
   end
 end
@@ -308,7 +308,7 @@ Capybara.add_selector(:checkbox) do
   describe do |options|
     desc = String.new
     desc << " with value #{options[:option].inspect}" if options[:option]
-    expression_filters.each { |ef| desc << " with #{ef.to_s} #{options[ef]}" if options.has_key?(ef) }
+    desc << describe_all_expression_filters(options)
     desc
   end
 end
@@ -364,7 +364,7 @@ Capybara.add_selector(:select) do
     desc << " with options #{options[:options].inspect}" if options[:options]
     desc << " with at least options #{options[:with_options].inspect}" if options[:with_options]
     desc << " with #{options[:selected].inspect} selected" if options[:selected]
-    expression_filters.each { |ef| desc << " with #{ef.to_s} #{options[ef]}" if options.has_key?(ef) }
+    desc << describe_all_expression_filters(options)
     desc
   end
 end
@@ -417,7 +417,7 @@ Capybara.add_selector(:file_field) do
 
   describe do |options|
     desc = String.new
-    expression_filters.each { |ef| desc << " with #{ef.to_s} #{options[ef]}" if options.has_key?(ef) }
+    desc << describe_all_expression_filters(options)
     desc
   end
 end

lib/capybara/selector/selector.rb

@@ -229,8 +229,12 @@ module Capybara
       locate_xpath
     end
 
+    def describe_all_expression_filters(opts={})
+      expression_filters.map { |ef| " with #{ef} #{opts[ef]}" if opts.has_key?(ef) }.join
+    end
+
     def find_by_attr(attribute, value)
-      finder_name = "find_by_#{attribute.to_s}_attr"
+      finder_name = "find_by_#{attribute}_attr"
       if respond_to?(finder_name, true)
         send(finder_name, value)
       else

lib/capybara/selenium/driver.rb

@@ -19,7 +19,7 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
         options[:desired_capabilities].merge!({ unexpectedAlertBehaviour: "ignore" })
       end
 
-      @browser = Selenium::WebDriver.for(options[:browser], options.reject { |key,val| SPECIAL_OPTIONS.include?(key) })
+      @browser = Selenium::WebDriver.for(options[:browser], options.reject { |key,_val| SPECIAL_OPTIONS.include?(key) })
 
       main = Process.pid
       at_exit do
@@ -94,7 +94,7 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
     browser.execute_script "return #{script}"
   end
 
-  def save_screenshot(path, options={})
+  def save_screenshot(path, _options={})
     browser.save_screenshot(path)
   end
 
@@ -236,8 +236,7 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
     browser.switch_to.window(handle) { yield }
   end
 
-  def accept_modal(type, options={}, &blk)
+  def accept_modal(_type, options={})
-    options = options.dup
     yield if block_given?
     modal = find_modal(options)
     modal.send_keys options[:with] if options[:with]
@@ -246,7 +245,7 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
     message
   end
 
-  def dismiss_modal(type, options={}, &blk)
+  def dismiss_modal(_type, options={})
     yield if block_given?
     modal = find_modal(options)
     message = modal.text

lib/capybara/session.rb

@@ -733,19 +733,18 @@ module Capybara
 
   private
     def accept_modal(type, text_or_options, options, &blk)
-      text_or_options, options = nil, text_or_options if text_or_options.is_a?(Hash)
+      driver.accept_modal(type, modal_options(text_or_options, options), &blk)
-      options[:text] ||= text_or_options unless text_or_options.nil?
-      options[:wait] ||= Capybara.default_max_wait_time
-
-      driver.accept_modal(type, options, &blk)
     end
 
     def dismiss_modal(type, text_or_options, options, &blk)
+      driver.dismiss_modal(type, modal_options(text_or_options, options), &blk)
+    end
+
+    def modal_options(text_or_options, options)
       text_or_options, options = nil, text_or_options if text_or_options.is_a?(Hash)
       options[:text] ||= text_or_options unless text_or_options.nil?
       options[:wait] ||= Capybara.default_max_wait_time
-
+      options
-      driver.dismiss_modal(type, options, &blk)
     end
 
 

lib/capybara/session/matchers.rb

@@ -17,13 +17,7 @@ module Capybara
     # @return [true]
     #
     def assert_current_path(path, options={})
-      query = Capybara::Queries::CurrentPathQuery.new(path, options)
+      _verify_current_path(path,options) { |query| raise Capybara::ExpectationNotMet, query.failure_message unless query.resolves_for?(self) }
-      document.synchronize(query.wait) do
-        unless query.resolves_for?(self)
-          raise Capybara::ExpectationNotMet, query.failure_message
-        end
-      end
-      return true
     end
 
     ##
@@ -34,13 +28,7 @@ module Capybara
     # @return [true]
     #
     def assert_no_current_path(path, options={})
-      query = Capybara::Queries::CurrentPathQuery.new(path, options)
+      _verify_current_path(path,options) { |query| raise Capybara::ExpectationNotMet, query.negative_failure_message if query.resolves_for?(self) }
-      document.synchronize(query.wait) do
-        if query.resolves_for?(self)
-          raise Capybara::ExpectationNotMet, query.negative_failure_message
-        end
-      end
-      return true
     end
 
     ##
@@ -66,5 +54,15 @@ module Capybara
     rescue Capybara::ExpectationNotMet
       return false
     end
+
+    private
+
+    def _verify_current_path(path, options)
+      query = Capybara::Queries::CurrentPathQuery.new(path, options)
+      document.synchronize(query.wait) do
+        yield(query)
+      end
+      return true
+    end
   end
 end