rom-rb / rom-factory

Data generator with support for persistence backends
MIT License
82 stars 42 forks source link

overriding factory attributes is causing related field callbacks to fail #63

Open lowang opened 3 years ago

lowang commented 3 years ago

Describe the bug

Overriding attributes is causing factory field resolver to receive nil instead of overwritten value.

To Reproduce

create file app_spec.rb with:

require 'rom'
require 'rom-factory'

ROM_CONTAINER = ROM.container(:sql, 'sqlite::memory') do |conf|
  conf.default.create_table(:users) do
    primary_key :id
    column :name, String, null: false
    column :email, String, null: false
  end

  class Users < ROM::Relation[:sql]
    schema(infer: true)
  end

  conf.register_relation(Users)
end

Factory = ROM::Factory.configure do |config|
  config.rom = ROM_CONTAINER
end

Factory.define(:user) do |f|
  f.name 'John'
  f.email { |name| name.downcase + '@example.com' }
end

RSpec.describe 'app' do
  let(:data) do
    { name: 'Alice' }
  end

  let(:user) { Factory[:user, data] }

  specify do
    pp user
  end
end

now rspec app_spec.rb is producing following error:

 NoMethodError:
       undefined method `downcase' for nil:NilClass
     # ./app_spec.rb:24:in `block (2 levels) in <top (required)>'
     # ./app_spec.rb:32:in `block (2 levels) in <top (required)>'
     # ./app_spec.rb:35:in `block (2 levels) in <top (required)>'

Expected behavior

I was expecting email callback to receive Alice value.

My environment

solnic commented 3 years ago

Thanks for reporting this, clearly a bug. I think we should re-think this API in general. Yielding attribute values via block args can be problematic when you have attributes that match ruby keywords.

flash-gordon commented 3 years ago

I still think having a convention for reserved words like do |then| -> do |then_| would be good enough

solnic commented 3 years ago

@flash-gordon yeah...seems like a pragmatic choice. I wonder though....maybe we could provide a struct with values that are lazy, this way you could still rely on more than one value but access it through one object.

flash-gordon commented 3 years ago

@solnic yeah I think it'd be possible too

yld commented 2 months ago

This makes ROM factory somewhat unusable