Merge pull request #155 from Shopify/add-test-factories

Add FactoryBot for test factories

Gemfile

@@ -3,16 +3,17 @@
 source "https://rubygems.org"
 
 gem "activerecord", "~> 7.1"
+gem "bundler"
 gem "jekyll", "~> 4.3"
 gem "rake", "~> 13.1"
 gem "sqlite3", "~> 1.7"
 gem "zeitwerk", "~> 2.6"
 
 gem "byebug"
-gem "bundler"
 gem "rubocop-shopify", require: false
 
 group :test do
+  gem "factory_bot_rails", "~> 6.4"
   gem "minitest", "~> 5.21"
   gem "minitest-rails", "~> 7.1"
   gem "minitest-hooks", "~> 1.5"

Gemfile.lock

@@ -50,6 +50,11 @@ GEM
       http_parser.rb (~> 0)
     erubi (1.12.0)
     eventmachine (1.2.7)
+    factory_bot (6.4.6)
+      activesupport (>= 5.0.0)
+    factory_bot_rails (6.4.3)
+      factory_bot (~> 6.4)
+      railties (>= 5.0.0)
     ffi (1.16.3)
     forwardable-extended (2.6.0)
     google-protobuf (4.26.1-arm64-darwin)
@@ -195,6 +200,7 @@ DEPENDENCIES
   activerecord (~> 7.1)
   bundler
   byebug
+  factory_bot_rails (~> 6.4)
   jekyll (~> 4.3)
   minitest (~> 5.21)
   minitest-hooks (~> 1.5)

app/models/property_value.rb

@@ -15,12 +15,12 @@ class PropertyValue < ApplicationRecord
     foreign_key: :primary_property_friendly_id,
     primary_key: :friendly_id
 
-  def primary_property_friendly_id=(id)
-    self.primary_property = Property.find_by(friendly_id: id)
+  def primary_property_friendly_id=(friendly_id)
+    self.primary_property = Property.find_by(friendly_id:)
   end
 
   validates :name, presence: true
-  validates :friendly_id, presence: true
+  validates :friendly_id, presence: true, uniqueness: true
   validates :handle, presence: true, uniqueness: { scope: :primary_property_friendly_id }
 
   def gid

test/factories/category_factory.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :category do
+    sequence(:id, 1) do
+      if parent.nil?
+        "aa"
+      else
+        "#{parent.id}-#{_1}"
+      end
+    end
+    name { "Category #{id}" }
+  end
+end

test/factories/property_factory.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :property do
+    sequence(:id, 1)
+    name { "Property#{id}" }
+    handle { name.downcase }
+    friendly_id { handle }
+  end
+end

test/factories/property_value_factory.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :property_value do
+    sequence(:id, 1)
+    name { "Value#{id}" }
+    handle { name.downcase }
+    friendly_id { [primary_property&.handle, handle].compact.join("__") }
+  end
+end

test/test_helper.rb

@@ -1,11 +1,24 @@
 # frozen_string_literal: true
 
 require "bundler/setup"
-
 Bundler.require(:test)
+
+require "minitest/rails"
+require "minitest/pride"
 require_relative "../application"
+
 Application.establish_db_connection!(env: :test)
 Application.load_and_reset_schema!
 
-require "minitest/rails"
-require "minitest/pride"
+FactoryBot.find_definitions
+
+class ApplicationTestCase < ActiveSupport::TestCase
+  include FactoryBot::Syntax::Methods
+  include Minitest::Hooks
+
+  def around_all
+    ActiveRecord::Base.transaction do
+      super
+    end
+  end
+end

test/unit/application_test.rb

@@ -2,12 +2,16 @@
 
 require_relative "../test_helper"
 
-class ApplicationTest < ActiveSupport::TestCase
+class ApplicationTest < ApplicationTestCase
   test "Zeitwerk compliance" do
     LOADER.eager_load(force: true)
   rescue Zeitwerk::NameError => e
     flunk(e.message)
   else
     assert(true)
   end
+
+  test "Factories are valid" do
+    FactoryBot.lint(traits: true)
+  end
 end

test/unit/category_test.rb

@@ -2,102 +2,99 @@
 
 require_relative "../test_helper"
 
-class CategoryTest < ActiveSupport::TestCase
+class CategoryTest < ApplicationTestCase
   def teardown
-    Category.destroy_all
+    Category.delete_all
   end
 
   test ".gid returns a global id" do
