Merge pull request #12316 from anansilva/6055-fix-rubocop-lint-constant-definition-in-block

Fix rubocop Lint/ConstantDefinitionInBlock group

.rubocop_todo.yml

@@ -6,23 +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: 16
-# Configuration parameters: AllowedMethods.
-# AllowedMethods: enums
-Lint/ConstantDefinitionInBlock:
-  Exclude:
-    - 'lib/tasks/import_product_images.rake'
-    - 'lib/tasks/users.rake'
-    - 'spec/controllers/spree/admin/base_controller_spec.rb'
-    - 'spec/helpers/serializer_helper_spec.rb'
-    - 'spec/lib/reports/line_items_spec.rb'
-    - 'spec/models/spree/ability_spec.rb'
-    - 'spec/models/spree/gateway_spec.rb'
-    - 'spec/models/spree/preferences/configuration_spec.rb'
-    - 'spec/models/spree/preferences/preferable_spec.rb'
-    - 'spec/validators/date_time_string_validator_spec.rb'
-    - 'spec/validators/integer_array_validator_spec.rb'
-
 # Offense count: 2
 Lint/DuplicateMethods:
   Exclude:

lib/tasks/import_product_images.rake

@@ -4,15 +4,15 @@ namespace :ofn do
   namespace :import do
     desc "Importing images for products from CSV"
     task :product_images, [:filename] => [:environment] do |_task, args|
-      COLUMNS = [:producer, :name, :image_url].freeze
-
       puts "Warning: use only with trusted URLs. This script will download whatever it can, " \
            "including local secrets, and expose the file as an image file."
 
       raise "Filename required" if args[:filename].blank?
 
+      columns = %i[producer name image_url].freeze
+
       csv = CSV.read(args[:filename], headers: true, header_converters: :symbol)
-      raise "CSV columns reqired: #{COLUMNS.map(&:to_s)}" if (COLUMNS - csv.headers).present?
+      raise "CSV columns reqired: #{columns.map(&:to_s)}" if (columns - csv.headers).present?
 
       csv.each.with_index do |entry, index|
         puts "#{index} #{entry[:producer]}, #{entry[:name]}"

lib/tasks/users.rake

@@ -5,23 +5,8 @@ require 'csv'
 namespace :ofn do
   desc 'remove the limit of enterprises a user can create'
   task :remove_enterprise_limit, [:user_id] => :environment do |_task, args|
-    RemoveEnterpriseLimit.new(args.user_id).call
-  end
-
-  class RemoveEnterpriseLimit
-    MAX_INTEGER = 2_147_483_647
-
-    def initialize(user_id)
-      @user_id = user_id
-    end
-
-    def call
-      user = Spree::User.find(user_id)
-      user.update_attribute(:enterprise_limit, MAX_INTEGER)
-    end
-
-    private
-
-    attr_reader :user_id
+    max_integer = 2_147_483_647
+    user = Spree::User.find(args.user_id)
+    user.update_attribute(:enterprise_limit, max_integer)
   end
 end

spec/controllers/spree/admin/base_controller_spec.rb

@@ -71,10 +71,9 @@ describe Spree::Admin::BaseController, type: :controller do
 
   describe "determining the name of the serializer to be used" do
     before do
-      class Api::Admin::AllowedPrefixBaseSerializer; end;
-
-      class Api::Admin::BaseSerializer; end;
       allow(controller).to receive(:ams_prefix_whitelist) { [:allowed_prefix] }
+      stub_const('Api::Admin::AllowedPrefixBaseSerializer', Class)
+      stub_const('Api::Admin::BaseSerializer', Class)
     end
 
     context "when a prefix is passed in" do

spec/helpers/serializer_helper_spec.rb

@@ -4,10 +4,9 @@ require 'spec_helper'
 
 describe SerializerHelper, type: :helper do
   let(:serializer) do
-    class ExampleEnterpriseSerializer < ActiveModel::Serializer
+    Class.new(ActiveModel::Serializer) do
       attributes :id, :name
     end
-    ExampleEnterpriseSerializer
   end
 
   describe "#required_attributes" do

