Add rubocop (#20)

* Add rubocop and rubocop-rspec

* Rubocop automatic fixes

* Set 100 as max line length

* Update rubygems before installing on Travis

* Run rubocop in Travis CI

.rubocop.yml

@@ -0,0 +1,42 @@
+require: rubocop-rspec
+inherit_from: .rubocop_todo.yml
+AllCops:
+  Include:
+    - Rakefile
+    - lib/**/*.rake
+
+Metrics/LineLength:
+  Max: 100
+
+Style/Documentation:
+  Enabled: false
+
+Style/DoubleNegation:
+  Enabled: false
+
+Style/IndentArray:
+  EnforcedStyle: consistent
+
+Style/MultilineMethodCallIndentation:
+  EnforcedStyle: indented
+
+Style/MultilineOperationIndentation:
+  EnforcedStyle: indented
+
+Style/RedundantBegin:
+  Enabled: false
+
+Style/SpaceInsideHashLiteralBraces:
+  EnforcedStyle: no_space
+
+Style/TrailingCommaInLiteral:
+  EnforcedStyleForMultiline: comma
+
+RSpec/MessageSpies:
+  EnforcedStyle: receive
+
+
+
+# Will be able to do this in >= v1.11
+# RSpec/DescribedClass:
+#   EnforcedStyle: explicit

.rubocop_todo.yml

@@ -0,0 +1,98 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2017-02-08 20:09:02 -0800 using RuboCop version 0.47.1.
+# The point is for the user to remove these configuration records
+# one by one as the offenses are removed from the code base.
+# Note that changes in the inspected code, or installation of new
+# versions of RuboCop, may require this file to be generated again.
+
+# Offense count: 8
+Metrics/AbcSize:
+  Max: 54
+
+# Offense count: 27
+# Configuration parameters: CountComments, ExcludedMethods.
+Metrics/BlockLength:
+  Max: 227
+
+# Offense count: 1
+# Configuration parameters: CountComments.
+Metrics/ClassLength:
+  Max: 164
+
+# Offense count: 5
+Metrics/CyclomaticComplexity:
+  Max: 11
+
+# Offense count: 11
+# Configuration parameters: CountComments.
+Metrics/MethodLength:
+  Max: 57
+
+# Offense count: 2
+Metrics/PerceivedComplexity:
+  Max: 12
+
+# Offense count: 43
+# Configuration parameters: SkipBlocks.
+RSpec/DescribedClass:
+  Exclude:
+    - 'spec/lib/percy/capybara/client_spec.rb'
+    - 'spec/lib/percy/capybara/http_fetcher_spec.rb'
+    - 'spec/lib/percy/capybara_spec.rb'
+
+# Offense count: 29
+# Configuration parameters: Max.
+RSpec/ExampleLength:
+  Exclude:
+    - 'spec/lib/percy/capybara/client/builds_spec.rb'
+    - 'spec/lib/percy/capybara/client/snapshots_spec.rb'
+    - 'spec/lib/percy/capybara/client_spec.rb'
+    - 'spec/lib/percy/capybara/loaders/base_loader_spec.rb'
+    - 'spec/lib/percy/capybara/loaders/filesystem_loader_spec.rb'
+    - 'spec/lib/percy/capybara/loaders/native_loader_spec.rb'
+    - 'spec/lib/percy/capybara/loaders/sprockets_loader_spec.rb'
+    - 'spec/lib/percy/capybara_spec.rb'
+
+# Offense count: 1
+# Configuration parameters: AssignmentOnly.
+RSpec/InstanceVariable:
+  Exclude:
+    - 'spec/lib/percy/capybara_spec.rb'
+
+# Offense count: 42
+RSpec/MultipleExpectations:
+  Max: 29
+
+# Offense count: 1
+# Configuration parameters: Max.
+RSpec/NestedGroups:
+  Exclude:
+    - 'spec/lib/percy/capybara/loaders/sprockets_loader_spec.rb'
+
+# Offense count: 10
+# Configuration parameters: IgnoreSymbolicNames.
+RSpec/VerifiedDoubles:
+  Exclude:
+    - 'spec/lib/percy/capybara/client/builds_spec.rb'
+    - 'spec/lib/percy/capybara/client_spec.rb'
+    - 'spec/lib/percy/capybara/loaders/base_loader_spec.rb'
+    - 'spec/lib/percy/capybara/loaders/sprockets_loader_spec.rb'
+    - 'spec/lib/percy/capybara_spec.rb'
+    - 'spec/support/test_helpers.rb'
+
+# Offense count: 4
+# Cop supports --auto-correct.
+# Configuration parameters: EnforcedStyle, SupportedStyles, AllowInnerSlashes.
+# SupportedStyles: slashes, percent_r, mixed
+Style/RegexpLiteral:
+  Exclude:
+    - 'lib/percy/capybara/loaders/native_loader.rb'
+    - 'spec/lib/percy/capybara/loaders/native_loader_spec.rb'
+    - 'spec/lib/percy/capybara/loaders/sprockets_loader_spec.rb'
+    - 'spec/support/test_helpers.rb'
+
+# Offense count: 1
+Style/StructInheritance:
+  Exclude:
+    - 'lib/percy/capybara/httpfetcher.rb'

.travis.yml

@@ -10,5 +10,8 @@ matrix:
   allow_failures:
   - rvm: ruby-head
 before_install:
+  - gem update --system
   - gem update bundler
-script: xvfb-run bundle exec rspec
+script:
+  - xvfb-run bundle exec rspec
+  - bundle exec rubocop -D

Gemfile

@@ -6,4 +6,9 @@ gemspec
 gem 'guard-rspec', require: false
 
 # (for development)
-# gem 'percy-client', path: '~/src/percy-client'
+# gem 'percy-client', path: '~/src/percy-client'
+
+group :test do
+  gem 'rubocop'
+  gem 'rubocop-rspec'
+end

Rakefile

@@ -1,2 +1 @@
 require 'bundler/gem_tasks'
-

lib/percy/capybara.rb

@@ -36,7 +36,7 @@ def self.initialize_build(options = {})
     #
     # @see Percy::Capybara::Client::Builds#finalize_current_build
     def self.finalize_build
-      return if !capybara_client.build_initialized?
+      return unless capybara_client.build_initialized?
       capybara_client.finalize_current_build
     end
 
@@ -45,7 +45,7 @@ def self.reset!
       @capybara_client = nil
     end
     # The 'reset' method is deprecated and will be removed: use the reset! method instead.
-    class << self; alias_method :reset, :reset!; end;
+    class << self; alias reset reset!; end
 
     # Manually disable Percy for the current capybara client. This can also be done with the
     # PERCY_ENABLE=0 environment variable.

lib/percy/capybara/client.rb

@@ -10,7 +10,7 @@ class Client
       include Percy::Capybara::Client::Builds
       include Percy::Capybara::Client::Snapshots
 
-      class Error < Exception; end
+      class Error < RuntimeError; end
       class BuildNotInitializedError < Error; end
       class WebMockBlockingConnectionsError < Error; end
 
@@ -29,26 +29,24 @@ def initialize(options = {})
 
         @loader_options = options[:loader_options] || {}
 
-        if defined?(Rails)
-          @sprockets_environment = options[:sprockets_environment] || Rails.application.assets
-          @sprockets_options = options[:sprockets_options] || Rails.application.config.assets
-        end
+        return unless defined?(Rails)
+
+        @sprockets_environment = options[:sprockets_environment] || Rails.application.assets
+        @sprockets_options = options[:sprockets_options] || Rails.application.config.assets
       end
 
       # Check that environment variables required for Percy::Client are set
       def required_environment_variables_set?
         if !ENV['PERCY_TOKEN'].nil? && ENV['PERCY_PROJECT'].nil?
-          raise RuntimeError.new(
-            '[percy] It looks like you were trying to enable Percy because PERCY_TOKEN is set, ' +
-            'but you are missing the PERCY_PROJECT environment variable!'
-          )
+          raise '[percy] It looks like you were trying to enable Percy because PERCY_TOKEN ' \
+            'is set, but you are missing the PERCY_PROJECT environment variable!'
         end
 
         !(ENV['PERCY_PROJECT'].nil? || ENV['PERCY_TOKEN'].nil?)
       end
 
       def enabled?
-        return @enabled if !@enabled.nil?
+        return @enabled unless @enabled.nil?
 
         # Disable if PERCY_ENABLE is set to 0
         return @enabled = false if ENV['PERCY_ENABLE'] == '0'
@@ -57,21 +55,21 @@ def enabled?
         return @enabled = true if required_environment_variables_set?
 
         # Disable otherwise
-        return @enabled = false
+        @enabled = false
       end
 
       def disable!
         @enabled = false
       end
 
-      def rescue_connection_failures(&block)
-        raise ArgumentError.new('block is required') if !block_given?
+      def rescue_connection_failures
+        raise ArgumentError, 'block is required' unless block_given?
         begin
-          block.call
-        rescue Percy::Client::ServerError,  # Rescue server errors.
-            Percy::Client::PaymentRequiredError,  # Rescue quota exceeded errors.
-            Percy::Client::ConnectionFailed,  # Rescue some networking errors.
-            Percy::Client::TimeoutError => e
+          yield
+        rescue Percy::Client::ServerError, # Rescue server errors.
+               Percy::Client::PaymentRequiredError, # Rescue quota exceeded errors.
+               Percy::Client::ConnectionFailed, # Rescue some networking errors.
+               Percy::Client::TimeoutError => e
           Percy.logger.error(e)
           @enabled = false
           @failed = true
@@ -80,7 +78,7 @@ def rescue_connection_failures(&block)
       end
 
       def failed?
-        return !!@failed
+        !!@failed
       end
 
       def initialize_loader(options = {})
@@ -105,9 +103,9 @@ def initialize_loader(options = {})
         else
           unless @warned_about_native_loader
             Percy.logger.warn \
-              '[DEPRECATED] The native_loader is deprecated and will be opt-in in a future release. ' +
-              'You should move to the faster, more reliable filesystem_loader. See the docs for ' +
-              'Non-Rails frameworks: https://percy.io/docs/clients/ruby/capybara '
+              '[DEPRECATED] The native_loader is deprecated and will be opt-in in a future ' \
+              'release. You should move to the faster, more reliable filesystem_loader. See the ' \
+              'docs for Non-Rails frameworks: https://percy.io/docs/clients/ruby/capybara '
             @warned_about_native_loader = true
           end
           Percy.logger.debug { 'Using native_loader to discover assets (slower).' }

lib/percy/capybara/client/builds.rb

@@ -3,31 +3,31 @@ module Capybara
     class Client
       module Builds
         def initialize_build(options = {})
-          return if !enabled?  # Silently skip if the client is disabled.
+          return unless enabled? # Silently skip if the client is disabled.
           return @current_build if build_initialized?
 
           # Gather build resources to upload with build.
           start = Time.now
           build_resources = options[:build_resources] || initialize_loader.build_resources
-          options[:resources] = build_resources if !build_resources.empty?
+          options[:resources] = build_resources unless build_resources.empty?
 
           # Extra debug info.
           build_resources.each { |br| Percy.logger.debug { "Build resource: #{br.resource_url}" } }
           Percy.logger.debug { "All build resources loaded (#{Time.now - start}s)" }
 
           rescue_connection_failures do
             @current_build = client.create_build(client.config.repo, options)
-            _upload_missing_build_resources(build_resources) if !build_resources.empty?
+            _upload_missing_build_resources(build_resources) unless build_resources.empty?
           end
           if failed?
-            Percy.logger.error { "Percy build failed! Check log above for errors." }
+            Percy.logger.error { 'Percy build failed! Check log above for errors.' }
             return
           end
           @current_build
         end
 
         def current_build
-          return if !enabled?  # Silently skip if the client is disabled.
+          return unless enabled?  # Silently skip if the client is disabled.
           @current_build
         end
 
@@ -37,16 +37,16 @@ def build_initialized?
         end
 
         def finalize_current_build
-          return if !enabled?  # Silently skip if the client is disabled.
-          if !build_initialized?
-            raise Percy::Capybara::Client::BuildNotInitializedError.new(
-              'Failed to finalize build because no build has been initialized.')
+          return unless enabled?  # Silently skip if the client is disabled.
+          unless build_initialized?
+            raise Percy::Capybara::Client::BuildNotInitializedError,
+                  'Failed to finalize build because no build has been initialized.'
           end
           result = rescue_connection_failures do
             client.finalize_build(current_build['data']['id'])
           end
           if failed?
-            Percy.logger.error { "Percy build failed! Check log above for errors." }
+            Percy.logger.error { 'Percy build failed! Check log above for errors.' }
             return
           end
           result
@@ -59,15 +59,15 @@ def _upload_missing_build_resources(build_resources)
             current_build['data']['relationships'] &&
             current_build['data']['relationships']['missing-resources'] &&
             current_build['data']['relationships']['missing-resources']['data']
-          return 0 if !new_build_resources
+          return 0 unless new_build_resources
 
           new_build_resources.each_with_index do |missing_resource, i|
             sha = missing_resource['id']
             resource = build_resources.find { |r| r.sha == sha }
             content = resource.content || File.read(resource.path)
             client.upload_resource(current_build['data']['id'], content)
-            if i % 10 == 0
-              puts "[percy] Uploading #{i+1} of #{new_build_resources.length} new resources..."
+            if (i % 10).zero?
+              puts "[percy] Uploading #{i + 1} of #{new_build_resources.length} new resources..."
             end
           end
           new_build_resources.length