From addac73a5890bceb74d4731ff9c121c448c2325b Mon Sep 17 00:00:00 2001 From: Joe Atzberger Date: Thu, 27 Sep 2018 15:31:40 -0700 Subject: [PATCH 1/8] Integrate rubocop in CI --- Rakefile | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Rakefile b/Rakefile index e85f9139..262be8df 100644 --- 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] From 63b5ccb4eeae62fab7d715aa9cbfcfa477cec51f Mon Sep 17 00:00:00 2001 From: Joe Atzberger Date: Thu, 27 Sep 2018 15:42:06 -0700 Subject: [PATCH 2/8] All the auto-corrects including remaining Style and RSpec --- .rubocop.yml | 3 + .rubocop_todo.yml | 73 --------- app/controllers/job_runs_controller.rb | 2 +- app/lib/csv_importer.rb | 6 +- app/lib/pre_assembly/bundle.rb | 34 ++--- app/lib/pre_assembly/digital_object.rb | 20 +-- app/lib/pre_assembly/logging.rb | 12 +- app/lib/pre_assembly/smpl.rb | 38 ++--- app/models/bundle_context.rb | 18 +-- app/models/job_run.rb | 4 +- app/services/discovery_report.rb | 2 +- config/application.rb | 2 +- config/deploy/old_prod.rb | 2 +- config/deploy/old_stage.rb | 2 +- config/puma.rb | 6 +- config/routes.rb | 4 +- lib/tasks/resque.rake | 2 +- .../bundle_contexts_controller_spec.rb | 25 ++-- spec/jobs/discovery_report_job_spec.rb | 1 + spec/jobs/preassembly_job_spec.rb | 1 + spec/lib/csv_importer_spec.rb | 2 +- spec/lib/pre_assembly/bundle_spec.rb | 140 +++++++++--------- spec/lib/pre_assembly/digital_object_spec.rb | 47 +++--- spec/lib/pre_assembly/object_file_spec.rb | 6 +- spec/lib/pre_assembly/smpl_spec.rb | 24 +-- spec/mailers/job_mailer_spec.rb | 6 +- spec/models/bundle_context_spec.rb | 66 +++++---- spec/models/job_run_spec.rb | 2 + spec/models/user_spec.rb | 6 +- spec/rails_helper.rb | 2 +- spec/services/discovery_report_spec.rb | 21 +-- spec/spec_helper.rb | 4 +- spec/support/bundle_setup.rb | 10 +- spec/views/bundle_context_spec.rb | 4 +- 34 files changed, 270 insertions(+), 327 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index c391c860..07bc626b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -33,6 +33,9 @@ RSpec/ContextWording: 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 index 2b0610b8..a381376f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -68,47 +68,12 @@ RSpec/DescribeClass: - '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: - 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' - # 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. @@ -124,13 +89,6 @@ 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: @@ -159,14 +117,6 @@ RSpec/VerifiedDoubles: - '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: @@ -181,26 +131,3 @@ Style/CommentedKeyword: # 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/app/controllers/job_runs_controller.rb b/app/controllers/job_runs_controller.rb index 7d7b5ecf..03552c1e 100644 --- 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 index 3b81ebb9..43b0c36c 100644 --- a/app/lib/csv_importer.rb +++ b/app/lib/csv_importer.rb @@ -4,10 +4,10 @@ class CsvImporter # @return [Array] # @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 index f4a82a4f..20d24446 100644 --- 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 diff --git a/app/lib/pre_assembly/digital_object.rb b/app/lib/pre_assembly/digital_object.rb index a9b58648..4aaa3f88 100644 --- a/app/lib/pre_assembly/digital_object.rb +++ b/app/lib/pre_assembly/digital_object.rb @@ -25,7 +25,7 @@ class DigitalObject attr_writer :dor_object, :druid_tree_dir - INIT_PARAMS = [:container, :stageable_items, :object_files] + INIT_PARAMS = [:container, :stageable_items, :object_files].freeze # @param [PreAssembly::Bundle] bundle # @param [Hash Object>] params @@ -45,7 +45,7 @@ def setup def stager(source, destination) if staging_style_symlink - FileUtils.ln_s source, destination, :force => true + FileUtils.ln_s source, destination, force: true else FileUtils.cp_r source, destination end @@ -112,7 +112,7 @@ def pid end def query_dor_by_barcode(barcode) - Dor::SearchService.query_by_id :barcode => barcode + Dor::SearchService.query_by_id barcode: barcode end def get_dor_item_apos(_pid) @@ -126,7 +126,7 @@ def dor_object end def content_type_tag - dor_object.nil? ? "" : dor_object.content_type_tag + dor_object.nil? ? '' : dor_object.content_type_tag end def container_basename @@ -171,15 +171,15 @@ def create_technical_metadata return unless content_md_creation == 'smpl_cm_style' tm = Nokogiri::XML::Document.new - tm_node = Nokogiri::XML::Node.new("technicalMetadata", tm) + tm_node = Nokogiri::XML::Node.new('technicalMetadata', tm) tm_node['objectId'] = pid - tm_node['datetime'] = Time.now.utc.strftime("%Y-%m-%d-T%H:%M:%SZ") + tm_node['datetime'] = Time.now.utc.strftime('%Y-%m-%d-T%H:%M:%SZ') tm << tm_node # find all technical metadata files and just append the xml to the combined technicalMetadata current_directory = Dir.pwd FileUtils.cd(File.join(bundle_dir, container_basename)) - Dir.glob("**/*_techmd.xml").sort.each do |filename| + Dir.glob('**/*_techmd.xml').sort.each do |filename| tech_md_xml = Nokogiri::XML(File.open(File.join(bundle_dir, container_basename, filename))) tm.root << tech_md_xml.root end @@ -205,11 +205,11 @@ def generate_content_metadata # Invoke the contentMetadata creation method used by the project # if we are not using a standard known style of content metadata generation, pass the task off to a custom method def create_content_metadata - if content_md_creation == "smpl_cm_style" + if content_md_creation == 'smpl_cm_style' self.content_md_xml = smpl_manifest.generate_cm(druid.id) else # otherwise use the content metadata generation gem - params = { :druid => druid.id, :objects => content_object_files, :add_exif => false, :bundle => content_md_creation.to_sym, :style => content_md_creation_style } + params = { druid: druid.id, objects: content_object_files, add_exif: false, bundle: content_md_creation.to_sym, style: content_md_creation_style } self.content_md_xml = Assembly::ContentMetadata.create_content_metadata(params) end end @@ -279,7 +279,7 @@ def content_md_file 'contentMetadata.xml' end - def retry_handler(method_name, logger, params = {}) + def retry_handler(method_name, _logger, params = {}) proc do |_exception, attempt_number, total_delay| log(" ** #{method_name} FAILED **; with params of #{params.inspect}; and trying attempt #{attempt_number} of #{Dor::Config.dor_services.num_attempts}; delayed #{total_delay} seconds") end diff --git a/app/lib/pre_assembly/logging.rb b/app/lib/pre_assembly/logging.rb index 5710de21..f0a3101d 100644 --- a/app/lib/pre_assembly/logging.rb +++ b/app/lib/pre_assembly/logging.rb @@ -1,15 +1,15 @@ module PreAssembly module Logging LEVELS = { - :fatal => Logger::FATAL, - :error => Logger::ERROR, - :warn => Logger::WARN, - :info => Logger::INFO, - :debug => Logger::DEBUG + fatal: Logger::FATAL, + error: Logger::ERROR, + warn: Logger::WARN, + info: Logger::INFO, + debug: Logger::DEBUG }.freeze LOG_FORMAT = "%-6s -- %s -- %s\n".freeze - TIME_FORMAT = "%Y-%m-%d %H:%M:%S".freeze + TIME_FORMAT = '%Y-%m-%d %H:%M:%S'.freeze @@log ||= Logger.new(File.join(Rails.root, 'log', "#{Rails.env}.log")) @@log.level = LEVELS[:info] diff --git a/app/lib/pre_assembly/smpl.rb b/app/lib/pre_assembly/smpl.rb index 2bb4fca7..e936140a 100644 --- a/app/lib/pre_assembly/smpl.rb +++ b/app/lib/pre_assembly/smpl.rb @@ -23,12 +23,12 @@ def initialize(params) # default publish/shelve/preserve attributes per "type" as defined in smpl filenames @file_attributes = { - 'default' => { :publish => 'no', :shelve => 'no', :preserve => 'yes' }, - 'pm' => { :publish => 'no', :shelve => 'no', :preserve => 'yes' }, - 'sh' => { :publish => 'no', :shelve => 'no', :preserve => 'yes' }, - 'sl' => { :publish => 'yes', :shelve => 'yes', :preserve => 'yes' }, - 'images' => { :publish => 'yes', :shelve => 'yes', :preserve => 'yes' }, - 'transcript' => { :publish => 'yes', :shelve => 'yes', :preserve => 'yes' } + 'default' => { publish: 'no', shelve: 'no', preserve: 'yes' }, + 'pm' => { publish: 'no', shelve: 'no', preserve: 'yes' }, + 'sh' => { publish: 'no', shelve: 'no', preserve: 'yes' }, + 'sl' => { publish: 'yes', shelve: 'yes', preserve: 'yes' }, + 'images' => { publish: 'yes', shelve: 'yes', preserve: 'yes' }, + 'transcript' => { publish: 'yes', shelve: 'yes', preserve: 'yes' } } @manifest = {} # read CSV @@ -53,9 +53,9 @@ def load_manifest shelve = defined?(row[:shelve]) ? row[:shelve] || nil : nil preserve = defined?(row[:preserve]) ? row[:preserve] || nil : nil - manifest[druid] = { :source_id => '', :files => [] } if manifest[druid].nil? + manifest[druid] = { source_id: '', files: [] } if manifest[druid].nil? manifest[druid][:source_id] = row[:source_id] if defined?(row[:source_id]) && row[:source_id] - manifest[druid][:files] << { :thumb => thumb, :publish => publish, :shelve => shelve, :preserve => preserve, :resource_type => resource_type, :role => role, :file_extention => file_extension, :filename => row[:filename], :label => row[:label], :sequence => row[:sequence] } + manifest[druid][:files] << { thumb: thumb, publish: publish, shelve: shelve, preserve: preserve, resource_type: resource_type, role: role, file_extention: file_extension, filename: row[:filename], label: row[:label], sequence: row[:sequence] } end # loop over all rows end # load_manifest @@ -74,7 +74,7 @@ def generate_cm(druid) label = file[:label] || '' resource_type = file[:resource_type] || 'media' if !seq.nil? && seq != '' && seq != current_seq # this is a new resource if we have a non-blank different sequence number - resources[seq.to_i] = { :label => label, :sequence => seq, :resource_type => resource_type, :files => [] } + resources[seq.to_i] = { label: label, sequence: seq, resource_type: resource_type, files: [] } current_seq = seq end resources[current_seq.to_i][:files] << file @@ -84,29 +84,29 @@ def generate_cm(druid) # generate the base of the XML file for this new druid # generate content metadata builder = Nokogiri::XML::Builder.new do |xml| - xml.contentMetadata(:objectId => druid, :type => 'media') do + xml.contentMetadata(objectId: druid, type: 'media') do resources.keys.sort.each do |seq| resource = resources[seq] - resource_attributes = { :sequence => seq.to_s, :id => "#{druid}_#{seq}", :type => resource[:resource_type] } + resource_attributes = { sequence: seq.to_s, id: "#{druid}_#{seq}", type: resource[:resource_type] } resource_attributes[:thumb] = 'yes' if resource[:thumb] # add the thumb=yes attribute to the resource if it was marked that way in the manifest xml.resource(resource_attributes) do xml.label resource[:label] resource[:files].each do |file| - filename = file[:filename] || "" + filename = file[:filename] || '' attrs = file_attributes[file[:role].downcase] || file_attributes['default'] - publish = file[:publish] || attrs[:publish] || "true" - preserve = file[:preserve] || attrs[:preserve] || "true" - shelve = file[:shelve] || attrs[:shelve] || "true" + publish = file[:publish] || attrs[:publish] || 'true' + preserve = file[:preserve] || attrs[:preserve] || 'true' + shelve = file[:shelve] || attrs[:shelve] || 'true' # look for a checksum file named the same as this file checksum = nil FileUtils.cd(File.join(bundle_dir, druid)) - md_files = Dir.glob("**/" + filename + ".md5") + md_files = Dir.glob('**/' + filename + '.md5') checksum = get_checksum(File.join(bundle_dir, druid, md_files[0])) if md_files.size == 1 # we found a corresponding md5 file, read it - xml.file(:id => filename, :preserve => preserve, :publish => publish, :shelve => shelve) do - xml.checksum(checksum, :type => 'md5') if checksum && checksum != '' + xml.file(id: filename, preserve: preserve, publish: publish, shelve: shelve) do + xml.checksum(checksum, type: 'md5') if checksum && checksum != '' end end end # end resource @@ -120,7 +120,7 @@ def generate_cm(druid) def get_checksum(md5_file) s = IO.read(md5_file) checksums = s.scan(/[0-9a-fA-F]{32}/) - checksums.first ? checksums.first.strip : "" + checksums.first ? checksums.first.strip : '' end def get_role(filename) diff --git a/app/models/bundle_context.rb b/app/models/bundle_context.rb index b85bad39..b3a4dd58 100644 --- a/app/models/bundle_context.rb +++ b/app/models/bundle_context.rb @@ -5,7 +5,7 @@ class BundleContext < ApplicationRecord validates :bundle_dir, :content_metadata_creation, :content_structure, presence: true validates :project_name, presence: true, format: { with: /\A[\w-]+\z/, - message: "only allows A-Z, a-z, 0-9, hyphen and underscore" } + message: 'only allows A-Z, a-z, 0-9, hyphen and underscore' } validates :staging_style_symlink, inclusion: { in: [true, false] } validate :verify_bundle_directory @@ -14,17 +14,17 @@ class BundleContext < ApplicationRecord after_initialize :normalize_bundle_dir, :default_enums enum content_structure: { - "simple_image" => 0, - "simple_book" => 1, - "book_as_image" => 2, - "file" => 3, - "smpl" => 4 + 'simple_image' => 0, + 'simple_book' => 1, + 'book_as_image' => 2, + 'file' => 3, + 'smpl' => 4 } enum content_metadata_creation: { - "default" => 0, - "filename" => 1, - "smpl_cm_style" => 2 + 'default' => 0, + 'filename' => 1, + 'smpl_cm_style' => 2 } accepts_nested_attributes_for :job_runs diff --git a/app/models/job_run.rb b/app/models/job_run.rb index 7ea2109e..c756442d 100644 --- a/app/models/job_run.rb +++ b/app/models/job_run.rb @@ -5,8 +5,8 @@ class JobRun < ApplicationRecord after_update :send_notification, if: -> { saved_change_to_output_location? } enum job_type: { - "discovery_report" => 0, - "preassembly" => 1 + 'discovery_report' => 0, + 'preassembly' => 1 } # throw to asynchronous processing via correct Job class for job_type diff --git a/app/services/discovery_report.rb b/app/services/discovery_report.rb index 3b629c8c..2289cefd 100644 --- a/app/services/discovery_report.rb +++ b/app/services/discovery_report.rb @@ -91,7 +91,7 @@ def skipped_files # @return [Boolean] def using_smpl_manifest? - content_md_creation == "smpl_cm_style" && File.exist?(File.join(bundle_dir, bundle.bundle_context.smpl_manifest)) + content_md_creation == 'smpl_cm_style' && File.exist?(File.join(bundle_dir, bundle.bundle_context.smpl_manifest)) end # @return [PreAssembly::Smpl] diff --git a/config/application.rb b/config/application.rb index d1340a97..aedc7fa3 100644 --- a/config/application.rb +++ b/config/application.rb @@ -2,7 +2,7 @@ require 'rails/all' require_relative 'erubis_monkeypatch.rb' -CERT_DIR = File.join(File.dirname(__FILE__), ".", "certs") +CERT_DIR = File.join(File.dirname(__FILE__), '.', 'certs') # Require the gems listed in Gemfile, including any gems # you've limited to :test, :development, or :production. diff --git a/config/deploy/old_prod.rb b/config/deploy/old_prod.rb index 7add6fc9..21409341 100644 --- a/config/deploy/old_prod.rb +++ b/config/deploy/old_prod.rb @@ -2,7 +2,7 @@ set :repo_url, 'https://github.com/sul-dlss/pre-assembly.git' Capistrano::OneTimeKey.generate_one_time_key! -set :rails_env, "production" +set :rails_env, 'production' set :branch, 'v3-legacy' set :honeybadger_env, 'lyberservices-prod' diff --git a/config/deploy/old_stage.rb b/config/deploy/old_stage.rb index 184ac6b7..0e029c66 100644 --- a/config/deploy/old_stage.rb +++ b/config/deploy/old_stage.rb @@ -2,7 +2,7 @@ set :repo_url, 'https://github.com/sul-dlss/pre-assembly.git' Capistrano::OneTimeKey.generate_one_time_key! -set :rails_env, "test" +set :rails_env, 'test' set :deploy_to, '/home/lyberadmin/pre-assembly' set :branch, 'v3-legacy' diff --git a/config/puma.rb b/config/puma.rb index a5eccf81..f1230396 100644 --- a/config/puma.rb +++ b/config/puma.rb @@ -4,16 +4,16 @@ # the maximum value specified for Puma. Default is set to 5 threads for minimum # and maximum; this matches the default thread size of Active Record. # -threads_count = ENV.fetch("RAILS_MAX_THREADS") { 5 } +threads_count = ENV.fetch('RAILS_MAX_THREADS') { 5 } threads threads_count, threads_count # Specifies the `port` that Puma will listen on to receive requests; default is 3000. # -port ENV.fetch("PORT") { 3000 } +port ENV.fetch('PORT') { 3000 } # Specifies the `environment` that Puma will run in. # -environment ENV.fetch("RAILS_ENV") { "development" } +environment ENV.fetch('RAILS_ENV') { 'development' } # Specifies the number of `workers` to boot in clustered mode. # Workers are forked webserver processes. If using threads and workers together diff --git a/config/routes.rb b/config/routes.rb index 2b6891c9..a9874edd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,8 +1,8 @@ -require 'resque/server' +require 'resque/server' Rails.application.routes.draw do # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html devise_for :users, skip: :all - root to: "bundle_contexts#new" + root to: 'bundle_contexts#new' resources :bundle_contexts, only: [:new, :create] resources :job_runs, only: [:show, :index] get 'job_runs/:id/download', to: 'job_runs#download', as: 'download' diff --git a/lib/tasks/resque.rake b/lib/tasks/resque.rake index cf677f9d..6e34fd3f 100644 --- a/lib/tasks/resque.rake +++ b/lib/tasks/resque.rake @@ -2,7 +2,7 @@ require 'resque/tasks' task 'resque:setup' => :environment require 'resque/pool/tasks' -task "resque:pool:setup" do +task 'resque:pool:setup' do ActiveRecord::Base.connection.disconnect! # close any sockets or files in pool manager Resque::Pool.after_prefork do |_job| ActiveRecord::Base.establish_connection # and re-open them in the resque worker parent diff --git a/spec/controllers/bundle_contexts_controller_spec.rb b/spec/controllers/bundle_contexts_controller_spec.rb index f0eb0fa4..cbb5721d 100644 --- a/spec/controllers/bundle_contexts_controller_spec.rb +++ b/spec/controllers/bundle_contexts_controller_spec.rb @@ -7,7 +7,7 @@ content_structure: 'simple_image', content_metadata_creation: 'default', bundle_dir: 'spec/test_data/smpl_multimedia', - job_runs_attributes: { "0" => { job_type: "preassembly" } } + job_runs_attributes: { '0' => { job_type: 'preassembly' } } } } end @@ -22,21 +22,22 @@ context 'users persisted in db' do before { sign_in(create(:user)) } - it "should have current_user" do + it 'has current_user' do expect(subject.current_user).not_to be_nil end context 'GET new' do - it "renders the new template" do + it 'renders the new template' do get :new expect(response).to render_template('new') expect(response).to have_http_status(200) end end - context "POST create" do - context "Valid Parameters" do + context 'POST create' do + context 'Valid Parameters' do let(:output_dir) { "#{Settings.job_output_parent_dir}/#{subject.current_user.sunet_id}/SMPL-multimedia" } + before do Dir.delete(output_dir) if Dir.exist?(output_dir) post :create, params: params @@ -50,23 +51,23 @@ it 'has the correct attributes' do bc = assigns(:bundle_context) expect(bc.project_name).to eq 'SMPL-multimedia' - expect(bc.content_structure).to eq "simple_image" - expect(bc.content_metadata_creation).to eq "default" - expect(bc.bundle_dir).to eq "spec/test_data/smpl_multimedia" + expect(bc.content_structure).to eq 'simple_image' + expect(bc.content_metadata_creation).to eq 'default' + expect(bc.bundle_dir).to eq 'spec/test_data/smpl_multimedia' end - it "persists the JobRun" do + it 'persists the JobRun' do Dir.delete(output_dir) if Dir.exist?(output_dir) expect { post :create, params: params }.to change(JobRun, :count).by(1) end - it "fails if job_type is nil" do + it 'fails if job_type is nil' do Dir.delete(output_dir) if Dir.exist?(output_dir) - params[:bundle_context][:job_runs_attributes] = { "0" => { job_type: "" } } + params[:bundle_context][:job_runs_attributes] = { '0' => { job_type: '' } } expect { post :create, params: params }.not_to change(JobRun, :count) end end - context "Invalid Parameters" do + context 'Invalid Parameters' do it 'do not create objects' do params[:bundle_context][:project_name] = nil expect { post :create, params: params }.not_to change(BundleContext, :count) diff --git a/spec/jobs/discovery_report_job_spec.rb b/spec/jobs/discovery_report_job_spec.rb index 02ff7893..f9dff3da 100644 --- a/spec/jobs/discovery_report_job_spec.rb +++ b/spec/jobs/discovery_report_job_spec.rb @@ -4,6 +4,7 @@ let(:outfile) { 'tmp/foo.out' } before { allow(job_run.to_discovery_report).to receive(:output_path).and_return(outfile) } + after { FileUtils.rm(outfile) if File.exist?(outfile) } # cleanup describe '#perform' do diff --git a/spec/jobs/preassembly_job_spec.rb b/spec/jobs/preassembly_job_spec.rb index fd9fd836..9f8ee0b7 100644 --- a/spec/jobs/preassembly_job_spec.rb +++ b/spec/jobs/preassembly_job_spec.rb @@ -4,6 +4,7 @@ let(:outfile) { 'tmp/foobar_progress.yaml' } before { allow(job_run.bundle_context).to receive(:progress_log_file).and_return(outfile) } + after { FileUtils.rm(outfile) if File.exist?(outfile) } # cleanup describe '#perform' do diff --git a/spec/lib/csv_importer_spec.rb b/spec/lib/csv_importer_spec.rb index 07540fe3..6ad44a44 100644 --- a/spec/lib/csv_importer_spec.rb +++ b/spec/lib/csv_importer_spec.rb @@ -4,7 +4,7 @@ described_class.parse_to_hash("#{Rails.root}/spec/test_data/flat_dir_images/manifest.csv") end - it "loads a CSV as a hash with indifferent access" do + it 'loads a CSV as a hash with indifferent access' do expect(manifest).to be_an(Array) expect(manifest.size).to eq(3) headers = %w[format sourceid object label year inst_notes prod_notes has_more_metadata description] diff --git a/spec/lib/pre_assembly/bundle_spec.rb b/spec/lib/pre_assembly/bundle_spec.rb index 7433063f..1ee88386 100644 --- a/spec/lib/pre_assembly/bundle_spec.rb +++ b/spec/lib/pre_assembly/bundle_spec.rb @@ -6,30 +6,32 @@ describe '#run_pre_assembly' do let(:exp_workflow_svc_url) { Regexp.new("^#{Dor::Config.dor_services.url}/objects/.*/apo_workflows/assemblyWF$") } + before do allow(RestClient).to receive(:post).with(a_string_matching(exp_workflow_svc_url), {}).and_return(instance_double(RestClient::Response, code: 200)) allow(Dor::Item).to receive(:find).with(any_args) end + it 'runs images_jp2_tif cleanly using images_jp2_tif.yaml for options' do bc = bundle_context_from_hash('images_jp2_tif') # need to delete progress log to ensure this test doesn't skip objects already run FileUtils.rm_rf(bc.output_dir) if Dir.exist?(bc.output_dir) bc.save - b = PreAssembly::Bundle.new bc + b = described_class.new bc pids = [] expect do pids = b.run_pre_assembly end.not_to raise_error - expect(pids).to eq ["druid:jy812bp9403", "druid:tz250tk7584", "druid:gn330dv6119"] + expect(pids).to eq ['druid:jy812bp9403', 'druid:tz250tk7584', 'druid:gn330dv6119'] end end describe '#load_skippables' do - it "returns expected hash of skippable items" do + it 'returns expected hash of skippable items' do allow(smpl_multimedia).to receive(:progress_log_file).and_return('spec/test_data/input/mock_progress_log.yaml') expect(smpl_multimedia.skippables).to eq({}) smpl_multimedia.load_skippables - expect(smpl_multimedia.skippables).to eq("aa" => true, "bb" => true) + expect(smpl_multimedia.skippables).to eq('aa' => true, 'bb' => true) end end @@ -42,24 +44,24 @@ describe '#processed_pids' do it 'pulls pids from digital_objects' do exp_pids = [11, 22, 33] - flat_dir_images.digital_objects = exp_pids.map { |p| double('dobj', :pid => p) } + flat_dir_images.digital_objects = exp_pids.map { |p| double('dobj', pid: p) } expect(flat_dir_images.processed_pids).to eq(exp_pids) end end describe '#digital_objects' do - it "finds the correct number of objects" do + it 'finds the correct number of objects' do b = bundle_setup(:folder_manifest) expect(b.digital_objects.size).to eq(3) end - it "handles containers correctly" do + it 'handles containers correctly' do expect(smpl_multimedia.digital_objects.first.container.size).to be > smpl_multimedia.bundle_dir.size end end describe '#object_discovery: discovery via manifest and crawl' do - it "discover_containers_via_manifest() should return expected information" do + it 'discover_containers_via_manifest() should return expected information' do vals = %w[123.tif 456.tif 789.tif] flat_dir_images.manifest_cols[:object_container] = :col_foo allow(flat_dir_images).to receive(:manifest_rows).and_return(vals.map { |v| { object: v } }) @@ -90,7 +92,7 @@ let(:files) { fs.map { |f| images_jp2_tif.path_in_bundle f } } let(:dirs) { %w[gn330dv6119 jy812bp9403 tz250tk7584].map { |d| images_jp2_tif.path_in_bundle d } } - it "finds expected files with correct relative paths" do + it 'finds expected files with correct relative paths' do tests = [ # Stageables. Expected relative paths. Type of item as stageables. [files, fs.map { |f| File.basename f }], # Files. @@ -107,43 +109,43 @@ end end - describe "object discovery: other" do - it "is able to exercise all_object_files()" do + describe 'object discovery: other' do + it 'is able to exercise all_object_files()' do fake_files = [[1, 2], [3, 4], [5, 6]] - fake_dobjs = fake_files.map { |fs| double('dobj', :object_files => fs) } + fake_dobjs = fake_files.map { |fs| double('dobj', object_files: fs) } flat_dir_images.digital_objects = fake_dobjs expect(flat_dir_images.all_object_files).to eq(fake_files.flatten) end - it "new_object_file() should return an ObjectFile with expected path values" do + it 'new_object_file() should return an ObjectFile with expected path values' do allow(flat_dir_images).to receive(:exclude_from_content).and_return(false) tests = [ # Stageable is a file: # - immediately in bundle dir. - { :stageable => 'BUNDLE/x.tif', - :file_path => 'BUNDLE/x.tif', - :exp_rel_path => 'x.tif' }, + { stageable: 'BUNDLE/x.tif', + file_path: 'BUNDLE/x.tif', + exp_rel_path: 'x.tif' }, # - within subdir of bundle dir. - { :stageable => 'BUNDLE/a/b/x.tif', - :file_path => 'BUNDLE/a/b/x.tif', - :exp_rel_path => 'x.tif' }, + { stageable: 'BUNDLE/a/b/x.tif', + file_path: 'BUNDLE/a/b/x.tif', + exp_rel_path: 'x.tif' }, # Stageable is a directory: # - immediately in bundle dir - { :stageable => 'BUNDLE/a', - :file_path => 'BUNDLE/a/x.tif', - :exp_rel_path => 'a/x.tif' }, + { stageable: 'BUNDLE/a', + file_path: 'BUNDLE/a/x.tif', + exp_rel_path: 'a/x.tif' }, # - immediately in bundle dir, with file deeper - { :stageable => 'BUNDLE/a', - :file_path => 'BUNDLE/a/b/x.tif', - :exp_rel_path => 'a/b/x.tif' }, + { stageable: 'BUNDLE/a', + file_path: 'BUNDLE/a/b/x.tif', + exp_rel_path: 'a/b/x.tif' }, # - within a subdir of bundle dir - { :stageable => 'BUNDLE/a/b', - :file_path => 'BUNDLE/a/b/x.tif', - :exp_rel_path => 'b/x.tif' }, + { stageable: 'BUNDLE/a/b', + file_path: 'BUNDLE/a/b/x.tif', + exp_rel_path: 'b/x.tif' }, # - within a subdir of bundle dir, with file deeper - { :stageable => 'BUNDLE/a/b', - :file_path => 'BUNDLE/a/b/c/d/x.tif', - :exp_rel_path => 'b/c/d/x.tif' } + { stageable: 'BUNDLE/a/b', + file_path: 'BUNDLE/a/b/c/d/x.tif', + exp_rel_path: 'b/c/d/x.tif' } ] tests.each do |t| ofile = flat_dir_images.new_object_file t[:stageable], t[:file_path] @@ -153,15 +155,15 @@ end end - it "exclude_from_content() should behave correctly" do - skip "web app does not need to support exclude_from_content" + it 'exclude_from_content() should behave correctly' do + skip 'web app does not need to support exclude_from_content' expect(smpl_multimedia.exclude_from_content(smpl_multimedia.path_in_bundle('image1.tif'))).to be_falsey expect(smpl_multimedia.exclude_from_content(smpl_multimedia.path_in_bundle('descMetadata.xml'))).to be_truthy end end describe '#load_checksums' do - it "loads checksums and attach them to the ObjectFiles" do + it 'loads checksums and attach them to the ObjectFiles' do smpl_multimedia.all_object_files.each { |f| expect(f.checksum).to be_nil } smpl_multimedia.digital_objects.each { |dobj| smpl_multimedia.load_checksums(dobj) } smpl_multimedia.all_object_files.each { |f| expect(f.checksum).to match(md5_regex) } @@ -169,7 +171,7 @@ end describe '#digital_objects' do - it "augments the digital objects with additional information" do + it 'augments the digital objects with additional information' do expect(flat_dir_images.digital_objects.size).to eq(3) flat_dir_images.digital_objects.each do |dobj| expect(dobj.label).to be_a(String) @@ -181,12 +183,12 @@ end describe '#objects_to_process' do - it "returns all objects if there are no skippables" do + it 'returns all objects if there are no skippables' do flat_dir_images.skippables = {} expect(flat_dir_images.objects_to_process).to eq(flat_dir_images.digital_objects) end - it "returns a filtered list of digital objects" do + it 'returns a filtered list of digital objects' do flat_dir_images.skippables = {} flat_dir_images.skippables[flat_dir_images.digital_objects.last.container] = true o2p = flat_dir_images.objects_to_process @@ -195,34 +197,34 @@ end end - describe "#log_progress_info" do - it "returns expected info about a digital object" do + describe '#log_progress_info' do + it 'returns expected info about a digital object' do dobj = flat_dir_images.digital_objects[0] exp = { - :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') } expect(flat_dir_images.log_progress_info(dobj)).to eq(exp) end end - describe "file and directory utilities" do + describe 'file and directory utilities' do let(:relative) { 'abc/def.jpg' } let(:full) { flat_dir_images.path_in_bundle(relative) } - it "#path_in_bundle returns expected value" do + it '#path_in_bundle returns expected value' do expect(flat_dir_images.path_in_bundle(relative)).to eq('spec/test_data/flat_dir_images/abc/def.jpg') end - it "#relative_path returns expected value" do + it '#relative_path returns expected value' do expect(flat_dir_images.relative_path(flat_dir_images.bundle_dir, full)).to eq(relative) end - it "#get_base_dir returns expected value" do + it '#get_base_dir returns expected value' do expect(flat_dir_images.get_base_dir('foo/bar/fubb.txt')).to eq('foo/bar') end - it "#get_base_dir raises error if given bogus arguments" do + it '#get_base_dir raises error if given bogus arguments' do exp_msg = /^Bad arg to get_base_dir/ bad_args = ['foo.txt', '', 'x\y\foo.txt'] bad_args.each do |arg| @@ -230,32 +232,32 @@ end end - it "#dir_glob returns expected information" do + it '#dir_glob returns expected information' do exp = [1, 2, 3].map { |n| flat_dir_images.path_in_bundle "image#{n}.tif" } - expect(flat_dir_images.dir_glob(flat_dir_images.path_in_bundle("*.tif"))).to eq(exp) + expect(flat_dir_images.dir_glob(flat_dir_images.path_in_bundle('*.tif'))).to eq(exp) end - it "#find_files_recursively returns expected information" do + it '#find_files_recursively returns expected information' do { - :flat_dir_images => [ - "checksums.txt", - "image1.tif", - "image2.tif", - "image3.tif", - "manifest.csv", - "manifest_badsourceid_column.csv" + flat_dir_images: [ + 'checksums.txt', + 'image1.tif', + 'image2.tif', + 'image3.tif', + 'manifest.csv', + 'manifest_badsourceid_column.csv' ], - :images_jp2_tif => [ - "gn330dv6119/image1.jp2", - "gn330dv6119/image1.tif", - "gn330dv6119/image2.jp2", - "gn330dv6119/image2.tif", - "jy812bp9403/00/image1.tif", - "jy812bp9403/00/image2.tif", - "jy812bp9403/05/image1.jp2", - "manifest.csv", - "tz250tk7584/00/image1.tif", - "tz250tk7584/00/image2.tif" + images_jp2_tif: [ + 'gn330dv6119/image1.jp2', + 'gn330dv6119/image1.tif', + 'gn330dv6119/image2.jp2', + 'gn330dv6119/image2.tif', + 'jy812bp9403/00/image1.tif', + 'jy812bp9403/00/image2.tif', + 'jy812bp9403/05/image1.jp2', + 'manifest.csv', + 'tz250tk7584/00/image1.tif', + 'tz250tk7584/00/image2.tif' ] }.each do |proj, files| b = described_class.new(bundle_context_from_hash(proj)) diff --git a/spec/lib/pre_assembly/digital_object_spec.rb b/spec/lib/pre_assembly/digital_object_spec.rb index 8835d7c7..ce02ba2d 100644 --- a/spec/lib/pre_assembly/digital_object_spec.rb +++ b/spec/lib/pre_assembly/digital_object_spec.rb @@ -7,6 +7,7 @@ let(:tmp_dir_args) { [nil, 'tmp'] } before(:all) { FileUtils.rm_rf('log/test_jobs') } + before do allow(bc).to receive(:progress_log_file).and_return(Tempfile.new('images_jp2_tif').path) dobj.object_files = [] @@ -16,10 +17,10 @@ def add_object_files(extension = 'tif') (1..2).each do |i| f = "image#{i}.#{extension}" dobj.object_files.push PreAssembly::ObjectFile.new( - :path => "#{dobj.bundle_dir}/#{dru}/#{f}", - :relative_path => f, - :exclude_from_content => false, - :checksum => i.to_s * 4 + path: "#{dobj.bundle_dir}/#{dru}/#{f}", + relative_path: f, + exclude_from_content: false, + checksum: i.to_s * 4 ) end end @@ -37,6 +38,7 @@ def add_object_files(extension = 'tif') let(:tmp_area) do Dir.mktmpdir(*tmp_dir_args) end + before do allow(dobj).to receive(:druid).and_return(druid) allow(dobj).to receive(:bundle_dir).and_return(tmp_area) @@ -45,9 +47,10 @@ def add_object_files(extension = 'tif') dobj.stageable_items.each { |si| FileUtils.touch si } FileUtils.mkdir dobj.assembly_staging_dir end + after { FileUtils.remove_entry tmp_area } - it "is able to copy stageable items successfully" do + it 'is able to copy stageable items successfully' do dobj.stage_files # Check outcome: both source and copy should exist. files.each_with_index do |f, i| @@ -59,7 +62,7 @@ def add_object_files(extension = 'tif') end end - it "is able to symlink stageable items successfully" do + it 'is able to symlink stageable items successfully' do allow(bc).to receive(:staging_style_symlink).and_return(true) dobj.stage_files # Check outcome: both source and copy should exist. @@ -73,7 +76,7 @@ def add_object_files(extension = 'tif') end end - describe "default content metadata" do + describe 'default content metadata' do let(:exp_xml) do noko_doc <<-END @@ -107,20 +110,20 @@ def add_object_files(extension = 'tif') before do allow(dobj).to receive(:druid).and_return(druid) - allow(dobj).to receive(:content_type_tag).and_return("") + allow(dobj).to receive(:content_type_tag).and_return('') allow(bc).to receive(:content_structure).and_return('simple_image') add_object_files('tif') add_object_files('jp2') dobj.create_content_metadata end - it "content_object_files() should filter @object_files correctly" do + it 'content_object_files() should filter @object_files correctly' do # Generate some object_files. files = %w[file5.tif file4.tif file3.tif file2.tif file1.tif file0.tif] n = files.size m = n / 2 dobj.object_files = files.map do |f| - PreAssembly::ObjectFile.new(:exclude_from_content => false, :relative_path => f) + PreAssembly::ObjectFile.new(exclude_from_content: false, relative_path: f) end # All of them are included in content. expect(dobj.content_object_files.size).to eq(n) @@ -132,11 +135,11 @@ def add_object_files(extension = 'tif') expect(ofiles.map(&:relative_path)).to eq(files[m..-1].sort) end - it "generates the expected xml text" do + it 'generates the expected xml text' do expect(noko_doc(dobj.content_md_xml)).to be_equivalent_to exp_xml end - it "is able to write the content_metadata XML to a file" do + it 'is able to write the content_metadata XML to a file' do Dir.mktmpdir(*tmp_dir_args) do |tmp_area| dobj.druid_tree_dir = tmp_area file_name = File.join(tmp_area, 'metadata', dobj.send(:content_md_file)) @@ -156,7 +159,7 @@ def add_object_files(extension = 'tif') end end - describe "bundled by filename, simple book content metadata without file attributes" do + describe 'bundled by filename, simple book content metadata without file attributes' do let(:exp_xml) do noko_doc <<-END @@ -184,7 +187,7 @@ def add_object_files(extension = 'tif') before do allow(dobj).to receive(:druid).and_return(druid) - allow(dobj).to receive(:content_type_tag).and_return("") + allow(dobj).to receive(:content_type_tag).and_return('') allow(bc).to receive(:content_structure).and_return('simple_book') allow(bc).to receive(:content_md_creation).and_return('filename') add_object_files('tif') @@ -192,12 +195,12 @@ def add_object_files(extension = 'tif') dobj.create_content_metadata end - it "generates the expected xml text" do + it 'generates the expected xml text' do expect(noko_doc(dobj.content_md_xml)).to be_equivalent_to(exp_xml) end end - describe "content metadata generated from object tag in DOR if present and overriding is allowed" do + describe 'content metadata generated from object tag in DOR if present and overriding is allowed' do let(:exp_xml) do noko_doc <<-END @@ -238,13 +241,13 @@ def add_object_files(extension = 'tif') dobj.create_content_metadata end - it "content_object_files() should filter @object_files correctly" do + it 'content_object_files() should filter @object_files correctly' do # Generate some object_files. files = %w[file5.tif file4.tif file3.tif file2.tif file1.tif file0.tif] n = files.size m = n / 2 dobj.object_files = files.map do |f| - PreAssembly::ObjectFile.new(:exclude_from_content => false, :relative_path => f) + PreAssembly::ObjectFile.new(exclude_from_content: false, relative_path: f) end # All of them are included in content. expect(dobj.content_object_files.size).to eq(n) @@ -256,13 +259,13 @@ def add_object_files(extension = 'tif') expect(ofiles.map(&:relative_path)).to eq(files[m..-1].sort) end - it "generates the expected xml text" do + it 'generates the expected xml text' do expect(dobj.content_md_creation_style).to eq(:file) expect(noko_doc(dobj.content_md_xml)).to be_equivalent_to(exp_xml) end end - describe "content metadata generated from object tag in DOR if present but overriding is not allowed" do + describe 'content metadata generated from object tag in DOR if present but overriding is not allowed' do let(:exp_xml) do noko_doc <<-END @@ -303,11 +306,11 @@ def add_object_files(extension = 'tif') dobj.create_content_metadata end - it "content_object_files() should filter @object_files correctly" do + it 'content_object_files() should filter @object_files correctly' do # Generate some object_files. files = %w[file5.tif file4.tif file3.tif file2.tif file1.tif file0.tif] dobj.object_files = files.map do |f| - PreAssembly::ObjectFile.new(:exclude_from_content => false, :relative_path => f) + PreAssembly::ObjectFile.new(exclude_from_content: false, relative_path: f) end # All of them are included in content. expect(dobj.content_object_files.size).to eq(files.size) diff --git a/spec/lib/pre_assembly/object_file_spec.rb b/spec/lib/pre_assembly/object_file_spec.rb index 8ad9ae9d..af66f36a 100644 --- a/spec/lib/pre_assembly/object_file_spec.rb +++ b/spec/lib/pre_assembly/object_file_spec.rb @@ -1,12 +1,12 @@ RSpec.describe PreAssembly::ObjectFile do before do @f = described_class.new( - :path => 'spec/test_data/flat_dir_images/image1.tif' + path: 'spec/test_data/flat_dir_images/image1.tif' ) end - describe "initialization" do - it "can initialize an ObjectFile" do + describe 'initialization' do + it 'can initialize an ObjectFile' do expect(@f).to be_kind_of described_class end end diff --git a/spec/lib/pre_assembly/smpl_spec.rb b/spec/lib/pre_assembly/smpl_spec.rb index 23896bd7..f4a57a63 100644 --- a/spec/lib/pre_assembly/smpl_spec.rb +++ b/spec/lib/pre_assembly/smpl_spec.rb @@ -2,10 +2,10 @@ let(:bundle_dir) { Rails.root.join('spec/test_data/smpl_multimedia') } let(:bc_params) do { - :project_name => 'ProjectBar', + project_name: 'ProjectBar', # :publish_attr => { :publish => 'no', :shelve => 'no', :preserve => 'yes' }, - :bundle_dir => bundle_dir, - :content_metadata_creation => :smpl_cm_style + bundle_dir: bundle_dir, + content_metadata_creation: :smpl_cm_style } end let(:bc) { build(:bundle_context, bc_params) } @@ -14,10 +14,10 @@ let(:dobj1) { setup_dobj('aa111aa1111', smpl_manifest) } let(:dobj2) { setup_dobj('bb222bb2222', smpl_manifest) } let(:smpl_manifest) do - described_class.new(:csv_filename => 'smpl_manifest.csv', :bundle_dir => bundle_dir) + described_class.new(csv_filename: 'smpl_manifest.csv', bundle_dir: bundle_dir) end - it "generates technicalMetadata for SMPL by combining all existing _techmd.xml files" do + it 'generates technicalMetadata for SMPL by combining all existing _techmd.xml files' do dobj1.create_technical_metadata exp_xml = noko_doc(dobj1.technical_md_xml) expect(exp_xml.css('technicalMetadata').size).to eq(1) # one top level node @@ -28,7 +28,7 @@ expect(counts.map(&:content)).to eq(%w[279 217 280 218]) end - it "generates content metadata from a SMPL manifest with no thumb columns" do + it 'generates content metadata from a SMPL manifest with no thumb columns' do dobj1.create_content_metadata dobj2.create_content_metadata expect(noko_doc(dobj1.content_md_xml)).to be_equivalent_to noko_doc(exp_xml_object_aa111aa1111) @@ -37,8 +37,8 @@ end describe '#create_content_metadata - with thumb declaration' do - it "generates content metadata from a SMPL manifest with a thumb column set to yes" do - smpl_manifest = described_class.new(:csv_filename => 'smpl_manifest_with_thumb.csv', :bundle_dir => bundle_dir) + it 'generates content metadata from a SMPL manifest with a thumb column set to yes' do + smpl_manifest = described_class.new(csv_filename: 'smpl_manifest_with_thumb.csv', bundle_dir: bundle_dir) dobj1 = setup_dobj('aa111aa1111', smpl_manifest) dobj2 = setup_dobj('bb222bb2222', smpl_manifest) dobj1.create_content_metadata @@ -47,8 +47,8 @@ expect(noko_doc(dobj2.content_md_xml)).to be_equivalent_to noko_doc(exp_xml_object_bb222bb2222) end - it "generates content metadata from a SMPL manifest with a thumb column set to true" do - smpl_manifest = described_class.new(:csv_filename => 'smpl_manifest_with_thumb_true.csv', :bundle_dir => bundle_dir) + it 'generates content metadata from a SMPL manifest with a thumb column set to true' do + smpl_manifest = described_class.new(csv_filename: 'smpl_manifest_with_thumb_true.csv', bundle_dir: bundle_dir) dobj1 = setup_dobj('aa111aa1111', smpl_manifest) dobj2 = setup_dobj('bb222bb2222', smpl_manifest) dobj1.create_content_metadata @@ -57,8 +57,8 @@ expect(noko_doc(dobj2.content_md_xml)).to be_equivalent_to noko_doc(exp_xml_object_bb222bb2222) end - it "generates content metadata from a SMPL manifest with no thumbs when the thumb column is set to no" do - smpl_manifest = described_class.new(:csv_filename => 'smpl_manifest_thumb_no.csv', :bundle_dir => bundle_dir) + it 'generates content metadata from a SMPL manifest with no thumbs when the thumb column is set to no' do + smpl_manifest = described_class.new(csv_filename: 'smpl_manifest_thumb_no.csv', bundle_dir: bundle_dir) dobj1 = setup_dobj('aa111aa1111', smpl_manifest) dobj2 = setup_dobj('bb222bb2222', smpl_manifest) dobj1.create_content_metadata diff --git a/spec/mailers/job_mailer_spec.rb b/spec/mailers/job_mailer_spec.rb index f3f55cfa..91186785 100644 --- a/spec/mailers/job_mailer_spec.rb +++ b/spec/mailers/job_mailer_spec.rb @@ -3,16 +3,16 @@ let(:job_notification) { described_class.with(job_run: job_run).completion_email } it 'renders the headers' do - expect(job_notification.subject).to eq("[Test_Project] Your Discovery report job completed") + expect(job_notification.subject).to eq('[Test_Project] Your Discovery report job completed') expect(job_notification.to).to eq([job_run.bundle_context.user.email]) - expect(job_notification.from).to eq(["no-reply-preassembly-job@stanford.edu"]) + expect(job_notification.from).to eq(['no-reply-preassembly-job@stanford.edu']) end describe 'subject' do before { job_run.job_type = 1 } it 'adapts depending on job_type' do - expect(job_notification.subject).to eq("[Test_Project] Your Preassembly job completed") + expect(job_notification.subject).to eq('[Test_Project] Your Preassembly job completed') end end diff --git a/spec/models/bundle_context_spec.rb b/spec/models/bundle_context_spec.rb index 420174bb..93238c98 100644 --- a/spec/models/bundle_context_spec.rb +++ b/spec/models/bundle_context_spec.rb @@ -1,14 +1,15 @@ RSpec.describe BundleContext, type: :model do + subject(:bc) { build(:bundle_context_with_deleted_output_dir, attr_hash) } + let(:attr_hash) do { project_name: 'Images_jp2_tif', bundle_dir: 'spec/test_data/images_jp2_tif' } end - subject(:bc) { build(:bundle_context_with_deleted_output_dir, attr_hash) } - context "validation" do - it "is not valid unless it has all required attributes" do + context 'validation' do + it 'is not valid unless it has all required attributes' do expect(BundleContext.new).not_to be_valid expect(bc).to be_valid end @@ -40,18 +41,18 @@ it do is_expected.to define_enum_for(:content_structure).with( - "simple_image" => 0, - "simple_book" => 1, - "book_as_image" => 2, - "file" => 3, - "smpl" => 4 + 'simple_image' => 0, + 'simple_book' => 1, + 'book_as_image' => 2, + 'file' => 3, + 'smpl' => 4 ) end it do is_expected.to define_enum_for(:content_metadata_creation).with( - "default" => 0, - "filename" => 1, - "smpl_cm_style" => 2 + 'default' => 0, + 'filename' => 1, + 'smpl_cm_style' => 2 ) end @@ -64,7 +65,7 @@ end it 'bundle_dir has trailing slash removed' do - expect(bc.bundle_dir).to eq "spec/test_data/images_jp2_tif" + expect(bc.bundle_dir).to eq 'spec/test_data/images_jp2_tif' end describe '#bundle' do @@ -76,27 +77,27 @@ end end - describe "#assembly_staging_dir" do + describe '#assembly_staging_dir' do it 'comes from Settings file' do expect(described_class.new.assembly_staging_dir).to eq Settings.assembly_staging_dir end end - describe "#smpl_manifest" do - it "returns the file name" do + describe '#smpl_manifest' do + it 'returns the file name' do expect(described_class.new.smpl_manifest).to eq 'smpl_manifest.csv' end end - describe "#manifest" do - it "returns the file name" do + describe '#manifest' do + it 'returns the file name' do expect(described_class.new.manifest).to eq 'manifest.csv' end end - describe "#path_in_bundle" do - it "creates a relative path" do - expect(bc.path_in_bundle("manifest.csv")).to eq "spec/test_data/images_jp2_tif/manifest.csv" + describe '#path_in_bundle' do + it 'creates a relative path' do + expect(bc.path_in_bundle('manifest.csv')).to eq 'spec/test_data/images_jp2_tif/manifest.csv' end end @@ -106,36 +107,36 @@ end end - describe "#progress_log_file" do + describe '#progress_log_file' do it 'is project_name + "_progress.yml" in output_dir' do expect(bc.progress_log_file).to eq "#{bc.output_dir}/#{bc.project_name}_progress.yml" end end - describe "manifest_rows" do - it "loads the manifest CSV" do - expect(CsvImporter).to receive(:parse_to_hash).with("spec/test_data/images_jp2_tif/manifest.csv") + describe 'manifest_rows' do + it 'loads the manifest CSV' do + expect(CsvImporter).to receive(:parse_to_hash).with('spec/test_data/images_jp2_tif/manifest.csv') bc.manifest_rows end - it "memoizes the manifest rows" do - expect(CsvImporter).to receive(:parse_to_hash).once.with("spec/test_data/images_jp2_tif/manifest.csv").and_call_original + it 'memoizes the manifest rows' do + expect(CsvImporter).to receive(:parse_to_hash).once.with('spec/test_data/images_jp2_tif/manifest.csv').and_call_original 2.times { bc.manifest_rows } end - it "expect the content of manifest rows" do + it 'expect the content of manifest rows' do expect(bc.manifest_rows).to eq( [ - { "druid" => "druid:jy812bp9403", "sourceid" => "bar-1.0", "object" => "jy812bp9403", "label" => "Label 1", "description" => "This is a description for label 1" }, - { "druid" => "druid:tz250tk7584", "sourceid" => "bar-2.1", "object" => "tz250tk7584", "label" => "Label 2", "description" => "This is a description for label 2" }, - { "druid" => "druid:gn330dv6119", "sourceid" => "bar-3.1", "object" => "gn330dv6119", "label" => "Label 3", "description" => "This is a description for label 3" } + { 'druid' => 'druid:jy812bp9403', 'sourceid' => 'bar-1.0', 'object' => 'jy812bp9403', 'label' => 'Label 1', 'description' => 'This is a description for label 1' }, + { 'druid' => 'druid:tz250tk7584', 'sourceid' => 'bar-2.1', 'object' => 'tz250tk7584', 'label' => 'Label 2', 'description' => 'This is a description for label 2' }, + { 'druid' => 'druid:gn330dv6119', 'sourceid' => 'bar-3.1', 'object' => 'gn330dv6119', 'label' => 'Label 3', 'description' => 'This is a description for label 3' } ] ) end end - describe "manifest_cols" do - it "sets the column names" do + describe 'manifest_cols' do + it 'sets the column names' do expect(bc.manifest_cols).to eq( label: 'label', source_id: 'sourceid', @@ -147,6 +148,7 @@ describe '#verify_output_dir (private method)' do before { FileUtils.mkdir_p(Settings.job_output_parent_dir) } + after { Dir.delete(bc.output_dir) if Dir.exist?(bc.output_dir) } # cleanup context 'when bundle_context is new' do diff --git a/spec/models/job_run_spec.rb b/spec/models/job_run_spec.rb index 06f6f9b1..2e02650d 100644 --- a/spec/models/job_run_spec.rb +++ b/spec/models/job_run_spec.rb @@ -21,7 +21,9 @@ describe 'send_notification' do let(:mock_mailer) { double JobMailer } let(:mock_delivery) { double ActionMailer::MessageDelivery } + before { job_run.save } + it 'does not send an email if output_location is unchanged' do expect(job_run).not_to receive(:send_notification) job_run.save diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index aec7a37c..0a39e754 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1,8 +1,8 @@ RSpec.describe User, type: :model do subject(:user) { build(:user, sunet_id: 'jdoe@stanford.edu') } - context "validation" do - it "is not valid unless it has all required attributes" do + context 'validation' do + it 'is not valid unless it has all required attributes' do expect(User.new).not_to be_valid expect(user).to be_valid end @@ -23,7 +23,7 @@ end end - context "email address" do + context 'email address' do it "returns the user's email address if the sunet_id is already an address" do expect(user.email).to eq('jdoe@stanford.edu') end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index a59cee49..0811d7a2 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -3,7 +3,7 @@ ENV['RAILS_ENV'] ||= 'test' require File.expand_path('../config/environment', __dir__) # Prevent database truncation if the environment is production -abort("The Rails environment is running in production mode!") if Rails.env.production? +abort('The Rails environment is running in production mode!') if Rails.env.production? require 'rspec/rails' # Add additional requires below this line. Rails is not loaded until this point! diff --git a/spec/services/discovery_report_spec.rb b/spec/services/discovery_report_spec.rb index 96eda2ef..eb53b1b9 100644 --- a/spec/services/discovery_report_spec.rb +++ b/spec/services/discovery_report_spec.rb @@ -1,7 +1,8 @@ RSpec.describe DiscoveryReport do - let(:bundle) { bundle_setup(:flat_dir_images) } subject(:report) { described_class.new(bundle) } + let(:bundle) { bundle_setup(:flat_dir_images) } + describe '#initialize' do it 'raises if PreAssembly::Bundle not received' do expect { described_class.new }.to raise_error(ArgumentError) @@ -62,14 +63,14 @@ ) end - context "folders are empty" do - it "adds empty_object error" do + context 'folders are empty' do + it 'adds empty_object error' do expect(report.process_dobj(dobj)).to match a_hash_including(errors: a_hash_including(empty_object: true)) end end - context "folders are not empty" do - let(:obj_file) { instance_double(PreAssembly::ObjectFile, path: "random/path", filesize: 324, mimetype: "") } + context 'folders are not empty' do + let(:obj_file) { instance_double(PreAssembly::ObjectFile, path: 'random/path', filesize: 324, mimetype: '') } before do allow(dobj).to receive(:object_files).and_return([obj_file, obj_file]) @@ -77,16 +78,16 @@ allow(report).to receive(:registration_check).and_return({}) # pretend everything is in Dor end - it "does not add empty_object error" do + it 'does not add empty_object error' do expect(report.process_dobj(dobj)).not_to include(a_hash_including(empty_object: true)) end end end - context "integration test" do + context 'integration test' do let(:bc_params) do { - project_name: "SmokeTest", + project_name: 'SmokeTest', content_structure: 0, bundle_dir: 'spec/test_data/images_jp2_tif', staging_style_symlink: false, @@ -99,13 +100,13 @@ let(:dobj) { report.bundle.objects_to_process.first } before do - allow(dobj).to receive(:pid).and_return("kk203bw3276") + allow(dobj).to receive(:pid).and_return('kk203bw3276') allow(report).to receive(:registration_check).and_return({}) # pretend everything is in Dor end it 'process_dobj gives expected output for one dobj' do expect(report.process_dobj(dobj)).to eq( - druid: "druid:kk203bw3276", + druid: 'druid:kk203bw3276', errors: { dupes: true }, counts: { total_size: 254_802, diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 844ae7fe..12c84fa3 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -40,7 +40,7 @@ # Allows RSpec to persist some state between runs in order to support # the `--only-failures` and `--next-failure` CLI options. We recommend # you configure your source control system to ignore this file. - config.example_status_persistence_file_path = "spec/examples.txt" + config.example_status_persistence_file_path = 'spec/examples.txt' # Limits the available syntax to the non-monkey patched syntax that is # recommended. For more details, see: @@ -56,7 +56,7 @@ # Use the documentation formatter for detailed output, # unless a formatter has already been configured # (e.g. via a command-line flag). - config.default_formatter = "doc" + config.default_formatter = 'doc' end # Print the 10 slowest examples and example groups at the diff --git a/spec/support/bundle_setup.rb b/spec/support/bundle_setup.rb index 72c2b8ad..15ebdc58 100644 --- a/spec/support/bundle_setup.rb +++ b/spec/support/bundle_setup.rb @@ -14,13 +14,13 @@ def noko_doc(x) def bundle_context_from_hash(proj) hash = hash_from_proj(proj) - cmc = hash["content_md_creation"]["style"] - cmc += '_cm_style' if cmc == "smpl" + cmc = hash['content_md_creation']['style'] + cmc += '_cm_style' if cmc == 'smpl' build( :bundle_context, - project_name: hash["project_name"], - content_structure: hash["project_style"]["content_structure"], - bundle_dir: hash["bundle_dir"], + project_name: hash['project_name'], + content_structure: hash['project_style']['content_structure'], + bundle_dir: hash['bundle_dir'], content_metadata_creation: cmc, user: build(:user, sunet_id: 'Jdoe@stanford.edu') ) diff --git a/spec/views/bundle_context_spec.rb b/spec/views/bundle_context_spec.rb index cd67bf75..348677fa 100644 --- a/spec/views/bundle_context_spec.rb +++ b/spec/views/bundle_context_spec.rb @@ -1,5 +1,5 @@ RSpec.describe 'bundle_contexts/new' do - context "Displays the Bundle Context Form" do + context 'Displays the Bundle Context Form' do it 'displays the form correctly' do assign(:bundle_context, BundleContext.new) # Should render the test page @@ -8,7 +8,7 @@ end end - context "Displays errors in Bundle Context Form" + context 'Displays errors in Bundle Context Form' it 'displays error message when missing project name' do bc = build(:bundle_context, project_name: nil).tap(&:valid?) assign(:bundle_context, bc) From f268a134a95da71df50872ab9624f969276be190 Mon Sep 17 00:00:00 2001 From: Joe Atzberger Date: Thu, 27 Sep 2018 15:49:16 -0700 Subject: [PATCH 3/8] auto-gen-config fresh rubocop_todo.yml file Now pretty clean, compared to where we started --- .rubocop_todo.yml | 50 +++++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index a381376f..349022e3 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,25 +1,30 @@ # 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: 2 +Lint/DuplicateMethods: + Exclude: + - 'app/lib/pre_assembly/digital_object.rb' + +# 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 +34,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,6 +67,14 @@ Naming/UncommunicativeMethodParamName: Exclude: - 'spec/support/bundle_setup.rb' +# Offense count: 1 +RSpec/BeforeAfterAll: + Exclude: + - 'spec/spec_helper.rb' + - 'spec/rails_helper.rb' + - 'spec/support/**/*.rb' + - 'spec/lib/pre_assembly/digital_object_spec.rb' + # Offense count: 2 RSpec/DescribeClass: Exclude: @@ -74,24 +87,16 @@ RSpec/ExampleLength: Exclude: - 'spec/lib/pre_assembly/bundle_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: 1 +# Offense count: 2 RSpec/LetSetup: Exclude: + - 'spec/views/bundle_contexts/new.html.erb_spec.rb' - 'spec/views/job_runs/index.json.jbuilder_spec.rb' # Offense count: 1 @@ -117,16 +122,23 @@ RSpec/VerifiedDoubles: - '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 +# Offense count: 6 Style/CommentedKeyword: Exclude: - 'app/lib/pre_assembly/smpl.rb' - - 'spec/lib/pre_assembly/smpl_spec.rb' # Offense count: 19 Style/Documentation: From 02cd5798aa82bb692141b107938f4f1a4d4e26b5 Mon Sep 17 00:00:00 2001 From: Joe Atzberger Date: Thu, 27 Sep 2018 16:01:25 -0700 Subject: [PATCH 4/8] Manual corrections to smpl.rb and csv_importer_spec.rb Mostly about line-length. (Ignore generated files.) Also includes the bizarre double-checking of `defined(x) && x`. --- .rubocop.yml | 2 ++ app/lib/pre_assembly/smpl.rb | 23 ++++++++++++----------- spec/lib/csv_importer_spec.rb | 4 +--- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 07bc626b..59feeb6d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -26,6 +26,8 @@ Layout/SpaceBeforeBlockBraces: # Configuration parameters: AllowURI, URISchemes. Metrics/LineLength: Max: 200 + Exclude: + - 'config/initializers/**/*.rb' # generated files RSpec/ContextWording: Enabled: false # too dogmatic diff --git a/app/lib/pre_assembly/smpl.rb b/app/lib/pre_assembly/smpl.rb index e936140a..4afb2b60 100644 --- a/app/lib/pre_assembly/smpl.rb +++ b/app/lib/pre_assembly/smpl.rb @@ -43,21 +43,22 @@ def load_manifest role = get_role(row[:filename]) file_extension = File.extname(row[:filename]) # set the resource type if available, otherwise we'll use a default - resource_type = defined?(row[:resource_type]) ? row[:resource_type] || nil : nil + resource_type = row[:resource_type] || nil # set the thumb attribute for this resource if it is set in the manifest to true, yes or thumb (set to false if no value or column is missing) - thumb = defined?(row[:thumb]) && row[:thumb] && %w[true yes thumb].include?(row[:thumb].downcase) ? true : false + thumb = row[:thumb] && %w[true yes thumb].include?(row[:thumb].downcase) ? true : false # set the publish/preserve/shelve if available, otherwise we'll use the defaults - publish = defined?(row[:publish]) ? row[:publish] || nil : nil - shelve = defined?(row[:shelve]) ? row[:shelve] || nil : nil - preserve = defined?(row[:preserve]) ? row[:preserve] || nil : nil - - manifest[druid] = { source_id: '', files: [] } if manifest[druid].nil? - manifest[druid][:source_id] = row[:source_id] if defined?(row[:source_id]) && row[:source_id] - manifest[druid][:files] << { thumb: thumb, publish: publish, shelve: shelve, preserve: preserve, resource_type: resource_type, role: role, file_extention: file_extension, filename: row[:filename], label: row[:label], sequence: row[:sequence] } - end # loop over all rows - end # load_manifest + publish = row[:publish] || nil + shelve = row[:shelve] || nil + preserve = row[:preserve] || nil + + manifest[druid] ||= { source_id: '', files: [] } + manifest[druid][:source_id] = row[:source_id] if row[:source_id] + files_hash = { role: role, file_extention: file_extension, filename: row[:filename], label: row[:label], sequence: row[:sequence] } + manifest[druid][:files] << files_hash.merge(thumb: thumb, publish: publish, shelve: shelve, preserve: preserve, resource_type: resource_type) + end + end # actually generate content metadata for a specific druid in the manifest # @return [String] XML diff --git a/spec/lib/csv_importer_spec.rb b/spec/lib/csv_importer_spec.rb index 6ad44a44..c90c651c 100644 --- a/spec/lib/csv_importer_spec.rb +++ b/spec/lib/csv_importer_spec.rb @@ -10,9 +10,7 @@ headers = %w[format sourceid object label year inst_notes prod_notes has_more_metadata description] expect(manifest).to all(be_an(ActiveSupport::HashWithIndifferentAccess)) # accessible w/ string and symbols expect(manifest).to all(include(*headers)) - expect(manifest[0][:description]).to be_nil - expect(manifest[1][:description]).to eq('') - expect(manifest[2][:description]).to eq('yo, this is a description') + expect(manifest.pluck(:description)).to eq([nil, '', 'yo, this is a description']) end end end From 3276315ea8f28981321374c1480ae72b9461592b Mon Sep 17 00:00:00 2001 From: Joe Atzberger Date: Thu, 27 Sep 2018 16:16:18 -0700 Subject: [PATCH 5/8] Allow conventional view specs (naming views, not classes) --- .rubocop.yml | 5 +++++ .rubocop_todo.yml | 6 ------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 59feeb6d..3d48ef14 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -32,6 +32,11 @@ Metrics/LineLength: 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 diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 349022e3..9d9fae65 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -75,12 +75,6 @@ RSpec/BeforeAfterAll: - 'spec/support/**/*.rb' - 'spec/lib/pre_assembly/digital_object_spec.rb' -# Offense count: 2 -RSpec/DescribeClass: - Exclude: - - 'spec/views/bundle_context_spec.rb' - - 'spec/views/job_runs/index.json.jbuilder_spec.rb' - # Offense count: 2 # Configuration parameters: Max. RSpec/ExampleLength: From 4520b861e938ddd17c0c7721b14c904eef304d57 Mon Sep 17 00:00:00 2001 From: Joe Atzberger Date: Thu, 27 Sep 2018 16:18:39 -0700 Subject: [PATCH 6/8] Fix Style/ClassAndModuleChildren in object_file.rb --- .rubocop_todo.yml | 8 -------- app/lib/pre_assembly/object_file.rb | 32 +++++++++++++++-------------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 9d9fae65..7e92725b 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -116,14 +116,6 @@ RSpec/VerifiedDoubles: - '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: diff --git a/app/lib/pre_assembly/object_file.rb b/app/lib/pre_assembly/object_file.rb index 310ba364..d8f8b4e0 100644 --- a/app/lib/pre_assembly/object_file.rb +++ b/app/lib/pre_assembly/object_file.rb @@ -1,20 +1,22 @@ -class PreAssembly::ObjectFile < Assembly::ObjectFile - attr_accessor :relative_path, :exclude_from_content - attr_reader :checksum +module PreAssembly + class ObjectFile < Assembly::ObjectFile + attr_accessor :relative_path, :exclude_from_content + attr_reader :checksum - def initialize(params = {}) - @path = params[:path] - @relative_path = params[:relative_path] - self.checksum = params[:checksum] - @exclude_from_content = params[:exclude_from_content] - end + def initialize(params = {}) + @path = params[:path] + @relative_path = params[:relative_path] + self.checksum = params[:checksum] + @exclude_from_content = params[:exclude_from_content] + end - def checksum=(value) - @checksum = value - self.provider_md5 = value # this is an attribute of the Assembly::ObjectFile class - end + def checksum=(value) + @checksum = value + self.provider_md5 = value # this is an attribute of the Assembly::ObjectFile class + end - def <=>(other) - @relative_path <=> other.relative_path + def <=>(other) + @relative_path <=> other.relative_path + end end end From d91b0bd14247d2ef588d8b3f41a8dd652d5fe32e Mon Sep 17 00:00:00 2001 From: Joe Atzberger Date: Thu, 27 Sep 2018 16:23:09 -0700 Subject: [PATCH 7/8] Remove `dor_object=` double-declaration --- .rubocop_todo.yml | 5 ----- app/lib/pre_assembly/digital_object.rb | 1 - 2 files changed, 6 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 7e92725b..0a9c0b97 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -6,11 +6,6 @@ # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 2 -Lint/DuplicateMethods: - Exclude: - - 'app/lib/pre_assembly/digital_object.rb' - # Offense count: 10 Metrics/AbcSize: Max: 74 diff --git a/app/lib/pre_assembly/digital_object.rb b/app/lib/pre_assembly/digital_object.rb index 4aaa3f88..9ea3bcfc 100644 --- a/app/lib/pre_assembly/digital_object.rb +++ b/app/lib/pre_assembly/digital_object.rb @@ -15,7 +15,6 @@ class DigitalObject attr_accessor :container, :content_md_xml, - :dor_object, :label, :manifest_row, :object_files, From 1614c8fc697aace4adef7571c360c8f8e997afe7 Mon Sep 17 00:00:00 2001 From: Joe Atzberger Date: Thu, 27 Sep 2018 16:40:29 -0700 Subject: [PATCH 8/8] Manual corrections, mostly RSpec. TODO now under 100 lines, good enough --- .rubocop_todo.yml | 29 ------------------- app/lib/pre_assembly/smpl.rb | 8 ++--- .../bundle_contexts_controller_spec.rb | 2 +- spec/lib/pre_assembly/bundle_spec.rb | 12 ++++---- spec/lib/pre_assembly/object_file_spec.rb | 8 ++--- spec/models/job_run_spec.rb | 4 +-- .../bundle_contexts/new.html.erb_spec.rb | 4 +-- .../job_runs/index.json.jbuilder_spec.rb | 4 +-- 8 files changed, 19 insertions(+), 52 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 0a9c0b97..e0001a73 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -76,51 +76,22 @@ RSpec/ExampleLength: Exclude: - 'spec/lib/pre_assembly/bundle_spec.rb' -# Offense count: 1 -# Configuration parameters: AssignmentOnly. -RSpec/InstanceVariable: - Exclude: - - 'spec/lib/pre_assembly/object_file_spec.rb' - -# Offense count: 2 -RSpec/LetSetup: - Exclude: - - 'spec/views/bundle_contexts/new.html.erb_spec.rb' - - '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 Style/ClassVars: Exclude: - 'app/lib/pre_assembly/logging.rb' -# Offense count: 6 -Style/CommentedKeyword: - Exclude: - - 'app/lib/pre_assembly/smpl.rb' - # Offense count: 19 Style/Documentation: Enabled: false diff --git a/app/lib/pre_assembly/smpl.rb b/app/lib/pre_assembly/smpl.rb index 4afb2b60..7864c3b1 100644 --- a/app/lib/pre_assembly/smpl.rb +++ b/app/lib/pre_assembly/smpl.rb @@ -110,10 +110,10 @@ def generate_cm(druid) xml.checksum(checksum, type: 'md5') if checksum && checksum != '' end end - end # end resource - end # end loop over resources - end # end CM tag - end # end XML tag + end + end + end + end FileUtils.cd(current_directory) builder.to_xml end diff --git a/spec/controllers/bundle_contexts_controller_spec.rb b/spec/controllers/bundle_contexts_controller_spec.rb index cbb5721d..fbdd27e0 100644 --- a/spec/controllers/bundle_contexts_controller_spec.rb +++ b/spec/controllers/bundle_contexts_controller_spec.rb @@ -23,7 +23,7 @@ before { sign_in(create(:user)) } it 'has current_user' do - expect(subject.current_user).not_to be_nil + expect(controller.current_user).not_to be_nil end context 'GET new' do diff --git a/spec/lib/pre_assembly/bundle_spec.rb b/spec/lib/pre_assembly/bundle_spec.rb index 1ee88386..ae1d58f2 100644 --- a/spec/lib/pre_assembly/bundle_spec.rb +++ b/spec/lib/pre_assembly/bundle_spec.rb @@ -8,7 +8,9 @@ let(:exp_workflow_svc_url) { Regexp.new("^#{Dor::Config.dor_services.url}/objects/.*/apo_workflows/assemblyWF$") } before do - allow(RestClient).to receive(:post).with(a_string_matching(exp_workflow_svc_url), {}).and_return(instance_double(RestClient::Response, code: 200)) + allow(RestClient).to receive(:post) + .with(a_string_matching(exp_workflow_svc_url), {}) + .and_return(instance_double(RestClient::Response, code: 200)) allow(Dor::Item).to receive(:find).with(any_args) end @@ -19,9 +21,7 @@ bc.save b = described_class.new bc pids = [] - expect do - pids = b.run_pre_assembly - end.not_to raise_error + expect { pids = b.run_pre_assembly }.not_to raise_error expect(pids).to eq ['druid:jy812bp9403', 'druid:tz250tk7584', 'druid:gn330dv6119'] end end @@ -44,7 +44,7 @@ describe '#processed_pids' do it 'pulls pids from digital_objects' do exp_pids = [11, 22, 33] - flat_dir_images.digital_objects = exp_pids.map { |p| double('dobj', pid: p) } + flat_dir_images.digital_objects = exp_pids.map { |p| instance_double(PreAssembly::DigitalObject, pid: p) } expect(flat_dir_images.processed_pids).to eq(exp_pids) end end @@ -112,7 +112,7 @@ describe 'object discovery: other' do it 'is able to exercise all_object_files()' do fake_files = [[1, 2], [3, 4], [5, 6]] - fake_dobjs = fake_files.map { |fs| double('dobj', object_files: fs) } + fake_dobjs = fake_files.map { |fs| instance_double(PreAssembly::DigitalObject, object_files: fs) } flat_dir_images.digital_objects = fake_dobjs expect(flat_dir_images.all_object_files).to eq(fake_files.flatten) end diff --git a/spec/lib/pre_assembly/object_file_spec.rb b/spec/lib/pre_assembly/object_file_spec.rb index af66f36a..c96f22ff 100644 --- a/spec/lib/pre_assembly/object_file_spec.rb +++ b/spec/lib/pre_assembly/object_file_spec.rb @@ -1,13 +1,9 @@ RSpec.describe PreAssembly::ObjectFile do - before do - @f = described_class.new( - path: 'spec/test_data/flat_dir_images/image1.tif' - ) - end + let(:object_file) { described_class.new(path: 'spec/test_data/flat_dir_images/image1.tif') } describe 'initialization' do it 'can initialize an ObjectFile' do - expect(@f).to be_kind_of described_class + expect(object_file).to be_a(described_class) # useless test ("Does Ruby Work??") end end end diff --git a/spec/models/job_run_spec.rb b/spec/models/job_run_spec.rb index 2e02650d..9c443315 100644 --- a/spec/models/job_run_spec.rb +++ b/spec/models/job_run_spec.rb @@ -19,8 +19,8 @@ end describe 'send_notification' do - let(:mock_mailer) { double JobMailer } - let(:mock_delivery) { double ActionMailer::MessageDelivery } + let(:mock_mailer) { instance_double JobMailer } + let(:mock_delivery) { instance_double ActionMailer::MessageDelivery } before { job_run.save } diff --git a/spec/views/bundle_contexts/new.html.erb_spec.rb b/spec/views/bundle_contexts/new.html.erb_spec.rb index d6fdd599..23bda2f5 100644 --- a/spec/views/bundle_contexts/new.html.erb_spec.rb +++ b/spec/views/bundle_contexts/new.html.erb_spec.rb @@ -1,9 +1,9 @@ RSpec.describe 'bundle_contexts/new.html.erb', type: :view do - let!(:job_runs) { create_list(:job_run, 2) } + let(:job_runs) { create_list(:job_run, 2) } it 'displays a list of job_runs in side panel' do assign(:bundle_context, BundleContext.new) - assign(:job_runs, JobRun.all.page(1)) + assign(:job_runs, job_runs) render expect(rendered).to include('') # the element we will render into expect(rendered).to include('