diff --git a/.rubocop.yml b/.rubocop.yml index 9e1f51b7..7760f7f0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -54,6 +54,12 @@ Metrics/ModuleLength: RSpec/MultipleExpectations: Enabled: false +# Yes, ideally examples would be short. Is it possible to pick a limit and say, +# "no example will ever be longer than this"? Hard to say. Sometimes they're +# quite long. +RSpec/ExampleLength: + Enabled: false + Style/AlignParameters: EnforcedStyle: with_fixed_indentation diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index fb310989..81319ae0 100644 --- a/.rubocop_todo.yml +++ b/.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/AmbiguousBlockAssociation: - Enabled: false - Metrics/AbcSize: Max: 22 # Goal: 15 @@ -21,18 +18,6 @@ Style/FrozenStringLiteralComment: RSpec/BeforeAfterAll: Enabled: false -RSpec/DescribedClass: - Enabled: false - -RSpec/ExampleWording: - Enabled: false - -RSpec/ExampleLength: - Enabled: false - -RSpec/ExpectActual: - Enabled: false - RSpec/FilePath: Enabled: false @@ -43,10 +28,7 @@ RSpec/NamedSubject: Enabled: false RSpec/NestedGroups: - Enabled: false - -RSpec/NotToNot: - Enabled: false + Max: 5 # goal: 3 Security/YAMLLoad: Enabled: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 30dca1f5..986d1742 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). ### Added -- None +- `PaperTrail.gem_version` returns a `Gem::Version`, nice for comparisons. ### Fixed diff --git a/lib/paper_trail.rb b/lib/paper_trail.rb index c5d1c0d3..4f6815d1 100644 --- a/lib/paper_trail.rb +++ b/lib/paper_trail.rb @@ -62,6 +62,13 @@ module PaperTrail !!paper_trail_store.fetch(:"enabled_for_#{model}", true) end + # Returns a `::Gem::Version`, convenient for comparisons. This is + # recommended over `::PaperTrail::VERSION::STRING`. + # @api public + def gem_version + ::Gem::Version.new(VERSION::STRING) + end + # Set the field which records when a version was created. # @api public def timestamp_field=(_field_name) diff --git a/lib/paper_trail/version_number.rb b/lib/paper_trail/version_number.rb index 1e86322c..c9b90afe 100644 --- a/lib/paper_trail/version_number.rb +++ b/lib/paper_trail/version_number.rb @@ -1,5 +1,9 @@ module PaperTrail - # :nodoc: + # The version number of the paper_trail gem. Not to be confused with + # `PaperTrail::Version`. Ruby constants are case-sensitive, apparently, + # and they are two different modules! It would be nice to remove `VERSION`, + # because of this confusion, but it's not worth the breaking change. + # People are encouraged to use `PaperTrail.gem_version` instead. module VERSION MAJOR = 6 MINOR = 0 diff --git a/spec/controllers/articles_controller_spec.rb b/spec/controllers/articles_controller_spec.rb index 7cc1cc31..1d6b030e 100644 --- a/spec/controllers/articles_controller_spec.rb +++ b/spec/controllers/articles_controller_spec.rb @@ -8,7 +8,7 @@ RSpec.describe ArticlesController, type: :controller do it "returns true" do assert PaperTrail.enabled? post :create, params_wrapper(article: { title: "Doh", content: FFaker::Lorem.sentence }) - expect(assigns(:article)).to_not be_nil + expect(assigns(:article)).not_to be_nil assert PaperTrail.enabled_for_controller? assert_equal 1, assigns(:article).versions.length end diff --git a/spec/generators/install_generator_spec.rb b/spec/generators/install_generator_spec.rb index ae148487..ab0e1338 100644 --- a/spec/generators/install_generator_spec.rb +++ b/spec/generators/install_generator_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" require "generator_spec/test_case" require File.expand_path("../../../lib/generators/paper_trail/install_generator", __FILE__) -describe PaperTrail::InstallGenerator, type: :generator do +RSpec.describe PaperTrail::InstallGenerator, type: :generator do include GeneratorSpec::TestCase destination File.expand_path("../tmp", __FILE__) @@ -15,17 +15,19 @@ describe PaperTrail::InstallGenerator, type: :generator do end it "generates a migration for creating the 'versions' table" do - expect(destination_root).to have_structure { - directory "db" do - directory "migrate" do - migration "create_versions" do - contains "class CreateVersions" - contains "def change" - contains "create_table :versions" - end - end - end - } + expect(destination_root).to( + have_structure { + directory("db") { + directory("migrate") { + migration("create_versions") { + contains "class CreateVersions" + contains "def change" + contains "create_table :versions" + } + } + } + } + ) end end @@ -36,31 +38,35 @@ describe PaperTrail::InstallGenerator, type: :generator do end it "generates a migration for creating the 'versions' table" do - expect(destination_root).to have_structure { - directory "db" do - directory "migrate" do - migration "create_versions" do - contains "class CreateVersions" - contains "def change" - contains "create_table :versions" - end - end - end - } + expect(destination_root).to( + have_structure { + directory("db") { + directory("migrate") { + migration("create_versions") { + contains "class CreateVersions" + contains "def change" + contains "create_table :versions" + } + } + } + } + ) end it "generates a migration for adding the 'object_changes' column to the 'versions' table" do - expect(destination_root).to have_structure { - directory "db" do - directory "migrate" do - migration "add_object_changes_to_versions" do - contains "class AddObjectChangesToVersions" - contains "def change" - contains "add_column :versions, :object_changes, :text" - end - end - end - } + expect(destination_root).to( + have_structure { + directory("db") { + directory("migrate") { + migration("add_object_changes_to_versions") { + contains "class AddObjectChangesToVersions" + contains "def change" + contains "add_column :versions, :object_changes, :text" + } + } + } + } + ) end end end diff --git a/spec/models/boolit_spec.rb b/spec/models/boolit_spec.rb index c5d6fb93..ce8c6dcf 100644 --- a/spec/models/boolit_spec.rb +++ b/spec/models/boolit_spec.rb @@ -5,7 +5,7 @@ describe Boolit, type: :model do it { is_expected.to be_versioned } it "has a default scope" do - expect(subject.default_scopes).to_not be_empty + expect(subject.default_scopes).not_to be_empty end describe "Versioning", versioning: true do @@ -13,12 +13,12 @@ describe Boolit, type: :model do before { subject.update_attributes!(name: FFaker::Name.name) } - it "should have versions" do + it "has two versions" do expect(subject.versions.size).to eq(2) end - it "should be able to be reified and persisted" do - expect { subject.versions.last.reify.save! }.to_not raise_error + it "can be reified and persisted" do + expect { subject.versions.last.reify.save! }.not_to raise_error end context "Instance falls out of default scope" do @@ -28,8 +28,8 @@ describe Boolit, type: :model do expect(Boolit.first).to be_nil end - it "should still be able to be reified and persisted" do - expect { subject.paper_trail.previous_version.save! }.to_not raise_error + it "still can be reified and persisted" do + expect { subject.paper_trail.previous_version.save! }.not_to raise_error end context "with `nil` attributes on the live instance" do @@ -40,7 +40,7 @@ describe Boolit, type: :model do end after { PaperTrail.serializer = PaperTrail::Serializers::YAML } - it "should not overwrite that attribute during the reification process" do + it "does not overwrite that attribute during the reification process" do expect(subject.paper_trail.previous_version.name).to be_nil end end diff --git a/spec/models/callback_modifier_spec.rb b/spec/models/callback_modifier_spec.rb index 9956ad7a..87c1ebde 100644 --- a/spec/models/callback_modifier_spec.rb +++ b/spec/models/callback_modifier_spec.rb @@ -4,13 +4,13 @@ describe CallbackModifier, type: :model do with_versioning do describe "callback-methods", versioning: true do describe "paper_trail_on_destroy" do - it "should add :destroy to paper_trail_options[:on]" do + it "adds :destroy to paper_trail_options[:on]" do modifier = NoArgDestroyModifier.create!(some_content: FFaker::Lorem.sentence) expect(modifier.paper_trail_options[:on]).to eq [:destroy] end context "when :before" do - it "should create the version before destroy" do + it "creates the version before destroy" do modifier = BeforeDestroyModifier.create!(some_content: FFaker::Lorem.sentence) modifier.test_destroy expect(modifier.versions.last.reify).not_to be_flagged_deleted @@ -18,7 +18,7 @@ describe CallbackModifier, type: :model do end context "when :after" do - it "should create the version after destroy" do + it "creates the version after destroy" do modifier = AfterDestroyModifier.create!(some_content: FFaker::Lorem.sentence) modifier.test_destroy expect(modifier.versions.last.reify).to be_flagged_deleted @@ -26,7 +26,7 @@ describe CallbackModifier, type: :model do end context "when no argument" do - it "should default to before destroy" do + it "defaults to before destroy" do modifier = NoArgDestroyModifier.create!(some_content: FFaker::Lorem.sentence) modifier.test_destroy expect(modifier.versions.last.reify).not_to be_flagged_deleted @@ -35,12 +35,12 @@ describe CallbackModifier, type: :model do end describe "paper_trail_on_update" do - it "should add :update to paper_trail_options[:on]" do + it "adds :update to paper_trail_options[:on]" do modifier = UpdateModifier.create!(some_content: FFaker::Lorem.sentence) expect(modifier.paper_trail_options[:on]).to eq [:update] end - it "should create a version" do + it "creates a version" do modifier = UpdateModifier.create!(some_content: FFaker::Lorem.sentence) modifier.update_attributes! some_content: "modified" expect(modifier.versions.last.event).to eq "update" @@ -48,36 +48,36 @@ describe CallbackModifier, type: :model do end describe "paper_trail_on_create" do - it "should add :create to paper_trail_options[:on]" do + it "adds :create to paper_trail_options[:on]" do modifier = CreateModifier.create!(some_content: FFaker::Lorem.sentence) expect(modifier.paper_trail_options[:on]).to eq [:create] end - it "should create a version" do + it "creates a version" do modifier = CreateModifier.create!(some_content: FFaker::Lorem.sentence) expect(modifier.versions.last.event).to eq "create" end end context "when no callback-method used" do - it "should set paper_trail_options[:on] to [:create, :update, :destroy]" do + it "sets paper_trail_options[:on] to [:create, :update, :destroy]" do modifier = DefaultModifier.create!(some_content: FFaker::Lorem.sentence) expect(modifier.paper_trail_options[:on]).to eq %i(create update destroy) end - it "should default to track destroy" do + it "tracks destroy" do modifier = DefaultModifier.create!(some_content: FFaker::Lorem.sentence) modifier.destroy expect(modifier.versions.last.event).to eq "destroy" end - it "should default to track update" do + it "tracks update" do modifier = DefaultModifier.create!(some_content: FFaker::Lorem.sentence) modifier.update_attributes! some_content: "modified" expect(modifier.versions.last.event).to eq "update" end - it "should default to track create" do + it "tracks create" do modifier = DefaultModifier.create!(some_content: FFaker::Lorem.sentence) expect(modifier.versions.last.event).to eq "create" end diff --git a/spec/models/gadget_spec.rb b/spec/models/gadget_spec.rb index fa2e5074..5f2a217d 100644 --- a/spec/models/gadget_spec.rb +++ b/spec/models/gadget_spec.rb @@ -6,61 +6,57 @@ describe Gadget, type: :model do let(:gadget) { Gadget.create!(name: "Wrench", brand: "Acme") } describe "updates", versioning: true do - it "should generate a version for updates to `name` attribute" do - expect { gadget.update_attribute(:name, "Hammer").to change { gadget.versions.size }.by(1) } + it "generates a version for updates to `name` attribute" do + expect { gadget.update_attribute(:name, "Hammer") }.to(change { gadget.versions.size }.by(1)) end - it "should ignore for updates to `brand` attribute" do - expect { gadget.update_attribute(:brand, "Stanley") }.to_not change { gadget.versions.size } + it "ignores for updates to `brand` attribute" do + expect { gadget.update_attribute(:brand, "Stanley") }.not_to(change { gadget.versions.size }) end - it "should still generate a version when only the `updated_at` attribute is updated" do + it "still generates a version when only the `updated_at` attribute is updated" do # Plus 1 second because MySQL lacks sub-second resolution expect { gadget.update_attribute(:updated_at, Time.now + 1) - }.to change { gadget.versions.size }.by(1) + }.to(change { gadget.versions.size }.by(1)) end end - describe "Methods" do - describe "Instance", versioning: true do - describe "private" do - describe "#changed_notably?" do - subject { Gadget.new(created_at: Time.now) } + describe "#changed_notably?", versioning: true do + subject { Gadget.new(created_at: Time.now) } - context "create events" do - it { expect(subject.paper_trail.changed_notably?).to be true } - end + context "create events" do + it "returns true" do + expect(subject.paper_trail.changed_notably?).to eq(true) + end + end - context "update events" do - before { subject.save! } + context "update events" do + before { subject.save! } - context "without update timestamps" do - it "should only acknowledge non-ignored attrs" do - subject.name = "Wrench" - expect(subject.paper_trail.changed_notably?).to be true - end + context "without update timestamps" do + it "only acknowledges non-ignored attrs" do + subject.name = "Wrench" + expect(subject.paper_trail.changed_notably?).to be true + end - it "should not acknowledge ignored attr (brand)" do - subject.brand = "Acme" - expect(subject.paper_trail.changed_notably?).to be false - end - end + it "does not acknowledge ignored attr (brand)" do + subject.brand = "Acme" + expect(subject.paper_trail.changed_notably?).to be false + end + end - context "with update timestamps" do - it "should only acknowledge non-ignored attrs" do - subject.name = "Wrench" - subject.updated_at = Time.now - expect(subject.paper_trail.changed_notably?).to be true - end + context "with update timestamps" do + it "only acknowledges non-ignored attrs" do + subject.name = "Wrench" + subject.updated_at = Time.now + expect(subject.paper_trail.changed_notably?).to be true + end - it "should not acknowledge ignored attrs and timestamps only" do - subject.brand = "Acme" - subject.updated_at = Time.now - expect(subject.paper_trail.changed_notably?).to be false - end - end - end + it "does not acknowledge ignored attrs and timestamps only" do + subject.brand = "Acme" + subject.updated_at = Time.now + expect(subject.paper_trail.changed_notably?).to be false end end end diff --git a/spec/models/joined_version_spec.rb b/spec/models/joined_version_spec.rb index 11f6c77c..70ea8157 100644 --- a/spec/models/joined_version_spec.rb +++ b/spec/models/joined_version_spec.rb @@ -15,19 +15,19 @@ describe JoinedVersion, type: :model, versioning: true do before { widget } # persist a widget describe "#subsequent" do - it "shouldn't error out when there is a default_scope that joins" do + it "does not raise error when there is a default_scope that joins" do JoinedVersion.subsequent(version).first end end describe "#preceding" do - it "shouldn't error out when there is a default scope that joins" do + it "does not raise error when there is a default scope that joins" do JoinedVersion.preceding(version).first end end describe "#between" do - it "shouldn't error out when there is a default scope that joins" do + it "does not raise error when there is a default scope that joins" do JoinedVersion.between(Time.now, 1.minute.from_now).first end end @@ -38,7 +38,7 @@ describe JoinedVersion, type: :model, versioning: true do describe "#index" do it { is_expected.to respond_to(:index) } - it "shouldn't error out when there is a default scope that joins" do + it "does not raise error when there is a default scope that joins" do widget # persist a widget version.index end diff --git a/spec/models/json_version_spec.rb b/spec/models/json_version_spec.rb index 9eebf3f6..0715f4a7 100644 --- a/spec/models/json_version_spec.rb +++ b/spec/models/json_version_spec.rb @@ -3,16 +3,15 @@ require "rails_helper" # The `json_versions` table tests postgres' `json` data type. So, that # table is only created when testing against postgres and ActiveRecord >= 4. if JsonVersion.table_exists? - - describe JsonVersion, type: :model do - it "should include the `VersionConcern` module to get base functionality" do - expect(JsonVersion).to include(PaperTrail::VersionConcern) + RSpec.describe JsonVersion, type: :model do + it "includes the VersionConcern module" do + expect(described_class).to include(PaperTrail::VersionConcern) end describe "Methods" do describe "Class" do describe "#where_object" do - it { expect(JsonVersion).to respond_to(:where_object) } + it { expect(described_class).to respond_to(:where_object) } it "escapes values" do f = Fruit.create(name: "Bobby") @@ -25,9 +24,9 @@ if JsonVersion.table_exists? end context "invalid arguments" do - it "should raise an error" do - expect { JsonVersion.where_object(:foo) }.to raise_error(ArgumentError) - expect { JsonVersion.where_object([]) }.to raise_error(ArgumentError) + it "raises an error" do + expect { described_class.where_object(:foo) }.to raise_error(ArgumentError) + expect { described_class.where_object([]) }.to raise_error(ArgumentError) end end @@ -43,15 +42,15 @@ if JsonVersion.table_exists? fruit.update_attributes!(name: fruit_names.sample, color: FFaker::Color.name) end - it "should be able to locate versions according to their `object` contents" do - expect(JsonVersion.where_object(name: name)).to eq([fruit.versions[1]]) - expect(JsonVersion.where_object(color: color)).to eq([fruit.versions[2]]) + it "locates versions according to their `object` contents" do + expect(described_class.where_object(name: name)).to eq([fruit.versions[1]]) + expect(described_class.where_object(color: color)).to eq([fruit.versions[2]]) end end end describe "#where_object_changes" do - it { expect(JsonVersion).to respond_to(:where_object_changes) } + it { expect(described_class).to respond_to(:where_object_changes) } it "escapes values" do f = Fruit.create(name: "Bobby") @@ -64,9 +63,9 @@ if JsonVersion.table_exists? end context "invalid arguments" do - it "should raise an error" do - expect { JsonVersion.where_object_changes(:foo) }.to raise_error(ArgumentError) - expect { JsonVersion.where_object_changes([]) }.to raise_error(ArgumentError) + it "raises an error" do + expect { described_class.where_object_changes(:foo) }.to raise_error(ArgumentError) + expect { described_class.where_object_changes([]) }.to raise_error(ArgumentError) end end diff --git a/spec/models/not_on_update_spec.rb b/spec/models/not_on_update_spec.rb index 95304a1e..5306fd69 100644 --- a/spec/models/not_on_update_spec.rb +++ b/spec/models/not_on_update_spec.rb @@ -4,7 +4,7 @@ describe NotOnUpdate, type: :model do describe "#touch_with_version", versioning: true do let!(:record) { described_class.create! } - it "should create a version, regardless" do + it "creates a version, regardless" do expect { record.paper_trail.touch_with_version }.to change { PaperTrail::Version.count }.by(+1) diff --git a/spec/models/post_with_status_spec.rb b/spec/models/post_with_status_spec.rb index 3255373f..4f2bc805 100644 --- a/spec/models/post_with_status_spec.rb +++ b/spec/models/post_with_status_spec.rb @@ -4,7 +4,7 @@ describe PostWithStatus, type: :model do with_versioning do let(:post) { described_class.create!(status: "draft") } - it "should stash the enum value properly in versions" do + it "saves the enum value in versions" do post.published! post.archived! expect(post.paper_trail.previous_version.published?).to be true @@ -23,7 +23,7 @@ describe PostWithStatus, type: :model do context "storing enum object_changes" do subject { post.versions.last } - it "should stash the enum value properly in versions object_changes" do + it "saves the enum value properly in versions object_changes" do post.published! post.archived! expect(subject.changeset["status"]).to eql %w(published archived) diff --git a/spec/models/skipper_spec.rb b/spec/models/skipper_spec.rb index 6d7d43f8..02b48b0f 100644 --- a/spec/models/skipper_spec.rb +++ b/spec/models/skipper_spec.rb @@ -9,11 +9,11 @@ describe Skipper, type: :model do let(:t1) { Time.zone.local(2015, 7, 15, 20, 34, 0) } let(:t2) { Time.zone.local(2015, 7, 15, 20, 34, 30) } - it "should not create a version" do + it "does not create a version" do skipper = Skipper.create!(another_timestamp: t1) expect { skipper.update_attributes!(another_timestamp: t2) - }.to_not change { skipper.versions.length } + }.not_to(change { skipper.versions.length }) end end end @@ -24,7 +24,7 @@ describe Skipper, type: :model do let(:t2) { Time.zone.local(2015, 7, 15, 20, 34, 30) } context "without preserve (default)" do - it "should have no timestamp" do + it "has no timestamp" do skipper = Skipper.create!(another_timestamp: t1) skipper.update_attributes!(another_timestamp: t2, name: "Foobar") skipper = skipper.versions.last.reify @@ -33,7 +33,7 @@ describe Skipper, type: :model do end context "with preserve" do - it "should preserve its timestamp" do + it "preserves its timestamp" do skipper = Skipper.create!(another_timestamp: t1) skipper.update_attributes!(another_timestamp: t2, name: "Foobar") skipper = skipper.versions.last.reify(unversioned_attributes: :preserve) diff --git a/spec/models/thing_spec.rb b/spec/models/thing_spec.rb index 41636d13..d0c37415 100644 --- a/spec/models/thing_spec.rb +++ b/spec/models/thing_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" describe Thing, type: :model do it { is_expected.to be_versioned } - describe "should not store object_changes", versioning: true do + describe "does not store object_changes", versioning: true do let(:thing) { Thing.create(name: "pencil") } it { expect(thing.versions.last.object_changes).to be_nil } diff --git a/spec/models/truck_spec.rb b/spec/models/truck_spec.rb index 41089db0..56c45141 100644 --- a/spec/models/truck_spec.rb +++ b/spec/models/truck_spec.rb @@ -1,5 +1,5 @@ require "rails_helper" describe Truck, type: :model do - it { is_expected.to_not be_versioned } + it { is_expected.not_to be_versioned } end diff --git a/spec/models/vehicle_spec.rb b/spec/models/vehicle_spec.rb index 5845018c..f36051fa 100644 --- a/spec/models/vehicle_spec.rb +++ b/spec/models/vehicle_spec.rb @@ -1,5 +1,5 @@ require "rails_helper" describe Vehicle, type: :model do - it { is_expected.to_not be_versioned } + it { is_expected.not_to be_versioned } end diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 41b8d040..a7e426d3 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe PaperTrail::Version, type: :model do - it "should include the `VersionConcern` module to get base functionality" do + it "includes the VersionConcern module" do expect(PaperTrail::Version).to include(PaperTrail::VersionConcern) end @@ -13,7 +13,7 @@ describe PaperTrail::Version, type: :model do context "serializer is YAML" do specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::YAML } - it "should store out as a plain hash" do + it "store out as a plain hash" do expect(value =~ /HashWithIndifferentAccess/).to be_nil end end @@ -21,7 +21,7 @@ describe PaperTrail::Version, type: :model do context "serializer is JSON" do before(:all) { PaperTrail.serializer = PaperTrail::Serializers::JSON } - it "should store out as a plain hash" do + it "store out as a plain hash" do expect(value =~ /HashWithIndifferentAccess/).to be_nil end @@ -35,12 +35,10 @@ describe PaperTrail::Version, type: :model do subject { PaperTrail::Version.new } describe "#paper_trail_originator" do - it { is_expected.to respond_to(:paper_trail_originator) } - context "No previous versions" do specify { expect(subject.previous).to be_nil } - it "should return nil" do + it "return nil" do expect(subject.paper_trail_originator).to be_nil end end @@ -58,22 +56,20 @@ describe PaperTrail::Version, type: :model do specify { expect(subject.previous).to be_instance_of(PaperTrail::Version) } - it "should return nil" do + it "return nil" do expect(subject.paper_trail_originator).to eq(name) end end end describe "#originator" do - it { is_expected.to respond_to(:originator) } - - it "should set the invoke `paper_trail_originator`" do + it "sets the invoke `paper_trail_originator`" do allow(ActiveSupport::Deprecation).to receive(:warn) is_expected.to receive(:paper_trail_originator) subject.originator end - it "should display a deprecation warning" do + it "displays a deprecation warning" do expect(ActiveSupport::Deprecation).to receive(:warn). with(/Use paper_trail_originator instead of originator/) subject.originator @@ -85,188 +81,173 @@ describe PaperTrail::Version, type: :model do let(:attributes) { { whodunnit: FFaker::Name.first_name } } - it { is_expected.to respond_to(:terminator) } - it "is an alias for the `whodunnit` attribute" do expect(subject.terminator).to eq(attributes[:whodunnit]) end end describe "#version_author" do - it { is_expected.to respond_to(:version_author) } - - it "should be an alias for the `terminator` method" do + it "is an alias for the `terminator` method" do expect(subject.method(:version_author)).to eq(subject.method(:terminator)) end end end - describe "Class" do - column_overrides = [false] - if ENV["DB"] == "postgres" && ::ActiveRecord::VERSION::MAJOR >= 4 - column_overrides << "json" - # 'jsonb' column types are only supported for ActiveRecord 4.2+ - column_overrides << "jsonb" if ::ActiveRecord::VERSION::STRING >= "4.2" - end + column_overrides = [false] + if ENV["DB"] == "postgres" && ::ActiveRecord::VERSION::MAJOR >= 4 + column_overrides << "json" + # 'jsonb' column types are only supported for ActiveRecord 4.2+ + column_overrides << "jsonb" if ::ActiveRecord::VERSION::STRING >= "4.2" + end + + column_overrides.shuffle.each do |override| + context "with a #{override || 'text'} column" do + before do + if override + ActiveRecord::Base.connection.execute("SAVEPOINT pgtest;") + %w(object object_changes).each do |column| + ActiveRecord::Base.connection.execute( + "ALTER TABLE versions DROP COLUMN #{column};" + ) + ActiveRecord::Base.connection.execute( + "ALTER TABLE versions ADD COLUMN #{column} #{override};" + ) + end + PaperTrail::Version.reset_column_information + end + end + + after do + if override + ActiveRecord::Base.connection.execute("ROLLBACK TO SAVEPOINT pgtest;") + PaperTrail::Version.reset_column_information + end + end + + describe "#where_object", versioning: true do + let(:widget) { Widget.new } + let(:name) { FFaker::Name.first_name } + let(:int) { rand(10) + 1 } - column_overrides.shuffle.each do |override| - context "with a #{override || 'text'} column" do before do - if override - ActiveRecord::Base.connection.execute("SAVEPOINT pgtest;") - %w(object object_changes).each do |column| - ActiveRecord::Base.connection.execute( - "ALTER TABLE versions DROP COLUMN #{column};" - ) - ActiveRecord::Base.connection.execute( - "ALTER TABLE versions ADD COLUMN #{column} #{override};" - ) - end - PaperTrail::Version.reset_column_information - end + widget.update_attributes!(name: name, an_integer: int) + widget.update_attributes!(name: "foobar", an_integer: 100) + widget.update_attributes!(name: FFaker::Name.last_name, an_integer: 15) end - after do - if override - ActiveRecord::Base.connection.execute("ROLLBACK TO SAVEPOINT pgtest;") - PaperTrail::Version.reset_column_information + + it "requires its argument to be a Hash" do + expect { + PaperTrail::Version.where_object(:foo) + }.to raise_error(ArgumentError) + expect { + PaperTrail::Version.where_object([]) + }.to raise_error(ArgumentError) + end + + context "`serializer == YAML`" do + specify do + expect(PaperTrail.serializer).to be PaperTrail::Serializers::YAML + end + + it "locates versions according to their `object` contents" do + expect( + PaperTrail::Version.where_object(name: name) + ).to eq([widget.versions[1]]) + expect( + PaperTrail::Version.where_object(an_integer: 100) + ).to eq([widget.versions[2]]) end end - describe "#where_object" do - it { expect(PaperTrail::Version).to respond_to(:where_object) } - - context "invalid arguments" do - it "should raise an error" do - expect { - PaperTrail::Version.where_object(:foo) - }.to raise_error(ArgumentError) - expect { - PaperTrail::Version.where_object([]) - }.to raise_error(ArgumentError) - end + context "JSON serializer" do + before(:all) do + PaperTrail.serializer = PaperTrail::Serializers::JSON end - context "valid arguments", versioning: true do - let(:widget) { Widget.new } - let(:name) { FFaker::Name.first_name } - let(:int) { rand(10) + 1 } + specify do + expect(PaperTrail.serializer).to be PaperTrail::Serializers::JSON + end - before do - widget.update_attributes!(name: name, an_integer: int) - widget.update_attributes!(name: "foobar", an_integer: 100) - widget.update_attributes!(name: FFaker::Name.last_name, an_integer: 15) - end + it "locates versions according to their `object` contents" do + expect( + PaperTrail::Version.where_object(name: name) + ).to eq([widget.versions[1]]) + expect( + PaperTrail::Version.where_object(an_integer: 100) + ).to eq([widget.versions[2]]) + end - context "`serializer == YAML`" do - specify do - expect(PaperTrail.serializer).to be PaperTrail::Serializers::YAML - end + after(:all) do + PaperTrail.serializer = PaperTrail::Serializers::YAML + end + end + end - it "should be able to locate versions according to their `object` contents" do - expect( - PaperTrail::Version.where_object(name: name) - ).to eq([widget.versions[1]]) - expect( - PaperTrail::Version.where_object(an_integer: 100) - ).to eq([widget.versions[2]]) - end - end + describe "#where_object_changes", versioning: true do + let(:widget) { Widget.new } + let(:name) { FFaker::Name.first_name } + let(:int) { rand(5) + 2 } - context "JSON serializer" do - before(:all) do - PaperTrail.serializer = PaperTrail::Serializers::JSON - end + before do + widget.update_attributes!(name: name, an_integer: 0) + widget.update_attributes!(name: "foobar", an_integer: 77) + widget.update_attributes!(name: FFaker::Name.last_name, an_integer: int) + end - specify do - expect(PaperTrail.serializer).to be PaperTrail::Serializers::JSON - end + it "requires its argument to be a Hash" do + expect { + PaperTrail::Version.where_object_changes(:foo) + }.to raise_error(ArgumentError) + expect { + PaperTrail::Version.where_object_changes([]) + }.to raise_error(ArgumentError) + end - it "should be able to locate versions according to their `object` contents" do - expect( - PaperTrail::Version.where_object(name: name) - ).to eq([widget.versions[1]]) - expect( - PaperTrail::Version.where_object(an_integer: 100) - ).to eq([widget.versions[2]]) - end + context "YAML serializer" do + specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::YAML } - after(:all) do - PaperTrail.serializer = PaperTrail::Serializers::YAML - end - end + it "locates versions according to their `object_changes` contents" do + expect( + widget.versions.where_object_changes(name: name) + ).to eq(widget.versions[0..1]) + expect( + widget.versions.where_object_changes(an_integer: 77) + ).to eq(widget.versions[1..2]) + expect( + widget.versions.where_object_changes(an_integer: int) + ).to eq([widget.versions.last]) + end + + it "handles queries for multiple attributes" do + expect( + widget.versions.where_object_changes(an_integer: 77, name: "foobar") + ).to eq(widget.versions[1..2]) end end - describe "#where_object_changes" do - context "invalid arguments" do - it "should raise an error" do - expect { - PaperTrail::Version.where_object_changes(:foo) - }.to raise_error(ArgumentError) - expect { - PaperTrail::Version.where_object_changes([]) - }.to raise_error(ArgumentError) - end + context "JSON serializer" do + before(:all) { PaperTrail.serializer = PaperTrail::Serializers::JSON } + specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::JSON } + + it "locates versions according to their `object_changes` contents" do + expect( + widget.versions.where_object_changes(name: name) + ).to eq(widget.versions[0..1]) + expect( + widget.versions.where_object_changes(an_integer: 77) + ).to eq(widget.versions[1..2]) + expect( + widget.versions.where_object_changes(an_integer: int) + ).to eq([widget.versions.last]) end - context "valid arguments", versioning: true do - let(:widget) { Widget.new } - let(:name) { FFaker::Name.first_name } - let(:int) { rand(5) + 2 } - - before do - widget.update_attributes!(name: name, an_integer: 0) - widget.update_attributes!(name: "foobar", an_integer: 77) - widget.update_attributes!(name: FFaker::Name.last_name, an_integer: int) - end - - context "YAML serializer" do - specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::YAML } - - it "locates versions according to their `object_changes` contents" do - expect( - widget.versions.where_object_changes(name: name) - ).to eq(widget.versions[0..1]) - expect( - widget.versions.where_object_changes(an_integer: 77) - ).to eq(widget.versions[1..2]) - expect( - widget.versions.where_object_changes(an_integer: int) - ).to eq([widget.versions.last]) - end - - it "handles queries for multiple attributes" do - expect( - widget.versions.where_object_changes(an_integer: 77, name: "foobar") - ).to eq(widget.versions[1..2]) - end - end - - context "JSON serializer" do - before(:all) { PaperTrail.serializer = PaperTrail::Serializers::JSON } - specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::JSON } - - it "locates versions according to their `object_changes` contents" do - expect( - widget.versions.where_object_changes(name: name) - ).to eq(widget.versions[0..1]) - expect( - widget.versions.where_object_changes(an_integer: 77) - ).to eq(widget.versions[1..2]) - expect( - widget.versions.where_object_changes(an_integer: int) - ).to eq([widget.versions.last]) - end - - it "handles queries for multiple attributes" do - expect( - widget.versions.where_object_changes(an_integer: 77, name: "foobar") - ).to eq(widget.versions[1..2]) - end - - after(:all) { PaperTrail.serializer = PaperTrail::Serializers::YAML } - end + it "handles queries for multiple attributes" do + expect( + widget.versions.where_object_changes(an_integer: 77, name: "foobar") + ).to eq(widget.versions[1..2]) end + + after(:all) { PaperTrail.serializer = PaperTrail::Serializers::YAML } end end end diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb index 422fd0b7..adac9cca 100644 --- a/spec/models/widget_spec.rb +++ b/spec/models/widget_spec.rb @@ -38,13 +38,13 @@ describe Widget, type: :model do describe "versioning option" do context "enabled", versioning: true do - it "should enable versioning" do + it "enables versioning" do expect(widget.versions.size).to eq(1) end end context "disabled (default)" do - it "should not enable versioning" do + it "does not enable versioning" do expect(widget.versions.size).to eq(0) end end @@ -52,16 +52,14 @@ describe Widget, type: :model do describe "Callbacks", versioning: true do describe "before_save" do - context ":on => :update" do - before { widget.update_attributes!(name: "Foobar") } + before { widget.update_attributes!(name: "Foobar") } - subject { widget.versions.last.reify } + subject { widget.versions.last.reify } - it "resets value for timestamp attrs for update so that value gets updated properly" do - # Travel 1 second because MySQL lacks sub-second resolution - Timecop.travel(1) do - expect { subject.save! }.to change(subject, :updated_at) - end + it "resets value for timestamp attrs for update so that value gets updated properly" do + # Travel 1 second because MySQL lacks sub-second resolution + Timecop.travel(1) do + expect { subject.save! }.to change(subject, :updated_at) end end end @@ -69,7 +67,7 @@ describe Widget, type: :model do describe "after_create" do let(:widget) { Widget.create!(name: "Foobar", created_at: Time.now - 1.week) } - it "corresponding version should use the widget's `updated_at`" do + it "corresponding version uses the widget's `updated_at`" do expect(widget.versions.last.created_at.to_i).to eq(widget.updated_at.to_i) end end @@ -81,22 +79,22 @@ describe Widget, type: :model do it { expect(subject.paper_trail).not_to be_live } - it "should clear the `versions_association_name` virtual attribute" do + it "clears the `versions_association_name` virtual attribute" do subject.save! expect(subject.paper_trail).to be_live end - it "corresponding version should use the widget updated_at" do + it "corresponding version uses the widget updated_at" do expect(widget.versions.last.created_at.to_i).to eq(widget.updated_at.to_i) end end describe "after_destroy" do - it "should create a version for that event" do + it "creates a version for that event" do expect { widget.destroy }.to change(widget.versions, :count).by(1) end - it "should assign the version into the `versions_association_name`" do + it "assigns the version into the `versions_association_name`" do expect(widget.version).to be_nil widget.destroy expect(widget.version).not_to be_nil @@ -122,19 +120,19 @@ describe Widget, type: :model do it "does not create an event for changes that did not happen" do widget.versions.map(&:changeset).each do |changeset| - expect(changeset.fetch("name", [])).to_not include(rolled_back_name) + expect(changeset.fetch("name", [])).not_to include(rolled_back_name) end end it "has not yet loaded the assocation" do - expect(widget.versions).to_not be_loaded + expect(widget.versions).not_to be_loaded end end end describe "Association", versioning: true do describe "sort order" do - it "should sort by the timestamp order from the `VersionConcern`" do + it "sorts by the timestamp order from the `VersionConcern`" do expect(widget.versions.to_sql).to eq( widget.versions.reorder(PaperTrail::Version.timestamp_sort_order).to_sql ) @@ -144,7 +142,7 @@ describe Widget, type: :model do if defined?(ActiveRecord::IdentityMap) && ActiveRecord::IdentityMap.respond_to?(:without) describe "IdentityMap", versioning: true do - it "should not clobber the IdentityMap when reifying" 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 @@ -153,207 +151,180 @@ describe Widget, type: :model do end end - describe "Methods" do - describe "Instance", versioning: true do - describe "#create" do - it "creates a version record" do - wordget = Widget.create - assert_equal 1, wordget.versions.length - end - end + describe "#create", versioning: true do + it "creates a version record" do + wordget = Widget.create + assert_equal 1, wordget.versions.length + end + end - describe "#destroy" do - it "creates a version record" do - widget = Widget.create - assert_equal 1, widget.versions.length - widget.destroy - versions_for_widget = PaperTrail::Version.with_item_keys("Widget", widget.id) - assert_equal 2, versions_for_widget.length - end - - it "can have multiple destruction records" do - versions = lambda { |widget| - # Workaround for AR 3. When we drop AR 3 support, we can simply use - # the `widget.versions` association, instead of `with_item_keys`. - PaperTrail::Version.with_item_keys("Widget", widget.id) - } - widget = Widget.create - assert_equal 1, widget.versions.length - widget.destroy - assert_equal 2, versions.call(widget).length - widget = widget.version.reify - widget.save - assert_equal 3, versions.call(widget).length - widget.destroy - assert_equal 4, versions.call(widget).length - assert_equal 2, versions.call(widget).where(event: "destroy").length - end - end - - describe "#paper_trail.originator" do - describe "return value" do - let(:orig_name) { FFaker::Name.name } - let(:new_name) { FFaker::Name.name } - - before do - PaperTrail.whodunnit = orig_name - end - - context "accessed from live model instance" do - specify { expect(widget.paper_trail).to be_live } - - it "should return the originator for the model at a given state" do - expect(widget.paper_trail.originator).to eq(orig_name) - widget.paper_trail.whodunnit(new_name) { |w| - w.update_attributes(name: "Elizabeth") - } - expect(widget.paper_trail.originator).to eq(new_name) - end - end - - context "accessed from a reified model instance" do - before do - widget.update_attributes(name: "Andy") - PaperTrail.whodunnit = new_name - widget.update_attributes(name: "Elizabeth") - end - - context "default behavior (no `options[:dup]` option passed in)" do - let(:reified_widget) { widget.versions[1].reify } - - it "should return the appropriate originator" do - expect(reified_widget.paper_trail.originator).to eq(orig_name) - end - - it "should not create a new model instance" do - expect(reified_widget).not_to be_new_record - end - end - - context "creating a new instance (`options[:dup] == true`)" do - let(:reified_widget) { widget.versions[1].reify(dup: true) } - - it "should return the appropriate originator" do - expect(reified_widget.paper_trail.originator).to eq(orig_name) - end - - it "should not create a new model instance" do - expect(reified_widget).to be_new_record - end - end - end - end - end - - describe "#version_at" do - context "Timestamp argument is AFTER object has been destroyed" do - it "should return `nil`" do - widget.update_attribute(:name, "foobar") - widget.destroy - expect(widget.paper_trail.version_at(Time.now)).to be_nil - end - end - end - - describe "#whodunnit" do - context "no block given" do - it "should raise an error" do - expect { - widget.paper_trail.whodunnit("Ben") - }.to raise_error(ArgumentError, "expected to receive a block") - end - end - - context "block given" do - let(:orig_name) { FFaker::Name.name } - let(:new_name) { FFaker::Name.name } - - before do - PaperTrail.whodunnit = orig_name - expect(widget.versions.last.whodunnit).to eq(orig_name) # persist `widget` - end - - it "should modify value of `PaperTrail.whodunnit` while executing the block" do - widget.paper_trail.whodunnit(new_name) do - expect(PaperTrail.whodunnit).to eq(new_name) - widget.update_attributes(name: "Elizabeth") - end - expect(widget.versions.last.whodunnit).to eq(new_name) - end - - context "after executing the block" do - it "reverts value of whodunnit to previous value" do - widget.paper_trail.whodunnit(new_name) { |w| - w.update_attributes(name: "Elizabeth") - } - expect(PaperTrail.whodunnit).to eq(orig_name) - end - end - - context "error within block" do - it "still reverts the whodunnit value to previous value" do - expect { - widget.paper_trail.whodunnit(new_name) { raise } - }.to raise_error(RuntimeError) - expect(PaperTrail.whodunnit).to eq(orig_name) - end - end - end - end - - describe "#touch_with_version" do - it "creates a version" do - count = widget.versions.size - # Travel 1 second because MySQL lacks sub-second resolution - Timecop.travel(1) do - widget.paper_trail.touch_with_version - end - expect(widget.versions.size).to eq(count + 1) - end - - it "increments the `:updated_at` timestamp" do - time_was = widget.updated_at - # Travel 1 second because MySQL lacks sub-second resolution - Timecop.travel(1) do - widget.paper_trail.touch_with_version - end - expect(widget.updated_at).to be > time_was - end - end - - describe "#update" do - it "creates a version record" do - widget = Widget.create - assert_equal 1, widget.versions.length - widget.update_attributes(name: "Bugle") - assert_equal 2, widget.versions.length - end - end + describe "#destroy", versioning: true do + it "creates a version record" do + widget = Widget.create + assert_equal 1, widget.versions.length + widget.destroy + versions_for_widget = PaperTrail::Version.with_item_keys("Widget", widget.id) + assert_equal 2, versions_for_widget.length end - describe "Class" do - describe ".paper_trail.enabled?" do - it "returns true" do - expect(Widget.paper_trail.enabled?).to eq(true) - end + it "can have multiple destruction records" do + versions = lambda { |widget| + # Workaround for AR 3. When we drop AR 3 support, we can simply use + # the `widget.versions` association, instead of `with_item_keys`. + PaperTrail::Version.with_item_keys("Widget", widget.id) + } + widget = Widget.create + assert_equal 1, widget.versions.length + widget.destroy + assert_equal 2, versions.call(widget).length + widget = widget.version.reify + widget.save + assert_equal 3, versions.call(widget).length + widget.destroy + assert_equal 4, versions.call(widget).length + assert_equal 2, versions.call(widget).where(event: "destroy").length + end + end + + describe "#paper_trail.originator", versioning: true do + describe "return value" do + let(:orig_name) { FFaker::Name.name } + let(:new_name) { FFaker::Name.name } + + before do + PaperTrail.whodunnit = orig_name end - describe ".disable" do - it "should set the `paper_trail.enabled?` to `false`" do - expect(Widget.paper_trail.enabled?).to eq(true) - Widget.paper_trail.disable - expect(Widget.paper_trail.enabled?).to eq(false) - end + it "returns the originator for the model at a given state" do + expect(widget.paper_trail).to be_live + expect(widget.paper_trail.originator).to eq(orig_name) + widget.paper_trail.whodunnit(new_name) { |w| + w.update_attributes(name: "Elizabeth") + } + expect(widget.paper_trail.originator).to eq(new_name) end - describe ".enable" do - it "should set the `paper_trail.enabled?` to `true`" do - Widget.paper_trail.disable - expect(Widget.paper_trail.enabled?).to eq(false) - Widget.paper_trail.enable - expect(Widget.paper_trail.enabled?).to eq(true) - end + it "returns the appropriate originator" do + widget.update_attributes(name: "Andy") + PaperTrail.whodunnit = new_name + widget.update_attributes(name: "Elizabeth") + reified_widget = widget.versions[1].reify + expect(reified_widget.paper_trail.originator).to eq(orig_name) + expect(reified_widget).not_to be_new_record + end + + it "can create a new instance with options[:dup]" do + widget.update_attributes(name: "Andy") + PaperTrail.whodunnit = new_name + widget.update_attributes(name: "Elizabeth") + reified_widget = widget.versions[1].reify(dup: true) + expect(reified_widget.paper_trail.originator).to eq(orig_name) + expect(reified_widget).to be_new_record end end end + + describe "#version_at", versioning: true do + context "Timestamp argument is AFTER object has been destroyed" do + it "returns nil" do + widget.update_attribute(:name, "foobar") + widget.destroy + expect(widget.paper_trail.version_at(Time.now)).to be_nil + end + end + end + + describe "#whodunnit", versioning: true do + context "no block given" do + it "raises an error" do + expect { + widget.paper_trail.whodunnit("Ben") + }.to raise_error(ArgumentError, "expected to receive a block") + end + end + + context "block given" do + let(:orig_name) { FFaker::Name.name } + let(:new_name) { FFaker::Name.name } + + before do + PaperTrail.whodunnit = orig_name + expect(widget.versions.last.whodunnit).to eq(orig_name) # persist `widget` + end + + it "modifies value of `PaperTrail.whodunnit` while executing the block" do + widget.paper_trail.whodunnit(new_name) do + expect(PaperTrail.whodunnit).to eq(new_name) + widget.update_attributes(name: "Elizabeth") + end + expect(widget.versions.last.whodunnit).to eq(new_name) + end + + it "reverts value of whodunnit to previous value after executing the block" do + widget.paper_trail.whodunnit(new_name) { |w| + w.update_attributes(name: "Elizabeth") + } + expect(PaperTrail.whodunnit).to eq(orig_name) + end + + it "reverts to previous value, even if error within block" do + expect { + widget.paper_trail.whodunnit(new_name) { raise } + }.to raise_error(RuntimeError) + expect(PaperTrail.whodunnit).to eq(orig_name) + end + end + end + + describe "#touch_with_version", versioning: true do + it "creates a version" do + count = widget.versions.size + # Travel 1 second because MySQL lacks sub-second resolution + Timecop.travel(1) do + widget.paper_trail.touch_with_version + end + expect(widget.versions.size).to eq(count + 1) + end + + it "increments the `:updated_at` timestamp" do + time_was = widget.updated_at + # Travel 1 second because MySQL lacks sub-second resolution + Timecop.travel(1) do + widget.paper_trail.touch_with_version + end + expect(widget.updated_at).to be > time_was + end + end + + describe "#update", versioning: true do + it "creates a version record" do + widget = Widget.create + assert_equal 1, widget.versions.length + widget.update_attributes(name: "Bugle") + assert_equal 2, widget.versions.length + end + end + + describe ".paper_trail.enabled?" do + it "returns true" do + expect(Widget.paper_trail.enabled?).to eq(true) + end + end + + describe ".disable" do + it "sets the `paper_trail.enabled?` to `false`" do + expect(Widget.paper_trail.enabled?).to eq(true) + Widget.paper_trail.disable + expect(Widget.paper_trail.enabled?).to eq(false) + end + end + + describe ".enable" do + it "sets the `paper_trail.enabled?` to `true`" do + Widget.paper_trail.disable + expect(Widget.paper_trail.enabled?).to eq(false) + Widget.paper_trail.enable + expect(Widget.paper_trail.enabled?).to eq(true) + end + end end diff --git a/spec/modules/paper_trail_spec.rb b/spec/modules/paper_trail_spec.rb index 4bf9fe6e..23fd2d3c 100644 --- a/spec/modules/paper_trail_spec.rb +++ b/spec/modules/paper_trail_spec.rb @@ -4,13 +4,13 @@ describe PaperTrail, type: :module, versioning: true do describe "#config" do it { is_expected.to respond_to(:config) } - it "should allow for config values to be set" do + it "allows for config values to be set" do expect(subject.config.enabled).to eq(true) subject.config.enabled = false expect(subject.config.enabled).to eq(false) end - it "should accept blocks and yield the config instance" do + it "accepts blocks and yield the config instance" do expect(subject.config.enabled).to eq(true) subject.config { |c| c.enabled = false } expect(subject.config.enabled).to eq(false) @@ -20,7 +20,7 @@ describe PaperTrail, type: :module, versioning: true do describe "#configure" do it { is_expected.to respond_to(:configure) } - it "should be an alias for the `config` method" do + it "is an alias for the `config` method" do expect(subject.method(:configure)).to eq(subject.method(:config)) end end diff --git a/spec/modules/version_concern_spec.rb b/spec/modules/version_concern_spec.rb index 10574084..03d2ce1f 100644 --- a/spec/modules/version_concern_spec.rb +++ b/spec/modules/version_concern_spec.rb @@ -18,7 +18,7 @@ describe PaperTrail::VersionConcern do end describe "persistence", versioning: true do - it "should store versions in the correct corresponding db location" do + it "stores versions in the correct corresponding db location" do foo_doc = Foo::Document.create!(name: "foobar") bar_doc = Bar::Document.create!(name: "raboof") expect(foo_doc.versions.first).to be_instance_of(Foo::Version) diff --git a/spec/modules/version_number_spec.rb b/spec/modules/version_number_spec.rb index 632b2ba4..727cfa79 100644 --- a/spec/modules/version_number_spec.rb +++ b/spec/modules/version_number_spec.rb @@ -1,34 +1,16 @@ require "spec_helper" -describe PaperTrail::VERSION do - describe "Constants" do - subject { PaperTrail::VERSION } - - describe "MAJOR" do - it { is_expected.to be_const_defined(:MAJOR) } - it { expect(subject::MAJOR).to be_a(Integer) } - end - describe "MINOR" do - it { is_expected.to be_const_defined(:MINOR) } - it { expect(subject::MINOR).to be_a(Integer) } - end - describe "TINY" do - it { is_expected.to be_const_defined(:TINY) } - it { expect(subject::TINY).to be_a(Integer) } - end - describe "PRE" do - it { is_expected.to be_const_defined(:PRE) } - if PaperTrail::VERSION::PRE - it { expect(subject::PRE).to be_instance_of(String) } - end - end +module PaperTrail + RSpec.describe VERSION do describe "STRING" do - it { is_expected.to be_const_defined(:STRING) } - it { expect(subject::STRING).to be_instance_of(String) } - - it "should join the numbers into a period separated string" do - expect(subject::STRING).to eq( - [subject::MAJOR, subject::MINOR, subject::TINY, subject::PRE].compact.join(".") + it "joins the numbers into a period separated string" do + expect(described_class::STRING).to eq( + [ + described_class::MAJOR, + described_class::MINOR, + described_class::TINY, + described_class::PRE + ].compact.join(".") ) end end diff --git a/spec/paper_trail/config_spec.rb b/spec/paper_trail/config_spec.rb index 7be7ab0c..33b07bd0 100644 --- a/spec/paper_trail/config_spec.rb +++ b/spec/paper_trail/config_spec.rb @@ -4,7 +4,7 @@ module PaperTrail RSpec.describe Config do describe ".instance" do it "returns the singleton instance" do - expect { described_class.instance }.to_not raise_error + expect { described_class.instance }.not_to raise_error end end diff --git a/spec/paper_trail/serializers/custom_yaml_serializer_spec.rb b/spec/paper_trail/serializers/custom_yaml_serializer_spec.rb index 1a14318a..d0f21832 100644 --- a/spec/paper_trail/serializers/custom_yaml_serializer_spec.rb +++ b/spec/paper_trail/serializers/custom_yaml_serializer_spec.rb @@ -29,7 +29,7 @@ RSpec.describe CustomYamlSerializer do context(".load") do it("deserializes YAML to Ruby, removing pairs with blank keys or values") do - expect(CustomYamlSerializer.load(word_hash.to_yaml)).to eq( + expect(described_class.load(word_hash.to_yaml)).to eq( word_hash.reject { |k, v| (k.blank? || v.blank?) } ) end @@ -37,7 +37,7 @@ RSpec.describe CustomYamlSerializer do context(".dump") do it("serializes Ruby to YAML, removing pairs with nil values") do - expect(CustomYamlSerializer.dump(word_hash)).to eq( + expect(described_class.dump(word_hash)).to eq( word_hash.reject { |_k, v| v.nil? }.to_yaml ) end diff --git a/spec/paper_trail/serializers/json_spec.rb b/spec/paper_trail/serializers/json_spec.rb index f1685d33..3cc782ae 100644 --- a/spec/paper_trail/serializers/json_spec.rb +++ b/spec/paper_trail/serializers/json_spec.rb @@ -28,7 +28,7 @@ module PaperTrail matches = described_class. where_object_condition(PaperTrail::Version.arel_table[:object], :arg1, "Val 1") expect(matches.instance_of?(Arel::Nodes::Matches)).to(eq(true)) - expect("%\"arg1\":\"Val 1\"%").to(eq(matches.right.val)) + expect(matches.right.val).to eq("%\"arg1\":\"Val 1\"%") end end @@ -37,7 +37,7 @@ module PaperTrail matches = described_class. where_object_condition(PaperTrail::Version.arel_table[:object], :arg1, nil) expect(matches.instance_of?(Arel::Nodes::Matches)).to(eq(true)) - expect("%\"arg1\":null%").to(eq(matches.right.val)) + expect(matches.right.val).to(eq("%\"arg1\":null%")) end end @@ -47,8 +47,8 @@ module PaperTrail where_object_condition(PaperTrail::Version.arel_table[:object], :arg1, -3.5) expect(grouping.instance_of?(Arel::Nodes::Grouping)).to(eq(true)) matches = grouping.select { |v| v.instance_of?(Arel::Nodes::Matches) } - expect("%\"arg1\":-3.5,%").to(eq(matches.first.right.val)) - expect("%\"arg1\":-3.5}%").to(eq(matches.last.right.val)) + expect(matches.first.right.val).to eq("%\"arg1\":-3.5,%") + expect(matches.last.right.val).to eq("%\"arg1\":-3.5}%") end end end diff --git a/spec/paper_trail_spec.rb b/spec/paper_trail_spec.rb index e5f0fb23..20779a88 100644 --- a/spec/paper_trail_spec.rb +++ b/spec/paper_trail_spec.rb @@ -1,84 +1,92 @@ require "rails_helper" -describe PaperTrail do +RSpec.describe PaperTrail do + describe ".gem_version" do + it "returns a Gem::Version" do + v = described_class.gem_version + expect(v).to be_a(::Gem::Version) + expect(v.to_s).to eq(::PaperTrail::VERSION::STRING) + end + end + context "when enabled" do it "affects all threads" do - Thread.new { PaperTrail.enabled = false }.join - assert_equal false, PaperTrail.enabled? + Thread.new { described_class.enabled = false }.join + assert_equal false, described_class.enabled? end after do - PaperTrail.enabled = true + described_class.enabled = true end end context "default" do - it "should have versioning off by default" do - expect(PaperTrail).not_to be_enabled + it "has versioning off by default" do + expect(described_class).not_to be_enabled end - it "should turn versioning on in a `with_versioning` block" do - expect(PaperTrail).not_to be_enabled + it "has versioning on in a `with_versioning` block" do + expect(described_class).not_to be_enabled with_versioning do - expect(PaperTrail).to be_enabled + expect(described_class).to be_enabled end - expect(PaperTrail).not_to be_enabled + expect(described_class).not_to be_enabled end context "error within `with_versioning` block" do - it "should revert the value of `PaperTrail.enabled?` to it's previous state" do - expect(PaperTrail).not_to be_enabled + it "reverts the value of `PaperTrail.enabled?` to its previous state" do + expect(described_class).not_to be_enabled expect { with_versioning { raise } }.to raise_error(RuntimeError) - expect(PaperTrail).not_to be_enabled + expect(described_class).not_to be_enabled end end end context "`versioning: true`", versioning: true do - it "should have versioning on by default" do - expect(PaperTrail).to be_enabled + it "has versioning on by default" do + expect(described_class).to be_enabled end - it "should keep versioning on after a with_versioning block" do - expect(PaperTrail).to be_enabled + it "keeps versioning on after a with_versioning block" do + expect(described_class).to be_enabled with_versioning do - expect(PaperTrail).to be_enabled + expect(described_class).to be_enabled end - expect(PaperTrail).to be_enabled + expect(described_class).to be_enabled end end context "`with_versioning` block at class level" do - it { expect(PaperTrail).not_to be_enabled } + it { expect(described_class).not_to be_enabled } with_versioning do - it "should have versioning on by default" do - expect(PaperTrail).to be_enabled + it "has versioning on by default" do + expect(described_class).to be_enabled end end - it "should not leak the `enabled?` state into successive tests" do - expect(PaperTrail).not_to be_enabled + it "does not leak the `enabled?` state into successive tests" do + expect(described_class).not_to be_enabled end end describe ".version" do - it { expect(PaperTrail).to respond_to(:version) } - it { expect(PaperTrail.version).to eq(PaperTrail::VERSION::STRING) } + it { expect(described_class).to respond_to(:version) } + it { expect(described_class.version).to eq(described_class::VERSION::STRING) } end describe ".whodunnit" do - before(:all) { PaperTrail.whodunnit = "foobar" } + before(:all) { described_class.whodunnit = "foobar" } - it "should get set to `nil` by default" do - expect(PaperTrail.whodunnit).to be_nil + it "is nil by default" do + expect(described_class.whodunnit).to be_nil end end describe ".controller_info" do - before(:all) { ::PaperTrail.controller_info = { foo: "bar" } } + before(:all) { described_class.controller_info = { foo: "bar" } } - it "should get set to an empty hash before each test" do - expect(PaperTrail.controller_info).to eq({}) + it "is set to an empty hash before each test" do + expect(described_class.controller_info).to eq({}) end end end diff --git a/spec/requests/articles_spec.rb b/spec/requests/articles_spec.rb index 49b4feda..bc06b1f2 100644 --- a/spec/requests/articles_spec.rb +++ b/spec/requests/articles_spec.rb @@ -6,14 +6,14 @@ describe "Articles management", type: :request, order: :defined do context "versioning disabled" do specify { expect(PaperTrail).not_to be_enabled } - it "should not create a version" do + it "does not create a version" do expect(PaperTrail).to be_enabled_for_controller expect { post articles_path, params_wrapper(valid_params) - }.to_not change(PaperTrail::Version, :count) + }.not_to change(PaperTrail::Version, :count) end - it "should not leak the state of the `PaperTrail.enabled_for_controller?` into the next test" do + it "does not leak the state of the `PaperTrail.enabled_for_controller?` into the next test" do expect(PaperTrail).to be_enabled_for_controller end end @@ -22,7 +22,7 @@ describe "Articles management", type: :request, order: :defined do let(:article) { Article.last } context "`current_user` method returns a `String`" do - it "should set that value as the `whodunnit`" do + it "sets that value as the `whodunnit`" do expect { post articles_path, params_wrapper(valid_params) }.to change(PaperTrail::Version, :count).by(1) diff --git a/test/unit/model_test.rb b/test/unit/model_test.rb index 13bda913..b2dd57f1 100644 --- a/test/unit/model_test.rb +++ b/test/unit/model_test.rb @@ -317,9 +317,7 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase end should "have versions that are not live" do - assert @widget.versions.map(&:reify).compact.all? { |w| - !w.paper_trail.live? - } + assert(@widget.versions.map(&:reify).compact.all? { |w| !w.paper_trail.live? }) end should "have stored changes" do diff --git a/test/unit/serializers/mixin_json_test.rb b/test/unit/serializers/mixin_json_test.rb index 0b7beeab..72518d2c 100644 --- a/test/unit/serializers/mixin_json_test.rb +++ b/test/unit/serializers/mixin_json_test.rb @@ -19,8 +19,10 @@ class MixinJsonTest < ActiveSupport::TestCase end should "`deserialize` JSON to Ruby, removing pairs with `blank` keys or values" do - assert_equal @hash.reject { |k, v| k.blank? || v.blank? }, + assert_equal( + @hash.reject { |k, v| k.blank? || v.blank? }, CustomJsonSerializer.load(@hash_as_json) + ) end end