Merge pull request #373 from sul-dlss/rubocop_ci

Integrate Rubocop in CI
sul-dlss · Sep 28, 2018 · e9c8697 · e9c8697
2 parents ae5cddd + 1614c8f
commit e9c8697
Show file tree

Hide file tree

Showing 38 changed files with 338 additions and 423 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -26,13 +26,23 @@ Layout/SpaceBeforeBlockBraces:
 # Configuration parameters: AllowURI, URISchemes.
 Metrics/LineLength:
   Max: 200
+  Exclude:
+    - 'config/initializers/**/*.rb' # generated files
 
 RSpec/ContextWording:
   Enabled: false # too dogmatic
 
+# Offense count: 2
+RSpec/DescribeClass:
+  Exclude:
+    - 'spec/views/**/*.rb' # In view tests, the view is named, not the class
+
 RSpec/ExampleLength:
   Max: 25
 
+RSpec/ImplicitSubject: # we use this for `define_enum_for`, `validate_presence_of`, etc.
+  Enabled: false
+
 # we like 'expect(x).to receive' better than 'have_received'
 RSpec/MessageSpies:
   Enabled: false

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -1,25 +1,25 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2018-09-26 16:12:20 -0700 using RuboCop version 0.59.0.
+# on 2018-09-27 15:46:41 -0700 using RuboCop version 0.59.0.
 # 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: 9
+# Offense count: 10
 Metrics/AbcSize:
   Max: 74
 
-# Offense count: 27
+# Offense count: 28
 # Configuration parameters: CountComments, ExcludedMethods.
 # ExcludedMethods: refine
 Metrics/BlockLength:
-  Max: 309
+  Max: 298
 
 # Offense count: 2
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 190
+  Max: 187
 
 # Offense count: 4
 Metrics/CyclomaticComplexity:
@@ -29,9 +29,9 @@ Metrics/CyclomaticComplexity:
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https
 Metrics/LineLength:
-  Max: 281
+  Max: 251
 
-# Offense count: 10
+# Offense count: 9
 # Configuration parameters: CountComments.
 Metrics/MethodLength:
   Max: 44
@@ -62,145 +62,36 @@ Naming/UncommunicativeMethodParamName:
   Exclude:
     - 'spec/support/bundle_setup.rb'
 
-# Offense count: 2
-RSpec/DescribeClass:
-  Exclude:
-    - 'spec/views/bundle_context_spec.rb'
-    - 'spec/views/job_runs/index.json.jbuilder_spec.rb'
-
 # Offense count: 1
-# Cop supports --auto-correct.
-# Configuration parameters: SkipBlocks, EnforcedStyle.
-# SupportedStyles: described_class, explicit
-RSpec/DescribedClass:
-  Exclude:
-    - 'spec/lib/pre_assembly/bundle_spec.rb'
-
-# Offense count: 5
-# Cop supports --auto-correct.
-RSpec/EmptyLineAfterFinalLet:
-  Exclude:
-    - 'spec/controllers/bundle_contexts_controller_spec.rb'
-    - 'spec/lib/pre_assembly/bundle_spec.rb'
-    - 'spec/models/bundle_context_spec.rb'
-    - 'spec/models/job_run_spec.rb'
-    - 'spec/services/discovery_report_spec.rb'
-
-# Offense count: 5
-# Cop supports --auto-correct.
-RSpec/EmptyLineAfterHook:
+RSpec/BeforeAfterAll:
   Exclude:
-    - 'spec/jobs/discovery_report_job_spec.rb'
-    - 'spec/jobs/preassembly_job_spec.rb'
-    - 'spec/lib/pre_assembly/bundle_spec.rb'
-    - 'spec/models/bundle_context_spec.rb'
-    - 'spec/models/job_run_spec.rb'
+    - 'spec/spec_helper.rb'
+    - 'spec/rails_helper.rb'
+    - 'spec/support/**/*.rb'
+    - 'spec/lib/pre_assembly/digital_object_spec.rb'
 
 # Offense count: 2
 # Configuration parameters: Max.
 RSpec/ExampleLength:
   Exclude:
     - 'spec/lib/pre_assembly/bundle_spec.rb'
 
