Lint: Fix various offenses, mostly rspec

- Fix Lint/RescueWithoutErrorClass
  - And I use the term "fix" loosely
- Fix RSpec/PredicateMatcher
  - The failure message should actually be better this way too.
- RSpec/LetBeforeExamples
- RSpec/HookArgument
- Fix RSpec/ExpectInHook in widget_spec.rb
- Fix RSpec/MessageSpies
  - I like spies. I've never linted them before, but I like how they
    separate test setup from assertions.

.rubocop_todo.yml

@@ -3,9 +3,6 @@ require: rubocop-rspec
 # Remove these configuration records
 # one by one as the offenses are removed from the code base.
 
-Lint/RescueWithoutErrorClass:
-  Enabled: false
-
 Metrics/AbcSize:
   Max: 22 # Goal: 15
 
@@ -22,24 +19,21 @@ RSpec/BeforeAfterAll:
   Enabled: false
 
 RSpec/ExpectInHook:
-  Enabled: false
+  Exclude:
+    - spec/paper_trail/associations_spec.rb
+    - spec/models/version_spec.rb
 
 RSpec/FilePath:
   Enabled: false
 
-RSpec/HookArgument:
-  Enabled: false
-
 RSpec/InstanceVariable:
   Exclude:
     - spec/paper_trail/associations_spec.rb
     - spec/paper_trail/model_spec.rb
 
-RSpec/LetBeforeExamples:
-  Enabled: false
-
 RSpec/MessageSpies:
-  Enabled: false
+  Exclude:
+    - spec/models/version_spec.rb
 
 RSpec/NamedSubject:
   Enabled: false
@@ -49,8 +43,5 @@ RSpec/NestedGroups:
     - spec/paper_trail/associations_spec.rb
     - spec/paper_trail/model_spec.rb
 
-RSpec/PredicateMatcher:
-  Enabled: false
-
 Security/YAMLLoad:
   Enabled: false

lib/paper_trail/record_trail.rb

@@ -170,7 +170,7 @@ module PaperTrail
     def next_version
       subsequent_version = source_version.next
       subsequent_version ? subsequent_version.reify : @record.class.find(@record.id)
-    rescue # TODO: Rescue something more specific
+    rescue StandardError # TODO: Rescue something more specific
       nil
     end
 

lib/paper_trail/version_concern.rb

@@ -162,7 +162,7 @@ module PaperTrail
 
       def primary_key_is_int?
         @primary_key_is_int ||= columns_hash[primary_key].type == :integer
-      rescue
+      rescue StandardError # TODO: Rescue something more specific
         true
       end
 
@@ -307,7 +307,7 @@ module PaperTrail
       else
         begin
           PaperTrail.serializer.load(object_changes)
-        rescue # TODO: Rescue something specific
+        rescue StandardError # TODO: Rescue something more specific
           {}
         end
       end

spec/models/gadget_spec.rb

@@ -1,10 +1,10 @@
 require "spec_helper"
 
 RSpec.describe Gadget, type: :model do
-  it { is_expected.to be_versioned }
-
   let(:gadget) { Gadget.create!(name: "Wrench", brand: "Acme") }
 
+  it { is_expected.to be_versioned }
+
   describe "updates", versioning: true do
     it "generates a version for updates to `name` attribute" do
       expect { gadget.update_attribute(:name, "Hammer") }.to(change { gadget.versions.size }.by(1))

spec/models/version_spec.rb

@@ -65,15 +65,16 @@ module PaperTrail
       it "sets the invoke `paper_trail_originator`" do
         allow(ActiveSupport::Deprecation).to receive(:warn)
         subject = PaperTrail::Version.new
-        expect(subject).to receive(:paper_trail_originator)
+        allow(subject).to receive(:paper_trail_originator)
         subject.originator
+        expect(subject).to have_received(:paper_trail_originator)
       end
 
       it "displays a deprecation warning" do
-        expect(ActiveSupport::Deprecation).to receive(:warn).
+        allow(ActiveSupport::Deprecation).to receive(:warn)
+        PaperTrail::Version.new.originator
+        expect(ActiveSupport::Deprecation).to have_received(:warn).
           with(/Use paper_trail_originator instead of originator/)
-        subject = PaperTrail::Version.new
-        subject.originator
       end
     end
 

spec/models/widget_spec.rb

@@ -1,12 +1,12 @@
 require "spec_helper"
 
 RSpec.describe Widget, type: :model do
+  let(:widget) { Widget.create! name: "Bob", an_integer: 1 }
+
   describe "`be_versioned` matcher" do
     it { is_expected.to be_versioned }
   end
 
-  let(:widget) { Widget.create! name: "Bob", an_integer: 1 }
-
   describe "`have_a_version_with` matcher", versioning: true do
     before do
       widget.update_attributes!(name: "Leonard", an_integer: 1)
@@ -145,8 +145,9 @@ RSpec.describe Widget, type: :model do
       it "does not clobber the IdentityMap when reifying" do
         widget.update_attributes name: "Henry", created_at: Time.now - 1.day
         widget.update_attributes name: "Harry"
-        expect(ActiveRecord::IdentityMap).to receive(:without).once
+        allow(ActiveRecord::IdentityMap).to receive(:without)
         widget.versions.last.reify
+        expect(ActiveRecord::IdentityMap).to have_receive(:without).once
       end
     end
   end
@@ -249,10 +250,11 @@ RSpec.describe Widget, type: :model do
 
       before do
         PaperTrail.whodunnit = orig_name
-        expect(widget.versions.last.whodunnit).to eq(orig_name) # persist `widget`
+        widget # persist `widget` (call the `let`)
       end
 
       it "modifies value of `PaperTrail.whodunnit` while executing the block" do
+        expect(widget.versions.last.whodunnit).to eq(orig_name)
         widget.paper_trail.whodunnit(new_name) do
           expect(PaperTrail.whodunnit).to eq(new_name)
           widget.update_attributes(name: "Elizabeth")
@@ -261,6 +263,7 @@ RSpec.describe Widget, type: :model do
       end
 
       it "reverts value of whodunnit to previous value after executing the block" do
+        expect(widget.versions.last.whodunnit).to eq(orig_name)
         widget.paper_trail.whodunnit(new_name) { |w|
           w.update_attributes(name: "Elizabeth")
         }
@@ -268,6 +271,7 @@ RSpec.describe Widget, type: :model do
       end
 
       it "reverts to previous value, even if error within block" do
+        expect(widget.versions.last.whodunnit).to eq(orig_name)
         expect {
           widget.paper_trail.whodunnit(new_name) { raise }
         }.to raise_error(RuntimeError)

spec/paper_trail/model_spec.rb

@@ -84,9 +84,9 @@ RSpec.describe(::PaperTrail, versioning: true) do
         end
 
         it "have versions that are not live" do
-          expect(
+          @widget.versions.map(&:reify).compact.each do |v|
-            @widget.versions.map(&:reify).compact.all? { |w| !w.paper_trail.live? }
+            expect(v.paper_trail).not_to be_live
-          ).to(be_truthy)
+          end
         end
 
         it "have stored changes" do

spec/spec_helper.rb

@@ -72,7 +72,7 @@ RSpec.configure do |config|
   # Truncation is about three times slower than transaction rollback, so it'll
   # be nice when we can drop support for rails < 5.
   if active_record_gem_version < ::Gem::Version.new("5")
-    config.before(:each) { DatabaseCleaner.start }
+    config.before { DatabaseCleaner.start }
-    config.after(:each) { DatabaseCleaner.clean }
+    config.after { DatabaseCleaner.clean }
   end
 end