-    assert_equal "gid://shopify/TaxonomyCategory/tt", Category.gid("tt")
+    assert_equal "gid://shopify/TaxonomyCategory/aa", Category.gid("aa")
   end
 
   test ".parent_id_of returns the parent ID" do
-    assert_nil Category.parent_id_of("tt")
-    assert_equal "tt", Category.parent_id_of("tt-0")
-    assert_equal "tt-0", Category.parent_id_of("tt-0-1")
+    assert_nil Category.parent_id_of("aa")
+    assert_equal "aa", Category.parent_id_of("aa-0")
+    assert_equal "aa-0", Category.parent_id_of("aa-0-1")
   end
 
-  test "#id must follow parent" do
-    assert_predicate Category.new(id: "tt", name: "Root"), :valid?
-    assert_predicate Category.new(id: "tt-0", name: "Child", parent: category), :valid?
-    assert_predicate Category.new(id: "tt-123232", name: "Big child", parent: category), :valid?
+  test "#id must follow parent's id" do
+    assert_predicate build(:category, id: "aa-0", parent:), :valid?
+    assert_predicate build(:category, id: "aa-123232", parent:), :valid?
+    assert_predicate build(:category, id: "bb-0", parent:), :invalid?
 
-    assert_predicate Category.new(id: "aa-0", name: "Child", parent: category), :invalid?
+    child = build(:category, id: "aa-0", parent:)
+    assert_predicate build(:category, id: "aa-0-1", parent: child), :valid?
+    assert_predicate build(:category, id: "aa-1-1", parent: child), :invalid?
+  end
 
-    child = Category.new(id: "tt-0", name: "Child", parent: category)
-    assert_predicate Category.new(id: "tt-1-1", name: "Child", parent: child), :invalid?
+  test "#id for root must be 2 chars" do
+    assert_predicate build(:category, id: "t"), :invalid?
+    assert_predicate build(:category, id: "ttt"), :invalid?
+    assert_predicate build(:category, id: "01"), :invalid?
   end
 
-  test "#id must follow a strict format" do
-    assert_predicate Category.new(id: "t", name: "Root too short"), :invalid?
-    assert_predicate Category.new(id: "ttt", name: "Root too long"), :invalid?
-    assert_predicate Category.new(id: "01", name: "Root of numbers"), :invalid?
-    assert_predicate Category.new(id: "tt-t", name: "Child of letters", parent: category), :invalid?
+  test "#id for roots must not have dashes" do
+    assert_predicate build(:category, id: "aa-t"), :invalid?
   end
 
   test "#id must match depth" do
-    assert_predicate Category.new(id: "tt-0", name: "Child", parent: category), :valid?
-
-    assert_predicate Category.new(id: "tt-0-1", name: "Child with grandchild ID", parent: category), :invalid?
+    assert_predicate build(:category, id: "aa-0", parent:), :valid?
+    assert_predicate build(:category, id: "aa-0-1", parent:), :invalid?
   end
 
   test "#gid returns a global id" do
-    assert_equal "gid://shopify/TaxonomyCategory/tt", category.gid
+    assert_equal "gid://shopify/TaxonomyCategory/aa", parent.gid
+    assert_equal "gid://shopify/TaxonomyCategory/aa-42", build(:category, id: "aa-42").gid
   end
 
   test "#root returns the top-most category node" do
-    child_category = Category.new(id: "tt-6", name: "Child", parent: category)
-    grandchild_category = Category.new(id: "tt-6-1", name: "Grandchild", parent: child_category)
-
-    assert_equal category, child_category.root
-    assert_equal category, grandchild_category.root
+    assert_equal parent, child.root
+    assert_equal parent, grandchild.root
   end
 
   test "#ancestors walk up the tree" do
-    child_category = Category.new(id: "tt-7", name: "Child", parent: category)
-    grandchild_category = Category.new(id: "tt-7-1", name: "Grandchild", parent: child_category)
-    category.reload
-
-    assert_equal [child_category, category], grandchild_category.ancestors
+    assert_equal [child, parent], grandchild.ancestors
   end
 
   test "#ancestors_and_self includes self" do
-    child_category = Category.create(id: "tt-7", name: "Child", parent: category)
-    grandchild_category = Category.new(id: "tt-7-1", name: "Grandchild", parent: child_category)
-    category.reload
-
-    assert_equal [grandchild_category, child_category, category], grandchild_category.ancestors_and_self
+    assert_equal [grandchild, child, parent], grandchild.ancestors_and_self
   end
 
   test "#children are sorted by name" do
