From 0473865b937019ed73b45ea319bb0197f712394a Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Sat, 4 Aug 2018 10:56:37 -0400 Subject: [PATCH] Remove static attributes Co-authored-by: George Wambold --- lib/factory_bot/attribute.rb | 1 - lib/factory_bot/attribute/static.rb | 16 ----- lib/factory_bot/declaration.rb | 1 - lib/factory_bot/declaration/static.rb | 26 -------- spec/acceptance/global_to_create_spec.rb | 64 ++++++++++--------- spec/acceptance/lint_spec.rb | 20 +++--- .../static_attribute_deprecation_spec.rb | 37 ----------- spec/factory_bot/attribute/static_spec.rb | 17 ----- spec/factory_bot/attribute_list_spec.rb | 56 ++++++++-------- spec/factory_bot/definition_proxy_spec.rb | 4 +- spec/factory_bot/factory_spec.rb | 38 ++++++----- spec/support/matchers/declaration.rb | 5 -- 12 files changed, 99 insertions(+), 186 deletions(-) delete mode 100644 lib/factory_bot/attribute/static.rb delete mode 100644 lib/factory_bot/declaration/static.rb delete mode 100644 spec/acceptance/static_attribute_deprecation_spec.rb delete mode 100644 spec/factory_bot/attribute/static_spec.rb diff --git a/lib/factory_bot/attribute.rb b/lib/factory_bot/attribute.rb index 9fab4c7..f878679 100644 --- a/lib/factory_bot/attribute.rb +++ b/lib/factory_bot/attribute.rb @@ -1,4 +1,3 @@ -require 'factory_bot/attribute/static' require 'factory_bot/attribute/dynamic' require 'factory_bot/attribute/association' require 'factory_bot/attribute/sequence' diff --git a/lib/factory_bot/attribute/static.rb b/lib/factory_bot/attribute/static.rb deleted file mode 100644 index fae30dc..0000000 --- a/lib/factory_bot/attribute/static.rb +++ /dev/null @@ -1,16 +0,0 @@ -module FactoryBot - class Attribute - # @api private - class Static < Attribute - def initialize(name, value, ignored) - super(name, ignored) - @value = value - end - - def to_proc - value = @value - -> { value } - end - end - end -end diff --git a/lib/factory_bot/declaration.rb b/lib/factory_bot/declaration.rb index 4f71c1c..1e05ce7 100644 --- a/lib/factory_bot/declaration.rb +++ b/lib/factory_bot/declaration.rb @@ -1,4 +1,3 @@ -require 'factory_bot/declaration/static' require 'factory_bot/declaration/dynamic' require 'factory_bot/declaration/association' require 'factory_bot/declaration/implicit' diff --git a/lib/factory_bot/declaration/static.rb b/lib/factory_bot/declaration/static.rb deleted file mode 100644 index 1b773a0..0000000 --- a/lib/factory_bot/declaration/static.rb +++ /dev/null @@ -1,26 +0,0 @@ -module FactoryBot - class Declaration - # @api private - class Static < Declaration - def initialize(name, value, ignored = false) - super(name, ignored) - @value = value - end - - def ==(other) - name == other.name && - value == other.value && - ignored == other.ignored - end - - protected - attr_reader :value - - private - - def build - [Attribute::Static.new(name, @value, @ignored)] - end - end - end -end diff --git a/spec/acceptance/global_to_create_spec.rb b/spec/acceptance/global_to_create_spec.rb index fbc5438..040639f 100644 --- a/spec/acceptance/global_to_create_spec.rb +++ b/spec/acceptance/global_to_create_spec.rb @@ -1,17 +1,17 @@ -describe 'global to_create' do +describe "global to_create" do before do - define_model('User', name: :string) - define_model('Post', name: :string) + define_model("User", name: :string) + define_model("Post", name: :string) FactoryBot.define do - to_create { |instance| instance.name = 'persisted!' } + to_create { |instance| instance.name = "persisted!" } trait :override_to_create do - to_create { |instance| instance.name = 'override' } + to_create { |instance| instance.name = "override" } end factory :user do - name { 'John Doe' } + name { "John Doe" } factory :child_user @@ -21,7 +21,7 @@ describe 'global to_create' do end factory :post do - name { 'Great title' } + name { "Great title" } factory :child_post @@ -32,52 +32,58 @@ describe 'global to_create' do end end - it 'handles base to_create' do - expect(FactoryBot.create(:user).name).to eq 'persisted!' - expect(FactoryBot.create(:post).name).to eq 'persisted!' + it "handles base to_create" do + expect(FactoryBot.create(:user).name).to eq "persisted!" + expect(FactoryBot.create(:post).name).to eq "persisted!" end - it 'handles child to_create' do - expect(FactoryBot.create(:child_user).name).to eq 'persisted!' - expect(FactoryBot.create(:child_post).name).to eq 'persisted!' + it "handles child to_create" do + expect(FactoryBot.create(:child_user).name).to eq "persisted!" + expect(FactoryBot.create(:child_post).name).to eq "persisted!" end - it 'handles child to_create with trait' do - expect(FactoryBot.create(:child_user_with_trait).name).to eq 'override' - expect(FactoryBot.create(:child_post_with_trait).name).to eq 'override' + it "handles child to_create with trait" do + expect(FactoryBot.create(:child_user_with_trait).name).to eq "override" + expect(FactoryBot.create(:child_post_with_trait).name).to eq "override" end - it 'handles inline trait override' do - expect(FactoryBot.create(:child_user, :override_to_create).name).to eq 'override' - expect(FactoryBot.create(:child_post, :override_to_create).name).to eq 'override' + it "handles inline trait override" do + user = FactoryBot.create(:child_user, :override_to_create) + post = FactoryBot.create(:child_post, :override_to_create) + + expect(user.name).to eq "override" + expect(post.name).to eq "override" end - it 'uses to_create globally across FactoryBot.define' do - define_model('Company', name: :string) + it "uses to_create globally across FactoryBot.define" do + define_model("Company", name: :string) FactoryBot.define do factory :company end - expect(FactoryBot.create(:company).name).to eq 'persisted!' - expect(FactoryBot.create(:company, :override_to_create).name).to eq 'override' + company = FactoryBot.create(:company) + override_company = FactoryBot.create(:company, :override_to_create) + + expect(company.name).to eq "persisted!" + expect(override_company.name).to eq "override" end end -describe 'global skip_create' do +describe "global skip_create" do before do - define_model('User', name: :string) - define_model('Post', name: :string) + define_model("User", name: :string) + define_model("Post", name: :string) FactoryBot.define do skip_create trait :override_to_create do - to_create { |instance| instance.name = 'override' } + to_create { |instance| instance.name = "override" } end factory :user do - name { 'John Doe' } + name { "John Doe" } factory :child_user @@ -87,7 +93,7 @@ describe 'global skip_create' do end factory :post do - name { 'Great title' } + name { "Great title" } factory :child_post diff --git a/spec/acceptance/lint_spec.rb b/spec/acceptance/lint_spec.rb index 396be85..8031e8e 100644 --- a/spec/acceptance/lint_spec.rb +++ b/spec/acceptance/lint_spec.rb @@ -1,10 +1,10 @@ -describe 'FactoryBot.lint' do - it 'raises when a factory is invalid' do - define_model 'User', name: :string do +describe "FactoryBot.lint" do + it "raises when a factory is invalid" do + define_model "User", name: :string do validates :name, presence: true end - define_model 'AlwaysValid' + define_model "AlwaysValid" FactoryBot.define do factory :user do @@ -26,26 +26,26 @@ The following factories are invalid: end.to raise_error FactoryBot::InvalidFactoryError, error_message end - it 'does not raise when all factories are valid' do - define_model 'User', name: :string do + it "does not raise when all factories are valid" do + define_model "User", name: :string do validates :name, presence: true end FactoryBot.define do factory :user do - name { 'assigned' } + name { "assigned" } end end expect { FactoryBot.lint }.not_to raise_error end - it 'allows for selective linting' do - define_model 'InvalidThing', name: :string do + it "allows for selective linting" do + define_model "InvalidThing", name: :string do validates :name, presence: true end - define_model 'ValidThing', name: :string + define_model "ValidThing", name: :string FactoryBot.define do factory :valid_thing diff --git a/spec/acceptance/static_attribute_deprecation_spec.rb b/spec/acceptance/static_attribute_deprecation_spec.rb deleted file mode 100644 index 46cc90f..0000000 --- a/spec/acceptance/static_attribute_deprecation_spec.rb +++ /dev/null @@ -1,37 +0,0 @@ -describe "static attribute deprecation warnings" do - context "using #add_attribute" do - it "prints where attribute is declared" do - define_model("User", name: :string) - - declare_factory = Proc.new do - FactoryBot.define do - factory :user do - add_attribute(:name, "alice") - end - end - end - - expect(declare_factory).to output( - /called from .*static_attribute_deprecation_spec\.rb:9/m - ).to_stderr - end - end - - context "an implicit attribute via method missing" do - it "prints where attribute is declared" do - define_model("User", name: :string) - - declare_factory = Proc.new do - FactoryBot.define do - factory :user do - name "Alice" - end - end - end - - expect(declare_factory).to output( - /called from .*static_attribute_deprecation_spec\.rb:27/m - ).to_stderr - end - end -end diff --git a/spec/factory_bot/attribute/static_spec.rb b/spec/factory_bot/attribute/static_spec.rb deleted file mode 100644 index 37c571e..0000000 --- a/spec/factory_bot/attribute/static_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -describe FactoryBot::Attribute::Static do - let(:name) { :first_name } - let(:value) { "John" } - - subject { FactoryBot::Attribute::Static.new(name, value, false) } - - its(:name) { should eq name } - - it "returns the value when executing the proc" do - expect(subject.to_proc.call).to eq value - end -end - -describe FactoryBot::Attribute::Static, "with a string name" do - subject { FactoryBot::Attribute::Static.new("name", nil, false) } - its(:name) { should eq :name } -end diff --git a/spec/factory_bot/attribute_list_spec.rb b/spec/factory_bot/attribute_list_spec.rb index 243b503..2450898 100644 --- a/spec/factory_bot/attribute_list_spec.rb +++ b/spec/factory_bot/attribute_list_spec.rb @@ -1,24 +1,30 @@ describe FactoryBot::AttributeList, "#define_attribute" do - let(:static_attribute) { FactoryBot::Attribute::Static.new(:full_name, "value", false) } - let(:dynamic_attribute) { FactoryBot::Attribute::Dynamic.new(:email, false, ->(u) { "#{u.full_name}@example.com" }) } - it "maintains a list of attributes" do - subject.define_attribute(static_attribute) - expect(subject.to_a).to eq [static_attribute] + attribute = double(:attribute, name: :attribute_name) + another_attribute = double(:attribute, name: :another_attribute_name) - subject.define_attribute(dynamic_attribute) - expect(subject.to_a).to eq [static_attribute, dynamic_attribute] + subject.define_attribute(attribute) + expect(subject.to_a).to eq [attribute] + + subject.define_attribute(another_attribute) + expect(subject.to_a).to eq [attribute, another_attribute] end it "returns the attribute" do - expect(subject.define_attribute(static_attribute)).to eq static_attribute - expect(subject.define_attribute(dynamic_attribute)).to eq dynamic_attribute + attribute = double(:attribute, name: :attribute_name) + + expect(subject.define_attribute(attribute)).to eq attribute end it "raises if an attribute has already been defined" do + attribute = double(:attribute, name: :attribute_name) + expect { - 2.times { subject.define_attribute(static_attribute) } - }.to raise_error(FactoryBot::AttributeDefinitionError, "Attribute already defined: full_name") + 2.times { subject.define_attribute(attribute) } + }.to raise_error( + FactoryBot::AttributeDefinitionError, + "Attribute already defined: attribute_name", + ) end end @@ -38,33 +44,29 @@ describe FactoryBot::AttributeList, "#define_attribute with a named attribute li end describe FactoryBot::AttributeList, "#apply_attributes" do - let(:full_name_attribute) { FactoryBot::Attribute::Static.new(:full_name, "John Adams", false) } - let(:city_attribute) { FactoryBot::Attribute::Static.new(:city, "Boston", false) } - let(:email_attribute) { FactoryBot::Attribute::Dynamic.new(:email, false, ->(model) { "#{model.full_name}@example.com" }) } - let(:login_attribute) { FactoryBot::Attribute::Dynamic.new(:login, false, ->(model) { "username-#{model.full_name}" }) } - def list(*attributes) FactoryBot::AttributeList.new.tap do |list| attributes.each { |attribute| list.define_attribute(attribute) } end end - it "adds attributes in the order defined regardless of attribute type" do - subject.define_attribute(full_name_attribute) - subject.define_attribute(login_attribute) - subject.apply_attributes(list(city_attribute, email_attribute)) - expect(subject.to_a).to eq [full_name_attribute, login_attribute, city_attribute, email_attribute] + it "adds attributes in the order defined" do + attribute1 = double(:attribute1, name: :attribute1) + attribute2 = double(:attribute2, name: :attribute2) + attribute3 = double(:attribute3, name: :attribute3) + + subject.define_attribute(attribute1) + subject.apply_attributes(list(attribute2, attribute3)) + expect(subject.to_a).to eq [attribute1, attribute2, attribute3] end end describe FactoryBot::AttributeList, "#associations" do - let(:full_name_attribute) { FactoryBot::Attribute::Static.new(:full_name, "value", false) } let(:email_attribute) { FactoryBot::Attribute::Dynamic.new(:email, false, ->(u) { "#{u.full_name}@example.com" }) } let(:author_attribute) { FactoryBot::Attribute::Association.new(:author, :user, {}) } let(:profile_attribute) { FactoryBot::Attribute::Association.new(:profile, :profile, {}) } before do - subject.define_attribute(full_name_attribute) subject.define_attribute(email_attribute) subject.define_attribute(author_attribute) subject.define_attribute(profile_attribute) @@ -77,11 +79,11 @@ end describe FactoryBot::AttributeList, "filter based on ignored attributes" do def build_ignored_attribute(name) - FactoryBot::Attribute::Static.new(name, "value", true) + FactoryBot::Attribute::Dynamic.new(name, true, -> { "value" }) end def build_non_ignored_attribute(name) - FactoryBot::Attribute::Static.new(name, "value", false) + FactoryBot::Attribute::Dynamic.new(name, false, -> { "value" }) end before do @@ -103,11 +105,11 @@ end describe FactoryBot::AttributeList, "generating names" do def build_ignored_attribute(name) - FactoryBot::Attribute::Static.new(name, "value", true) + FactoryBot::Attribute::Dynamic.new(name, true, -> { "value" }) end def build_non_ignored_attribute(name) - FactoryBot::Attribute::Static.new(name, "value", false) + FactoryBot::Attribute::Dynamic.new(name, false, -> { "value" }) end def build_association(name) diff --git a/spec/factory_bot/definition_proxy_spec.rb b/spec/factory_bot/definition_proxy_spec.rb index 224ba78..d3dd5af 100644 --- a/spec/factory_bot/definition_proxy_spec.rb +++ b/spec/factory_bot/definition_proxy_spec.rb @@ -30,7 +30,9 @@ describe FactoryBot::DefinitionProxy, "#transient" do add_attribute(:attribute_name, &attribute_value) end - expect(subject).to have_dynamic_declaration(:attribute_name).ignored.with_value(attribute_value) + expect(subject).to have_dynamic_declaration(:attribute_name). + ignored. + with_value(attribute_value) end end diff --git a/spec/factory_bot/factory_spec.rb b/spec/factory_bot/factory_spec.rb index 7c99c7d..afcb121 100644 --- a/spec/factory_bot/factory_spec.rb +++ b/spec/factory_bot/factory_spec.rb @@ -60,20 +60,16 @@ describe FactoryBot::Factory do end it "returns the overridden value in the generated attributes" do - declaration = FactoryBot::Declaration::Static.new(@name, 'The price is wrong, Bob!') + declaration = + FactoryBot::Declaration::Dynamic.new(@name, false, -> { flunk }) @factory.declare_attribute(declaration) result = @factory.run(FactoryBot::Strategy::AttributesFor, @hash) expect(result[@name]).to eq @value end - it "does not call a lazy attribute block for an overridden attribute" do - declaration = FactoryBot::Declaration::Dynamic.new(@name, false, -> { flunk }) - @factory.declare_attribute(declaration) - @factory.run(FactoryBot::Strategy::AttributesFor, @hash) - end - it "overrides a symbol parameter with a string parameter" do - declaration = FactoryBot::Declaration::Static.new(@name, 'The price is wrong, Bob!') + declaration = + FactoryBot::Declaration::Dynamic.new(@name, false, -> { flunk }) @factory.declare_attribute(declaration) @hash = { @name.to_s => @value } result = @factory.run(FactoryBot::Strategy::AttributesFor, @hash) @@ -83,10 +79,16 @@ describe FactoryBot::Factory do describe "overriding an attribute with an alias" do before do - @factory.declare_attribute(FactoryBot::Declaration::Static.new(:test, 'original')) + attribute = FactoryBot::Declaration::Dynamic.new( + :test, + false, -> { "original" } + ) + @factory.declare_attribute(attribute) FactoryBot.aliases << [/(.*)_alias/, '\1'] - @result = @factory.run(FactoryBot::Strategy::AttributesFor, - test_alias: 'new') + @result = @factory.run( + FactoryBot::Strategy::AttributesFor, + test_alias: "new", + ) end it "uses the passed in value for the alias" do @@ -224,18 +226,22 @@ describe FactoryBot::Factory, "human names" do end describe FactoryBot::Factory, "running a factory" do - subject { FactoryBot::Factory.new(:user) } - let(:attribute) { FactoryBot::Attribute::Static.new(:name, "value", false) } - let(:declaration) { FactoryBot::Declaration::Static.new(:name, "value", false) } + subject { FactoryBot::Factory.new(:user) } + let(:attribute) do + FactoryBot::Attribute::Dynamic.new(:name, false, -> { "value" }) + end + let(:declaration) do + FactoryBot::Declaration::Dynamic.new(:name, false, -> { "value" }) + end let(:strategy) { double("strategy", result: "result", add_observer: true) } - let(:attributes) { [attribute] } + let(:attributes) { [attribute] } let(:attribute_list) do double("attribute-list", declarations: [declaration], to_a: attributes) end before do define_model("User", name: :string) - allow(FactoryBot::Declaration::Static).to receive(:new). + allow(FactoryBot::Declaration::Dynamic).to receive(:new). and_return declaration allow(declaration).to receive(:to_attributes).and_return attributes allow(FactoryBot::Strategy::Build).to receive(:new).and_return strategy diff --git a/spec/support/matchers/declaration.rb b/spec/support/matchers/declaration.rb index d9d9ad3..7714044 100644 --- a/spec/support/matchers/declaration.rb +++ b/spec/support/matchers/declaration.rb @@ -1,8 +1,4 @@ module DeclarationMatchers - def have_static_declaration(name) - DeclarationMatcher.new(:static).named(name) - end - def have_dynamic_declaration(name) DeclarationMatcher.new(:dynamic).named(name) end @@ -60,7 +56,6 @@ module DeclarationMatchers def expected_declaration case @declaration_type - when :static then FactoryBot::Declaration::Static.new(@name, @value, ignored?) when :dynamic then FactoryBot::Declaration::Dynamic.new(@name, ignored?, @value) when :implicit then FactoryBot::Declaration::Implicit.new(@name, @factory, ignored?) when :association