spec/lib/reports/line_items_spec.rb

@@ -3,35 +3,37 @@
 require 'spec_helper'
 
 describe Reporting::LineItems do
-  subject(:reports_line_items) { described_class.new(order_permissions, params) }
-
   # This object lets us add some test coverage despite the very deep coupling between the class
   # under test and the various objects it depends on. Other more common moking strategies where very
   # hard.
-  class FakeOrderPermissions
-    def initialize(line_items, orders_relation)
-      @relations = Spree::LineItem.where(id: line_items.map(&:id))
-      @orders_relation = orders_relation
+  let(:fake_order_permissions) do
+    Class.new do
+      def initialize(line_items, orders_relation)
+        @relations = Spree::LineItem.where(id: line_items.map(&:id))
+        @orders_relation = orders_relation
+      end
+
+      def visible_line_items
+        relations
+      end
+
+      def editable_line_items
+        line_item = FactoryBot.create(:line_item)
+        Spree::LineItem.where(id: line_item.id)
+      end
+
+      def visible_orders
+        orders_relation
+      end
+
+      private
+
+      attr_reader :relations, :orders_relation
     end
-
-    def visible_line_items
-      relations
-    end
-
-    def editable_line_items
-      line_item = FactoryBot.create(:line_item)
-      Spree::LineItem.where(id: line_item.id)
-    end
-
-    def visible_orders
-      orders_relation
-    end
-
-    private
-
-    attr_reader :relations, :orders_relation
   end
 