-    beta_child = Category.create(id: "tt-1", name: "Beta", parent: category)
-    alpha_child = Category.create(id: "tt-2", name: "Alpha", parent: category)
-    category.reload
+    beta_child = create(:category, name: "Beta", parent:)
+    alpha_child = create(:category, name: "Alpha", parent:)
+    parent.reload
 
-    assert_equal [alpha_child, beta_child], category.children.to_a
+    assert_equal [alpha_child, beta_child], parent.children.to_a
   end
 
   test "#descendants is depth-first" do
-    l2_category = Category.create(id: "tt-8", name: "Child", parent: category)
-    l3_category = Category.create(id: "tt-8-1", name: "Grandchild", parent: l2_category)
-    l3_sibling = Category.create(id: "tt-8-2", name: "Alpha Grandchild", parent: l2_category)
-    l4_category = Category.create(id: "tt-8-2-1", name: "Great Grandchild", parent: l3_sibling)
-    category.reload
+    l2_beta = create(:category, name: "Beta", parent: child)
+    l2_alpha = create(:category, name: "Alpha", parent: child)
+    l3_child = create(:category, parent: l2_alpha)
+    parent.reload
 
-    assert_equal [l2_category, l3_sibling, l4_category, l3_category], category.descendants
+    assert_equal [child, l2_alpha, l3_child, l2_beta], parent.descendants
   end
 
   test "#descendants_and_self includes self" do
-    child_category = Category.create(id: "tt-7", name: "Child", parent: category)
-    grandchild_category = Category.create(id: "tt-7-1", name: "Grandchild", parent: child_category)
-    category.reload
+    grandchild.save!
+    parent.reload
 
-    assert_equal [category, child_category, grandchild_category], category.descendants_and_self
+    assert_equal [parent, child, grandchild], parent.descendants_and_self
   end
 
   private
 
-  def category
-    @category ||= Category.create!(id: "tt", name: "Electronics")
+  def parent
+    @parent ||= build(:category, id: "aa")
+  end
+
+  def child
+    @child ||= build(:category, parent:)
+  end
+
+  def grandchild
+    @grandchild ||= build(:category, parent: child)
   end
 end

test/unit/property_test.rb

@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+require_relative "../test_helper"
+
+class PropertyTest < ApplicationTestCase
+  def teardown
+    Property.delete_all
+  end
+
+  test "default ordering is alphabetical" do
+    material = create(:property, name: "Material")
+    size = create(:property, name: "size")
+    color = create(:property, name: "Color")
+
+    assert_equal [color, material, size], Property.all.to_a
+  end
+
+  test ".base returns base properties" do
+    base_property.save!
+    extended_property.save!
+
+    assert_equal [base_property], Property.base
+  end
+
+  test ".extended returns properties based off others" do
+    base_property.save!
+    extended_property.save!
+
+    assert_equal [extended_property], Property.extended
+  end
+
+  test "#gid returns a global id" do
+    assert_equal "gid://shopify/TaxonomyAttribute/42", build(:property, id: 42).gid
+  end
+
+  test "#gid returns base_property.gid when extended" do
+    refute_equal base_property.id, extended_property.id
+    assert_equal base_property.gid, extended_property.gid
+  end
+
+  test "#base?" do
+    assert_predicate base_property, :base?
+    refute_predicate extended_property, :base?
+  end
+
+  test "#extended?" do
+    refute_predicate base_property, :extended?
+    assert_predicate extended_property, :extended?
+  end
+
+  test "#friendly_id must be unique" do
+    create(:property, friendly_id: "material")
+    another_material = build(:property, friendly_id: "material")
+
+    refute_predicate another_material, :valid?
+  end
+
+  test "#property_values must match base_property#property_values" do
+    value = build(:property_value)
+    base_property.property_values = [value]
+    extended_property.property_values = [value]
+
+    assert_predicate base_property, :valid?
+    assert_predicate extended_property, :valid?
+
+    extended_property.property_values = []
+
+    refute_predicate extended_property, :valid?
+  end
+
+  private
+
+  def base_property
+    @base_property ||= build(:property)
+  end
+
+  def extended_property
+    @extended_property ||= build(:property, base_property:)
+  end
+end