Enable Rubocop and fix rspec warnings (#9)

.rspec

@@ -1,2 +1,3 @@
 --color
 --format progress
+--require spec_helper

.rubocop.yml

@@ -0,0 +1,8 @@
+inherit_gem:
+  salsify_rubocop: conf/rubocop.yml
+
+AllCops:
+  TargetRubyVersion: 2.3
+
+Style/FrozenStringLiteralComment:
+  Enabled: true

.travis.yml

@@ -1,11 +1,14 @@
 language: ruby
 sudo: false
 gemfile:
-- gemfiles/rails_4.2.gemfile
-- gemfiles/rails_5.0.gemfile
-- gemfiles/rails_5.1.gemfile
-- gemfiles/rails_5.2.gemfile
+  - gemfiles/rails_4.2.gemfile
+  - gemfiles/rails_5.0.gemfile
+  - gemfiles/rails_5.1.gemfile
+  - gemfiles/rails_5.2.gemfile
 rvm:
-- 2.3.8
-- 2.4.5
-- 2.5.3
+  - 2.3.8
+  - 2.4.5
+  - 2.5.3
+script:
+  - bundle exec rspec
+  - bundle exec rubocop

Gemfile

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 source 'https://rubygems.org'
 
 gemspec

Rakefile

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'bundler/setup'
 Bundler::GemHelper.install_tasks
 

delayed_job_groups.gemspec

@@ -1,4 +1,6 @@
-# encoding: UTF-8
+
+# frozen_string_literal: true
+
 lib = File.expand_path('../lib', __FILE__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 require 'delayed/job_groups/version'
@@ -8,12 +10,12 @@ Gem::Specification.new do |spec|
   spec.version       = Delayed::JobGroups::VERSION
   spec.authors       = ['Joel Turkel', 'Randy Burkes']
   spec.email         = ['jturkel@salsify.com', 'rlburkes@gmail.com']
-  spec.description   = %q{Aggregates Delayed::Job jobs into groups with group level management and lifecycle callbacks}
-  spec.summary       = %q{Delayed::Job job groups plugin}
+  spec.description   = 'Aggregates Delayed::Job jobs into groups with group level management and lifecycle callbacks'
+  spec.summary       = 'Delayed::Job job groups plugin'
   spec.homepage      = 'https://github.com/salsify/delayed_job_groups_plugin'
   spec.license       = 'MIT'
 
-  spec.files         = `git ls-files`.split($/)
+  spec.files         = `git ls-files`.split($INPUT_RECORD_SEPARATOR)
   spec.test_files    = Dir.glob('spec/**/*')
   spec.require_paths = ['lib']
 
@@ -28,11 +30,12 @@ Gem::Specification.new do |spec|
   spec.add_dependency 'activerecord', '>= 4.2', '< 5.3'
   spec.add_development_dependency 'coveralls'
   spec.add_development_dependency 'database_cleaner', '>= 1.2'
+  spec.add_development_dependency 'mime-types'
   spec.add_development_dependency 'rake'
   spec.add_development_dependency 'rspec', '~> 3'
   spec.add_development_dependency 'rspec-its'
+  spec.add_development_dependency 'salsify_rubocop', '0.52.1.1'
   spec.add_development_dependency 'simplecov'
-  spec.add_development_dependency 'timecop'
-  spec.add_development_dependency 'mime-types'
   spec.add_development_dependency 'sqlite3'
+  spec.add_development_dependency 'timecop'
 end

lib/delayed/job_groups/compatibility.rb

@@ -1,4 +1,4 @@
-# encoding: UTF-8
+# frozen_string_literal: true
 
 require 'active_support/version'
 require 'active_record/version'
@@ -8,7 +8,7 @@ module JobGroups
     module Compatibility
 
       def self.mass_assignment_security_enabled?
-        ::ActiveRecord::VERSION::MAJOR < 4 || defined?(::ActiveRecord::MassAssignmentSecurity)
+        defined?(::ActiveRecord::MassAssignmentSecurity)
       end
 
     end

lib/delayed/job_groups/job_extensions.rb

@@ -1,4 +1,4 @@
-# encoding: UTF-8
+# frozen_string_literal: true
 
 module Delayed
   module JobGroups

lib/delayed/job_groups/job_group.rb

@@ -1,4 +1,4 @@
-# encoding: UTF-8
+# frozen_string_literal: true
 
 module Delayed
   module JobGroups
@@ -18,22 +18,12 @@ class JobGroup < ActiveRecord::Base
 
       validates :queueing_complete, :blocked, :failure_cancels_group, inclusion: [true, false]
 
-      if ActiveRecord::VERSION::MAJOR >= 4
-        has_many :active_jobs, -> { where(failed_at: nil) }, class_name: '::Delayed::Job'
-      else
-        has_many :active_jobs, class_name: Job, conditions: {failed_at: nil}
-      end
-
+      has_many :active_jobs, -> { where(failed_at: nil) }, class_name: '::Delayed::Job'
 
       # Only delete dependent jobs that are unlocked so we can determine if there are in-flight jobs
       # for canceled job groups
-      if ActiveRecord::VERSION::MAJOR >= 4
-        has_many :queued_jobs, -> { where(failed_at: nil, locked_by: nil) }, class_name: '::Delayed::Job',
-                 dependent: :delete_all
-      else
-        has_many :queued_jobs, class_name: Job, conditions: {failed_at: nil, locked_by: nil},
-                 dependent: :delete_all
-      end
+      has_many :queued_jobs, -> { where(failed_at: nil, locked_by: nil) }, class_name: '::Delayed::Job',
+                dependent: :delete_all
 
       def mark_queueing_complete
         with_lock do
@@ -78,7 +68,7 @@ def self.check_for_completion(job_group_id)
         end
       end
 
-      def self.has_pending_jobs?(job_group_ids)
+      def self.has_pending_jobs?(job_group_ids) # rubocop:disable Naming/PredicateName
         job_group_ids = Array(job_group_ids)
         return false if job_group_ids.empty?
         Delayed::Job.where(job_group_id: job_group_ids, failed_at: nil).exists?

lib/delayed/job_groups/plugin.rb

@@ -1,4 +1,4 @@
-# encoding: UTF-8
+# frozen_string_literal: true
 
 require 'delayed_job'
 require 'set'
@@ -8,7 +8,7 @@ module JobGroups
     class Plugin < Delayed::Plugin
 
       callbacks do |lifecycle|
-        lifecycle.before(:error) do |worker, job|
+        lifecycle.before(:error) do |_worker, job|
           # If the job group has been cancelled then don't let the job be retried
           if job.in_job_group? && job_group_cancelled?(job.job_group_id)
             def job.max_attempts
@@ -17,7 +17,7 @@ def job.max_attempts
           end
         end
 
-        lifecycle.before(:failure) do |worker, job|
+        lifecycle.before(:failure) do |_worker, job|
           # If a job in the job group fails, then cancel the whole job group.
           # Need to check that the job group is present since another
           # job may have concurrently cancelled it.
@@ -26,7 +26,7 @@ def job.max_attempts
           end
         end
 
-        lifecycle.after(:perform) do |worker, job|
+        lifecycle.after(:perform) do |_worker, job|
           # Make sure we only check to see if the job group is complete
           # if the job succeeded
           if job.in_job_group? && job_completed?(job)
@@ -35,8 +35,6 @@ def job.max_attempts
         end
       end
 
-      private
-
       def self.job_group_cancelled?(job_group_id)
         !JobGroup.exists?(job_group_id)
       end
@@ -49,4 +47,3 @@ def self.job_completed?(job)
     end
   end
 end
-

lib/delayed/job_groups/version.rb

@@ -1,4 +1,4 @@
-# encoding: UTF-8
+# frozen_string_literal: true
 
 module Delayed
   module JobGroups

lib/delayed_job_groups_plugin.rb

@@ -1,4 +1,4 @@
-# encoding: UTF-8
+# frozen_string_literal: true
 
 require 'active_support'
 require 'active_record'

lib/generators/delayed_job_groups_plugin/install_generator.rb

@@ -1,4 +1,4 @@
-# encoding: UTF-8
+# frozen_string_literal: true
 
 require 'rails/generators'
 require 'rails/generators/migration'
@@ -8,7 +8,7 @@ module DelayedJobGroupsPlugin
   class InstallGenerator < Rails::Generators::Base
     include Rails::Generators::Migration
 
-    self.source_paths << File.join(File.dirname(__FILE__), 'templates')
+    source_paths << File.join(File.dirname(__FILE__), 'templates')
 
     def create_migration_file
       migration_template('migration.rb', 'db/migrate/create_delayed_job_groups.rb')

lib/generators/delayed_job_groups_plugin/templates/migration.rb

@@ -1,4 +1,4 @@
-# encoding: UTF-8
+# frozen_string_literal: true
 
 class CreateDelayedJobGroups < ActiveRecord::Migration
 

spec/db/schema.rb

@@ -1,6 +1,6 @@
-# encoding: UTF-8
+# frozen_string_literal: true
 
-ActiveRecord::Schema.define(:version => 0) do
+ActiveRecord::Schema.define(version: 0) do
 
   create_table(:delayed_jobs, force: true) do |t|
     t.integer :priority, default: 0

spec/delayed/job_groups/job_group_spec.rb

@@ -1,6 +1,4 @@
-# encoding: UTF-8
-
-require 'spec_helper'
+# frozen_string_literal: true
 
 describe Delayed::JobGroups::JobGroup do
 
@@ -19,7 +17,7 @@
 
   before do
     Timecop.freeze(current_time)
-    Delayed::Job.stub(:enqueue)
+    allow(Delayed::Job).to receive(:enqueue)
   end
 
   after do
@@ -28,21 +26,21 @@
 
   shared_examples "the job group was completed" do
     it "queues the completion job" do
-      Delayed::Job.should have_received(:enqueue).with(on_completion_job, on_completion_job_options)
+      expect(Delayed::Job).to have_received(:enqueue).with(on_completion_job, on_completion_job_options)
     end
 
     it "destroys the job group" do
-      job_group.should have_been_destroyed
+      expect(job_group).to have_been_destroyed
     end
   end
 
   shared_examples "the job group was not completed" do
     it "does not queue the completion job" do
-      Delayed::Job.should_not have_received(:enqueue)
+      expect(Delayed::Job).not_to have_received(:enqueue)
     end
 
     it "does not destroy the job group" do
-      job_group.should_not have_been_destroyed
+      expect(job_group).not_to have_been_destroyed
     end
   end
 
@@ -51,15 +49,16 @@
     context "when no jobs exist" do
       before { job_group.mark_queueing_complete }
 
-      it { should be_queueing_complete }
+      it { is_expected.to be_queueing_complete }
       it_behaves_like "the job group was completed"
     end
 
     context "when no jobs exist but the job group is blocked" do
       let(:blocked) { true }
+
       before { job_group.mark_queueing_complete }
 
-      it { should be_queueing_complete }
+      it { is_expected.to be_queueing_complete }
       it_behaves_like "the job group was not completed"
     end
 
@@ -69,7 +68,7 @@
         job_group.mark_queueing_complete
       end
 
-      it { should be_queueing_complete }
+      it { is_expected.to be_queueing_complete }
       it_behaves_like "the job group was not completed"
     end
   end
@@ -117,11 +116,11 @@
       include_context "complete job and check job group complete"
 
       it "queues the completion job with empty options" do
-        Delayed::Job.should have_received(:enqueue).with(on_completion_job, {})
+        expect(Delayed::Job).to have_received(:enqueue).with(on_completion_job, {})
       end
 
       it "destroys the job group" do
-        job_group.should have_been_destroyed
+        expect(job_group).to have_been_destroyed
       end
     end
 
@@ -131,11 +130,11 @@
       include_context "complete job and check job group complete"
 
       it "doesn't queues the non-existent completion job" do
-        Delayed::Job.should_not have_received(:enqueue)
+        expect(Delayed::Job).not_to have_received(:enqueue)
       end
 
       it "destroys the job group" do
-        job_group.should have_been_destroyed
+        expect(job_group).to have_been_destroyed
       end
     end
   end
@@ -149,7 +148,7 @@
 
     shared_examples "it enqueues the job in the correct blocked state" do
       it "enqueues the job in the same blocked state as the job group" do
-        Delayed::Job.should have_received(:enqueue).with(job, job_group_id: job_group.id, blocked: blocked)
+        expect(Delayed::Job).to have_received(:enqueue).with(job, job_group_id: job_group.id, blocked: blocked)
       end
     end
 
@@ -169,7 +168,7 @@
         job_group.unblock
       end
 
-      its(:blocked?) { should be(false) }
+      its(:blocked?) { is_expected.to be(false) }
     end
 
     context "when the JobGroup is blocked" do
@@ -184,10 +183,10 @@
           Timecop.freeze(unblock_time) { job_group.unblock }
         end
 
-        its(:blocked?) { should be(false) }
+        its(:blocked?) { is_expected.to be(false) }
 
         it "sets the job's run_at to the unblocked time" do
-          job.reload.run_at.should eq unblock_time
+          expect(job.reload.run_at).to eq unblock_time
         end
 
         it_behaves_like "the job group was not completed"
@@ -199,7 +198,7 @@
           job_group.unblock
         end
 
-        its(:blocked?) { should be(false) }
+        its(:blocked?) { is_expected.to be(false) }
         it_behaves_like "the job group was completed"
       end
     end
@@ -214,15 +213,15 @@
     end
 
     it "destroys the job group" do
-      job_group.should have_been_destroyed
+      expect(job_group).to have_been_destroyed
     end
 
     it "destroys queued jobs" do
-      queued_job.should have_been_destroyed
+      expect(queued_job).to have_been_destroyed
     end
 
     it "does not destroy running jobs" do
-      running_job.should_not have_been_destroyed
+      expect(running_job).not_to have_been_destroyed
     end
   end