-# Offense count: 1
-# Cop supports --auto-correct.
-# Configuration parameters: CustomTransform, IgnoredWords.
-RSpec/ExampleWording:
-  Exclude:
-    - 'spec/controllers/bundle_contexts_controller_spec.rb'
-
-# Offense count: 3
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: single_line_only, disallow
-RSpec/ImplicitSubject:
-  Exclude:
-    - 'spec/models/bundle_context_spec.rb'
-    - 'spec/models/job_run_spec.rb'
-
-# Offense count: 1
-# Configuration parameters: AssignmentOnly.
-RSpec/InstanceVariable:
-  Exclude:
-    - 'spec/lib/pre_assembly/object_file_spec.rb'
-
-# Offense count: 2
-# Cop supports --auto-correct.
-RSpec/LeadingSubject:
-  Exclude:
-    - 'spec/models/bundle_context_spec.rb'
-    - 'spec/services/discovery_report_spec.rb'
-
-# Offense count: 1
-RSpec/LetSetup:
-  Exclude:
-    - 'spec/views/job_runs/index.json.jbuilder_spec.rb'
-
 # Offense count: 1
 # Configuration parameters: AggregateFailuresByDefault.
 RSpec/MultipleExpectations:
   Max: 7
 
-# Offense count: 1
-RSpec/NamedSubject:
-  Exclude:
-    - 'spec/controllers/bundle_contexts_controller_spec.rb'
-
 # Offense count: 10
 RSpec/SubjectStub:
   Exclude:
     - 'spec/models/bundle_context_spec.rb'
     - 'spec/services/discovery_report_spec.rb'
 
-# Offense count: 4
-# Configuration parameters: IgnoreSymbolicNames.
-RSpec/VerifiedDoubles:
-  Exclude:
-    - 'spec/lib/pre_assembly/bundle_spec.rb'
-    - 'spec/models/job_run_spec.rb'
-
-# Offense count: 1
-# Cop supports --auto-correct.
-# Configuration parameters: AutoCorrect, EnforcedStyle.
-# SupportedStyles: nested, compact
-Style/ClassAndModuleChildren:
-  Exclude:
-    - 'app/lib/pre_assembly/object_file.rb'
-
 # Offense count: 1
 Style/ClassVars:
   Exclude:
     - 'app/lib/pre_assembly/logging.rb'
 
-# Offense count: 8
-Style/CommentedKeyword:
-  Exclude:
-    - 'app/lib/pre_assembly/smpl.rb'
-    - 'spec/lib/pre_assembly/smpl_spec.rb'
-
 # Offense count: 19
 Style/Documentation:
   Enabled: false
-
-# Offense count: 140
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, UseHashRocketsWithSymbolValues, PreferHashRocketsForNonAlnumEndingSymbols.
-# SupportedStyles: ruby19, hash_rockets, no_mixed_keys, ruby19_no_mixed_keys
-Style/HashSyntax:
-  Exclude:
-    - 'app/lib/csv_importer.rb'
-    - 'app/lib/pre_assembly/bundle.rb'
-    - 'app/lib/pre_assembly/digital_object.rb'
-    - 'app/lib/pre_assembly/logging.rb'
-    - 'app/lib/pre_assembly/smpl.rb'
-    - 'spec/lib/pre_assembly/bundle_spec.rb'
-    - 'spec/lib/pre_assembly/digital_object_spec.rb'
-    - 'spec/lib/pre_assembly/object_file_spec.rb'
-    - 'spec/lib/pre_assembly/smpl_spec.rb'
-
-# Offense count: 210
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline.
-# SupportedStyles: single_quotes, double_quotes
-Style/StringLiterals:
-  Enabled: false
diff --git a/Rakefile b/Rakefile
@@ -4,3 +4,8 @@
 require_relative 'config/application'
 
 Rails.application.load_tasks
+
+require 'rubocop/rake_task'
+RuboCop::RakeTask.new
+
+task default: [:spec, :rubocop]
diff --git a/app/controllers/job_runs_controller.rb b/app/controllers/job_runs_controller.rb
@@ -1,6 +1,6 @@
 class JobRunsController < ApplicationController
   def index
-    flash[:notice] = "Success! Your job is queued. A link to your validation report will be emailed to you when it is ready." if params[:created]
+    flash[:notice] = 'Success! Your job is queued. A link to your validation report will be emailed to you when it is ready.' if params[:created]
     @job_runs = JobRun.order('created_at desc').page params[:page]
   end
 

diff --git a/app/lib/csv_importer.rb b/app/lib/csv_importer.rb
@@ -4,10 +4,10 @@ class CsvImporter
   # @return [Array<ActiveSupport::HashWithIndifferentAccess>]
   # @raise if file missing/unreadable
   def self.parse_to_hash(filename)