+  subject(:reports_line_items) { described_class.new(order_permissions, params) }
+
   describe '#list' do
     let!(:order) do
       create(
@@ -44,7 +46,7 @@ describe Reporting::LineItems do
     let!(:line_item1) { create(:line_item, order:) }
 
     let(:orders_relation) { Spree::Order.where(id: order.id) }
-    let(:order_permissions) { FakeOrderPermissions.new([line_item1], orders_relation) }
+    let(:order_permissions) { fake_order_permissions.new([line_item1], orders_relation) }
     let(:params) { {} }
 
     it 'returns masked data' do
@@ -58,7 +60,7 @@ describe Reporting::LineItems do
       let!(:line_item2) { create(:line_item, order:) }
       let!(:line_item3) { create(:line_item, order:) }
       let(:order_permissions) do
-        FakeOrderPermissions.new([line_item1, line_item2, line_item3], orders_relation)
+        fake_order_permissions.new([line_item1, line_item2, line_item3], orders_relation)
       end
       let(:params) { { variant_id_in: [line_item3.variant.id, line_item1.variant.id] } }
 

spec/lib/tasks/enterprises_rake_spec.rb

@@ -4,14 +4,16 @@ require 'spec_helper'
 require 'rake'
 
 describe 'enterprises.rake' do
+  before(:all) do
+    Rake.application.rake_require("tasks/enterprises")
+    Rake::Task.define_task(:environment)
+  end
+
   describe ':remove_enterprise' do
     context 'when the enterprises exists' do
       it 'removes the enterprise' do
         enterprise = create(:enterprise)
 
-        Rake.application.rake_require 'tasks/enterprises'
-        Rake::Task.define_task(:environment)
-
         expect {
           Rake.application.invoke_task "ofn:remove_enterprise[#{enterprise.id}]"
         }.to change { Enterprise.count }.by(-1)

spec/lib/tasks/import_product_images_rake_spec.rb

@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rake'
+
+RSpec.describe 'ofn:import:product_images' do
+  before(:all) do
+    Rake.application.rake_require("tasks/import_product_images")
+    Rake::Task.define_task(:environment)
+  end
+
+  before do
+    Rake::Task['ofn:import:product_images'].reenable
+  end
+
+  describe 'task' do
+    context "filename is blank" do
+      it 'raises an error' do
+        expect {
+          Rake.application.invoke_task('ofn:import:product_images')
+        }.to raise_error(RuntimeError,
+                         'Filename required')
+      end
+    end
+
+    context "invalid CSV format" do
+      it 'raises an error if CSV columns are missing' do
+        allow(CSV).to receive(:read).and_return(CSV::Table.new([]))
+
+        expect {
+          Rake.application.invoke_task('ofn:import:product_images["path/to/csv/file.csv"]')
+        }.to raise_error(RuntimeError, 'CSV columns reqired: ["producer", "name", "image_url"]')
+      end
+    end
+
+    context "valid CSV" do
+      it 'imports images for each product in the CSV that exists and does not have images' do
+        filename = 'path/to/csv/file.csv'
+
+        csv_data = [
+          { producer: 'Producer 1', name: 'Product 1', image_url: 'http://example.com/image1.jpg' },
+          { producer: 'Producer 2', name: 'Product 2', image_url: 'http://example.com/image2.jpg' },
+          { producer: 'Producer 3', name: 'Product 3', image_url: 'http://example.com/image3.jpg' }
+        ]
+
+        csv_rows = csv_data.map do |hash|
+          CSV::Row.new(hash.keys, hash.values)
+        end
+
+        csv_table = CSV::Table.new(csv_rows)
+
+        allow(CSV).to receive(:read).and_return(csv_table)
+
+        allow(Enterprise).to receive(:find_by!).with(name: 'Producer 1').and_return(double)
+        allow(Enterprise).to receive(:find_by!).with(name: 'Producer 2').and_return(double)
+        allow(Enterprise).to receive(:find_by!).with(name: 'Producer 3').and_return(double)
+
+        allow(Spree::Product).to receive(:where).and_return(
+          class_double('Spree::Product', first: nil),
+          class_double('Spree::Product', first: instance_double('Spree::Product', image: nil)),
+          class_double('Spree::Product', first: instance_double('Spree::Product', image: true))
+        )
+
+        allow_any_instance_of(ImageImporter).to receive(:import).and_return(true)
+
+        expected_output = <<~OUTPUT
+          Warning: use only with trusted URLs. This script will download whatever it can, including local secrets, and expose the file as an image file.
+          0 Producer 1, Product 1
+           product not found.
+          1 Producer 2, Product 2
+           image added.
+          2 Producer 3, Product 3
+           image exists, not updated.
+        OUTPUT
+
+        expect {
+          Rake.application.invoke_task('ofn:import:product_images["path/to/csv/file.csv"]')
+        }.to output(expected_output).to_stdout
+      end
+    end
+  end
+end

spec/lib/tasks/users_rake_spec.rb

@@ -4,27 +4,27 @@ require 'spec_helper'
 require 'rake'
 
 describe 'users.rake' do
-  before(:all) do
+  before do
     Rake.application.rake_require 'tasks/users'
     Rake::Task.define_task(:environment)
+    Rake::Task['ofn:remove_enterprise_limit'].reenable
   end
 
   describe ':remove_enterprise_limit' do
     context 'when the user exists' do
-      it 'sets the enterprise_limit to the maximum integer' do
-        max_integer = 2_147_483_647
-        user = create(:user)
+      let(:user) { create(:user) }
 
+      it 'sets the enterprise_limit to the maximum integer' do
         Rake.application.invoke_task "ofn:remove_enterprise_limit[#{user.id}]"
 
-        expect(user.reload.enterprise_limit).to eq(max_integer)
+        expect(user.reload.enterprise_limit).to eq(2_147_483_647)
       end
     end
 
     context 'when the user does not exist' do
       it 'raises' do
         expect {
-          RemoveEnterpriseLimit.new(-1).call
+          Rake.application.invoke_task "ofn:remove_enterprise_limit[123]"
         }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end

spec/models/spree/ability_spec.rb

@@ -13,8 +13,6 @@ describe Spree::Ability do
     user.spree_roles.clear
   end
 
-  TOKEN = 'token123'
-
   after(:each) {
     user.spree_roles = []
   }

spec/models/spree/gateway_spec.rb

@@ -3,20 +3,20 @@
 require 'spec_helper'
 
 describe Spree::Gateway do
-  class Provider
-    def initialize(options); end
+  let(:test_gateway) do
+    Class.new(Spree::Gateway) do
+      def provider_class
+        Class.new do
+          def initialize(options = {}); end
 
-    def imaginary_method; end
-  end
-
-  class TestGateway < Spree::Gateway
-    def provider_class
-      Provider
+          def imaginary_method; end
+        end
+      end
     end
   end
 
   it "passes through all arguments on a method_missing call" do
-    gateway = TestGateway.new
+    gateway = test_gateway.new
     expect(gateway.provider).to receive(:imaginary_method).with('foo')
     gateway.imaginary_method('foo')
   end

spec/models/spree/preferences/configuration_spec.rb

@@ -3,25 +3,24 @@
 require 'spec_helper'
 
 describe Spree::Preferences::Configuration do
-  before :all do
-    class AppConfig < Spree::Preferences::Configuration
+  let(:config) do
+    Class.new(Spree::Preferences::Configuration) do
       preference :color, :string, default: :blue
-    end
-    @config = AppConfig.new
+    end.new
   end
 
   it "has named methods to access preferences" do
-    @config.color = 'orange'
-    expect(@config.color).to eq 'orange'
+    config.color = 'orange'
+    expect(config.color).to eq 'orange'
   end
 
   it "uses [ ] to access preferences" do
-    @config[:color] = 'red'
-    expect(@config[:color]).to eq 'red'
+    config[:color] = 'red'
+    expect(config[:color]).to eq 'red'
   end
 
   it "uses set/get to access preferences" do
-    @config.set :color, 'green'
-    expect(@config.get(:color)).to eq 'green'
+    config.set :color, 'green'
+    expect(config.get(:color)).to eq 'green'
   end
 end

spec/models/spree/preferences/preferable_spec.rb

@@ -3,8 +3,8 @@
 require 'spec_helper'
 
 describe Spree::Preferences::Preferable do
-  before :all do
-    class A
+  a_class = A =
+    Class.new do
       include Spree::Preferences::Preferable
       attr_reader :id
 
@@ -15,16 +15,38 @@ describe Spree::Preferences::Preferable do
       preference :color, :string, default: 'green', description: "My Favorite Color"
     end
 
-    class B < A
+  b_class = B =
+    Class.new(a_class) do
       preference :flavor, :string
     end
-  end
 
-  before :each do
-    @a = A.new
-    allow(@a).to receive_messages(persisted?: true)
-    @b = B.new
-    allow(@b).to receive_messages(persisted?: true)
+  let(:a) { a_class.new }
+  let(:b) { b_class.new }
+
+  create_pref_test =
+    Class.new(ActiveRecord::Migration[4.2]) do
+      def self.up
+        create_table :pref_tests do |t|
+          t.string :col
+        end
+      end
+
+      def self.down
+        drop_table :pref_tests
+      end
+    end
+
+  pref_test_class =
+    Class.new(ApplicationRecord) do
+      self.table_name = 'pref_tests'
+
+      preference :pref_test_pref, :string, default: 'abc'
+      preference :pref_test_any, :any, default: []
+    end
+
+  before do
+    allow(a).to receive_messages(persisted?: true)
+    allow(b).to receive_messages(persisted?: true)
 
     # ensure we're persisting as that is the default
     #
@@ -34,235 +56,213 @@ describe Spree::Preferences::Preferable do
 
   describe "preference definitions" do
     it "parent should not see child definitions" do
-      expect(@a.has_preference?(:color)).to be_truthy
-      expect(@a.has_preference?(:flavor)).not_to be_truthy
+      expect(a.has_preference?(:color)).to be_truthy
+      expect(a.has_preference?(:flavor)).not_to be_truthy
     end
 
     it "child should have parent and own definitions" do
-      expect(@b.has_preference?(:color)).to be_truthy
-      expect(@b.has_preference?(:flavor)).to be_truthy
+      expect(b.has_preference?(:color)).to be_truthy
+      expect(b.has_preference?(:flavor)).to be_truthy
     end
 
     it "instances have defaults" do
-      expect(@a.preferred_color).to eq 'green'
-      expect(@b.preferred_color).to eq 'green'
-      expect(@b.preferred_flavor).to be_nil
+      expect(a.preferred_color).to eq 'green'
+      expect(b.preferred_color).to eq 'green'
+      expect(b.preferred_flavor).to be_nil
     end
 
     it "can be asked if it has a preference definition" do
-      expect(@a.has_preference?(:color)).to be_truthy
-      expect(@a.has_preference?(:bad)).to be_falsy
+      expect(a.has_preference?(:color)).to be_truthy
+      expect(a.has_preference?(:bad)).to be_falsy
     end
 
     it "can be asked and raises" do
       expect {
-        @a.has_preference! :flavor
+        a.has_preference! :flavor
       }.to raise_error(NoMethodError, "flavor preference not defined")
     end
 
     it "has a type" do
-      expect(@a.preferred_color_type).to eq :string
-      expect(@a.preference_type(:color)).to eq :string
+      expect(a.preferred_color_type).to eq :string
+      expect(a.preference_type(:color)).to eq :string
     end
 
     it "has a default" do
-      expect(@a.preferred_color_default).to eq 'green'
-      expect(@a.preference_default(:color)).to eq 'green'
+      expect(a.preferred_color_default).to eq 'green'
+      expect(a.preference_default(:color)).to eq 'green'
     end
 
     it "has a description" do
-      expect(@a.preferred_color_description).to eq "My Favorite Color"
-      expect(@a.preference_description(:color)).to eq "My Favorite Color"
+      expect(a.preferred_color_description).to eq "My Favorite Color"
+      expect(a.preference_description(:color)).to eq "My Favorite Color"
     end
 
     it "raises if not defined" do
       expect {
-        @a.get_preference :flavor
+        a.get_preference :flavor
       }.to raise_error(NoMethodError, "flavor preference not defined")
     end
   end
 
   describe "preference access" do
     it "handles ghost methods for preferences" do
-      @a.preferred_color = 'blue'
-      expect(@a.preferred_color).to eq 'blue'
+      a.preferred_color = 'blue'
+      expect(a.preferred_color).to eq 'blue'
 
-      @a.prefers_color = 'green'
-      expect(@a.prefers_color?).to eq 'green'
+      a.prefers_color = 'green'
+      expect(a.prefers_color?).to eq 'green'
     end
 
     it "has genric readers" do
-      @a.preferred_color = 'red'
-      expect(@a.prefers?(:color)).to eq 'red'
-      expect(@a.preferred(:color)).to eq 'red'
+      a.preferred_color = 'red'
+      expect(a.prefers?(:color)).to eq 'red'
+      expect(a.preferred(:color)).to eq 'red'
     end
 
     it "parent and child instances have their own prefs" do
-      @a.preferred_color = 'red'
-      @b.preferred_color = 'blue'
+      a.preferred_color = 'red'
+      b.preferred_color = 'blue'
 
-      expect(@a.preferred_color).to eq 'red'
-      expect(@b.preferred_color).to eq 'blue'
+      expect(a.preferred_color).to eq 'red'
+      expect(b.preferred_color).to eq 'blue'
     end
 
     it "raises when preference not defined" do
       expect {
-        @a.set_preference(:bad, :bone)
+        a.set_preference(:bad, :bone)
       }.to raise_exception(NoMethodError, "bad preference not defined")
     end
 
     it "builds a hash of preferences" do
-      @b.preferred_flavor = :strawberry
-      expect(@b.preferences[:flavor]).to eq 'strawberry'
-      expect(@b.preferences[:color]).to eq 'green' # default from A
+      b.preferred_flavor = :strawberry
+      expect(b.preferences[:flavor]).to eq 'strawberry'
+      expect(b.preferences[:color]).to eq 'green' # default from A
     end
 
     context "database fallback" do
       before do
-        @a.instance_variable_set("@pending_preferences", {})
+        a.instance_variable_set("@pending_preferences", {})
       end
 
       it "retrieves a preference from the database before falling back to default" do
         preference = double(value: "chatreuse", key: 'a/color/123')
         expect(Spree::Preference).to receive(:find_by).and_return(preference)
-        expect(@a.preferred_color).to eq 'chatreuse'
+        expect(a.preferred_color).to eq 'chatreuse'
       end
 
       it "defaults if no database key exists" do
         expect(Spree::Preference).to receive(:find_by).and_return(nil)
-        expect(@a.preferred_color).to eq 'green'
+        expect(a.preferred_color).to eq 'green'
       end
     end
 
     context "converts integer preferences to integer values" do
       before do
-        A.preference :is_integer, :integer
+        a_class.preference :is_integer, :integer
       end
 
       it "with strings" do
-        @a.set_preference(:is_integer, '3')
-        expect(@a.preferences[:is_integer]).to eq 3
+        a.set_preference(:is_integer, '3')
+        expect(a.preferences[:is_integer]).to eq 3
 
-        @a.set_preference(:is_integer, '')
-        expect(@a.preferences[:is_integer]).to eq 0
+        a.set_preference(:is_integer, '')
+        expect(a.preferences[:is_integer]).to eq 0
       end
     end
 
     context "converts decimal preferences to BigDecimal values" do
       before do
-        A.preference :if_decimal, :decimal
+        a_class.preference :if_decimal, :decimal
       end
 
       it "returns a BigDecimal" do
-        @a.set_preference(:if_decimal, 3.3)
-        expect(@a.preferences[:if_decimal].class).to eq BigDecimal
+        a.set_preference(:if_decimal, 3.3)
+        expect(a.preferences[:if_decimal].class).to eq BigDecimal
       end
 
       it "with strings" do
-        @a.set_preference(:if_decimal, '3.3')
-        expect(@a.preferences[:if_decimal]).to eq 3.3
+        a.set_preference(:if_decimal, '3.3')
+        expect(a.preferences[:if_decimal]).to eq 3.3
 
-        @a.set_preference(:if_decimal, '')
-        expect(@a.preferences[:if_decimal]).to eq 0.0
+        a.set_preference(:if_decimal, '')
+        expect(a.preferences[:if_decimal]).to eq 0.0
       end
 
       context "when the value cannot be converted to BigDecimal" do
         it "returns the original value" do
-          @a.set_preference(:if_decimal, "invalid")
-          expect(@a.preferences[:if_decimal]).to eq "invalid"
+          a.set_preference(:if_decimal, "invalid")
+          expect(a.preferences[:if_decimal]).to eq "invalid"
         end
       end
     end
 
     context "converts boolean preferences to boolean values" do
       before do
-        A.preference :is_boolean, :boolean, default: true
+        a_class.preference :is_boolean, :boolean, default: true
       end
 
       it "with strings" do
-        @a.set_preference(:is_boolean, '0')
-        expect(@a.preferences[:is_boolean]).to be_falsy
-        @a.set_preference(:is_boolean, 'f')
-        expect(@a.preferences[:is_boolean]).to be_falsy
-        @a.set_preference(:is_boolean, 't')
-        expect(@a.preferences[:is_boolean]).to be_truthy
+        a.set_preference(:is_boolean, '0')
+        expect(a.preferences[:is_boolean]).to be_falsy
+        a.set_preference(:is_boolean, 'f')
+        expect(a.preferences[:is_boolean]).to be_falsy
+        a.set_preference(:is_boolean, 't')
+        expect(a.preferences[:is_boolean]).to be_truthy
       end
 
       it "with integers" do
-        @a.set_preference(:is_boolean, 0)
-        expect(@a.preferences[:is_boolean]).to be_falsy
-        @a.set_preference(:is_boolean, 1)
-        expect(@a.preferences[:is_boolean]).to be_truthy
+        a.set_preference(:is_boolean, 0)
+        expect(a.preferences[:is_boolean]).to be_falsy
+        a.set_preference(:is_boolean, 1)
+        expect(a.preferences[:is_boolean]).to be_truthy
       end
 
       it "with an empty string" do
-        @a.set_preference(:is_boolean, '')
-        expect(@a.preferences[:is_boolean]).to be_falsy
+        a.set_preference(:is_boolean, '')
+        expect(a.preferences[:is_boolean]).to be_falsy
       end
 
       it "with an empty hash" do
-        @a.set_preference(:is_boolean, [])
-        expect(@a.preferences[:is_boolean]).to be_falsy
+        a.set_preference(:is_boolean, [])
+        expect(a.preferences[:is_boolean]).to be_falsy
       end
     end
 
     context "converts any preferences to any values" do
       before do
-        A.preference :product_ids, :any, default: []
-        A.preference :product_attributes, :any, default: {}
+        a_class.preference :product_ids, :any, default: []
+        a_class.preference :product_attributes, :any, default: {}
       end
 
       it "with array" do
-        expect(@a.preferences[:product_ids]).to eq []
-        @a.set_preference(:product_ids, [1, 2])
-        expect(@a.preferences[:product_ids]).to eq [1, 2]
+        expect(a.preferences[:product_ids]).to eq []
+        a.set_preference(:product_ids, [1, 2])
+        expect(a.preferences[:product_ids]).to eq [1, 2]
       end
 
       it "with hash" do
-        expect(@a.preferences[:product_attributes]).to eq({})
-        @a.set_preference(:product_attributes, { id: 1, name: 2 })
+        expect(a.preferences[:product_attributes]).to eq({})
+        a.set_preference(:product_attributes, { id: 1, name: 2 })
         attributes_hash = { id: 1, name: 2 }
-        expect(@a.preferences[:product_attributes]).to eq attributes_hash
+        expect(a.preferences[:product_attributes]).to eq attributes_hash
       end
     end
   end
 
   describe "persisted preferables" do
     before(:all) do
-      class CreatePrefTest < ActiveRecord::Migration[4.2]
-        def self.up
-          create_table :pref_tests do |t|
-            t.string :col
-          end
-        end
-
-        def self.down
-          drop_table :pref_tests
-        end
-      end
-
-      @migration_verbosity = ActiveRecord::Migration.verbose
       ActiveRecord::Migration.verbose = false
-      CreatePrefTest.migrate(:up)
-
-      class PrefTest < ApplicationRecord
-        preference :pref_test_pref, :string, default: 'abc'
-        preference :pref_test_any, :any, default: []
-      end
+      create_pref_test.migrate(:up)
     end
 
     after(:all) do
-      CreatePrefTest.migrate(:down)
-      ActiveRecord::Migration.verbose = @migration_verbosity
-    end
-
-    before(:each) do
-      @pt = PrefTest.create
+      create_pref_test.migrate(:down)
+      ActiveRecord::Migration.verbose = true
     end
 
     describe "pending preferences for new activerecord objects" do
       it "saves preferences after record is saved" do
-        pr = PrefTest.new
+        pr = pref_test_class.new
         pr.set_preference(:pref_test_pref, 'XXX')
         expect(pr.get_preference(:pref_test_pref)).to eq 'XXX'
         pr.save!
@@ -270,7 +270,7 @@ describe Spree::Preferences::Preferable do
       end
 
       it "saves preferences for serialized object" do
-        pr = PrefTest.new
+        pr = pref_test_class.new
         pr.set_preference(:pref_test_any, [1, 2])
         expect(pr.get_preference(:pref_test_any)).to eq [1, 2]
         pr.save!
@@ -280,7 +280,7 @@ describe Spree::Preferences::Preferable do
 
     describe "requires a valid id" do
       it "for cache_key" do
-        pref_test = PrefTest.new
+        pref_test = pref_test_class.new
         expect(pref_test.preference_cache_key(:pref_test_pref)).to be_nil
 
         pref_test.save
@@ -288,46 +288,48 @@ describe Spree::Preferences::Preferable do
       end
 
       it "but returns default values" do
-        pref_test = PrefTest.new
+        pref_test = pref_test_class.new
         expect(pref_test.get_preference(:pref_test_pref)).to eq 'abc'
       end
 
       it "adds prefs in a pending hash until after_create" do
-        pref_test = PrefTest.new
+        pref_test = pref_test_class.new
         expect(pref_test).to receive(:add_pending_preference).with(:pref_test_pref, 'XXX')
         pref_test.set_preference(:pref_test_pref, 'XXX')
       end
     end
 
+    let!(:pt) { pref_test_class.create }
+
     it "clear preferences" do
-      @pt.set_preference(:pref_test_pref, 'xyz')
-      expect(@pt.preferred_pref_test_pref).to eq 'xyz'
-      @pt.clear_preferences
-      expect(@pt.preferred_pref_test_pref).to eq 'abc'
+      pt.set_preference(:pref_test_pref, 'xyz')
+      expect(pt.preferred_pref_test_pref).to eq 'xyz'
+      pt.clear_preferences
+      expect(pt.preferred_pref_test_pref).to eq 'abc'
     end
 
     it "clear preferences when record is deleted" do
-      @pt.save!
-      @pt.preferred_pref_test_pref = 'lmn'
-      @pt.save!
-      @pt.destroy
-      @pt1 = PrefTest.new(col: 'aaaa')
-      @pt1.id = @pt.id
-      @pt1.save!
-      expect(@pt1.get_preference(:pref_test_pref)).not_to eq 'lmn'
-      expect(@pt1.get_preference(:pref_test_pref)).to eq 'abc'
+      pt.save!
+      pt.preferred_pref_test_pref = 'lmn'
+      pt.save!
+      pt.destroy
+      pt1 = pref_test_class.new(col: 'aaaa')
+      pt1.id = pt.id
+      pt1.save!
+      expect(pt1.get_preference(:pref_test_pref)).not_to eq 'lmn'
+      expect(pt1.get_preference(:pref_test_pref)).to eq 'abc'
     end
   end
 
   it "builds cache keys" do
-    expect(@a.preference_cache_key(:color)).to match %r{a/color/\d+}
+    expect(a.preference_cache_key(:color)).to match %r{a/color/\d+}
   end
 
   it "can add and remove preferences" do
-    A.preference :test_temp, :boolean, default: true
-    expect(@a.preferred_test_temp).to be_truthy
-    A.remove_preference :test_temp
-    expect(@a.has_preference?(:test_temp)).to be_falsy
-    expect(@a.respond_to?(:preferred_test_temp)).to be_falsy
+    a_class.preference :test_temp, :boolean, default: true
+    expect(a.preferred_test_temp).to be_truthy
+    a_class.remove_preference :test_temp
+    expect(a.has_preference?(:test_temp)).to be_falsy
+    expect(a.respond_to?(:preferred_test_temp)).to be_falsy
   end
 end

spec/validators/date_time_string_validator_spec.rb

@@ -3,14 +3,6 @@
 require "spec_helper"
 
 describe DateTimeStringValidator do
-  class TestModel
-    include ActiveModel::Validations
-
-    attr_accessor :timestamp
-
-    validates :timestamp, date_time_string: true
-  end
-
   describe "internationalization" do
     it "has translation for NOT_STRING_ERROR" do
       expect(described_class.not_string_error).not_to be_blank
@@ -22,7 +14,14 @@ describe DateTimeStringValidator do
   end
 
   describe "validation" do
-    let(:instance) { TestModel.new }
+    let(:instance) do
+      Class.new do
+        include ActiveModel::Validations
+        attr_accessor :timestamp
+
+        validates :timestamp, date_time_string: true
+      end.new
+    end
 
     it "does not add error when nil" do
       instance.timestamp = nil

spec/validators/integer_array_validator_spec.rb

@@ -3,14 +3,6 @@
 require "spec_helper"
 
 describe IntegerArrayValidator do
-  class TestModel
-    include ActiveModel::Validations
-
-    attr_accessor :ids
-
-    validates :ids, integer_array: true
-  end
-
   describe "internationalization" do
     it "has translation for NOT_ARRAY_ERROR" do
       expect(described_class.not_array_error).not_to be_blank
@@ -22,7 +14,14 @@ describe IntegerArrayValidator do
   end
 
   describe "validation" do
-    let(:instance) { TestModel.new }
+    let(:instance) do
+      Class.new do
+        include ActiveModel::Validations
+        attr_accessor :ids
+
+        validates :ids, integer_array: true
+      end.new
+    end
 
     it "does not add error when nil" do
       instance.ids = nil