-    raise ArgumentError, "CSV filename required" unless filename.present?
+    raise ArgumentError, 'CSV filename required' unless filename.present?
     raise ArgumentError, "Required file not found: #{filename}." unless File.readable?(filename)
-    file_contents = IO.read(filename).encode("utf-8", replace: nil)
-    csv = CSV.parse(file_contents, :headers => true)
+    file_contents = IO.read(filename).encode('utf-8', replace: nil)
+    csv = CSV.parse(file_contents, headers: true)
     csv.map { |row| row.to_hash.with_indifferent_access }
   end
 end
diff --git a/app/lib/pre_assembly/bundle.rb b/app/lib/pre_assembly/bundle.rb
@@ -28,7 +28,7 @@ class Bundle
 
     def initialize(bundle_context)
       @bundle_context = bundle_context
-      self.smpl_manifest = PreAssembly::Smpl.new(:csv_filename => bundle_context.smpl_manifest, :bundle_dir => bundle_dir) if bundle_context.smpl_cm_style?
+      self.smpl_manifest = PreAssembly::Smpl.new(csv_filename: bundle_context.smpl_manifest, bundle_dir: bundle_dir) if bundle_context.smpl_cm_style?
       self.skippables = {}
       load_skippables
     end
@@ -55,11 +55,11 @@ def run_pre_assembly
 
     def run_log_msg
       log_params = {
-        :content_structure => content_structure,
-        :project_name => project_name,
-        :bundle_dir => bundle_dir,
-        :assembly_staging_dir => Settings.assembly_staging_dir,
-        :environment => ENV['RAILS_ENV']
+        content_structure: content_structure,
+        project_name: project_name,
+        bundle_dir: bundle_dir,
+        assembly_staging_dir: Settings.assembly_staging_dir,
+        environment: ENV['RAILS_ENV']
       }
       log_params.map { |k, v| "#{k}=#{v.inspect}" }.join(', ')
     end
@@ -82,14 +82,14 @@ def object_filenames_unique?(dobj)
     def digital_objects
       @digital_objects ||= discover_containers_via_manifest.each_with_index.map do |c, i|
         params = {
-          :container       => c,
-          :stageable_items => discover_items_via_crawl(c)
+          container: c,
+          stageable_items: discover_items_via_crawl(c)
         }
         params[:object_files] = discover_object_files(params[:stageable_items])
         DigitalObject.new(self, params).tap do |dobj|
           r = manifest_rows[i]
           # Get label and source_id from column names declared in YAML config.
-          dobj.label        = manifest_cols[:label] ? r[manifest_cols[:label]] : ""
+          dobj.label        = manifest_cols[:label] ? r[manifest_cols[:label]] : ''
           dobj.source_id    = r[manifest_cols[:source_id]] if manifest_cols[:source_id]
           # Also store a hash of all values from the manifest row, using column names as keys.
           dobj.manifest_row = r
@@ -146,9 +146,9 @@ def all_object_files
 
     def new_object_file(stageable, file_path)
       ObjectFile.new(
-        :path                 => file_path,
-        :relative_path        => relative_path(get_base_dir(stageable), file_path),
-        :exclude_from_content => exclude_from_content(file_path)
+        path: file_path,
+        relative_path: relative_path(get_base_dir(stageable), file_path),
+        exclude_from_content: exclude_from_content(file_path)
       )
     end
 
@@ -162,7 +162,7 @@ def exclude_from_content(file_path)
     # Takes a DigitalObject. For each of its ObjectFiles,
     # sets the checksum attribute.
     def load_checksums(dobj)
-      log "  - load_checksums()"
+      log '  - load_checksums()'
       dobj.object_files.each { |file| file.checksum = file.md5 }
     end
 
@@ -220,10 +220,10 @@ def objects_to_process
 
     def log_progress_info(dobj)
       {
-        :container          => dobj.container,
-        :pid                => dobj.pid,
-        :pre_assem_finished => dobj.pre_assem_finished,
-        :timestamp          => Time.now.strftime('%Y-%m-%d %H:%I:%S')
+        container: dobj.container,
+        pid: dobj.pid,
+        pre_assem_finished: dobj.pre_assem_finished,
+        timestamp: Time.now.strftime('%Y-%m-%d %H:%I:%S')
       }
     end