r4noobie
r4noobie

Reputation: 41

Can I create invalid values with FactoryGirl?

I have email factory (spec/factories/email.rb):

FactoryGirl.define do
  factory :email, class: String do
    skip_create

    transient do
      username 'user'
      subdomain 'mail'
      domain_name 'example.com'

      host { [subdomain, domain_name].compact.join('.') }
    end

    trait(:with_blank_host) { host '' }
    trait(:with_blank_username) { username '' }

    initialize_with { new("#{username}@#{host}") }
  end
end

And I have spec (spec/models/user_spec.rb):

RSpec.describe User, type: :model do
  # ...

  it { is_expected.to_not allow_value(FactoryGirl.create(:email, :with_blank_host)).for(:email) }
  it { is_expected.to_not allow_value(FactoryGirl.create(:email, :with_blank_username)).for(:email) }
end

Is it correct to use FactoryGirl in this way? Is it a bad practices?

Upvotes: 3

Views: 165

Answers (1)

astreal
astreal

Reputation: 3513

Assuming the logic behind creating those email without host/username won't be use anywhere else but the tests. Why do you want to keep it in the factory?

Why not simply doing:

FactoryGirl.define do
  factory :email, class: String do
    skip_create

    transient do
      username 'user'
      subdomain 'mail'
      domain_name 'example.com'

      host { [subdomain, domain_name].compact.join('.') }
    end
    initialize_with { new("#{username}@#{host}") }
  end
end

RSpec.describe User, type: :model do
  # ...

  it 'should not allow blank host' do
    is_expected.to_not allow_value(FactoryGirl.create(:email, host: '')).for(:email)
  end

  it 'should not allow blank username' do
    is_expected.to_not allow_value(FactoryGirl.create(:email, username: '').for(:email)
  end
end

On top of that does it really make sense to have a factory to create a email string? why not simply using a string (This is way simpler):

is_expected.to_not allow_value(FactoryGirl.create(:email, host: '')).for(:email)

vs

is_expected.to_not allow_value('test@').for(:email)

If you want to have a consistent :email through your test, why not putting it in the User.

FactoryGirl.define do
  factory :user

    transient do
      username 'user'
      subdomain 'mail'
      domain_name 'example.com'

      host { [subdomain, domain_name].compact.join('.') }
    end

    before(:create) do |user, evaluator|
      user.email = "#{username}@#{host}"
    end
  end
end

But if the logic belongs to those particular tests, why abstracting all of this in factory instead of simply using a before/callback. As a rule of thumb, I only put logic that will be used across spec files in the factory, everything else is in the proper before/after callback

RSpec.describe User, type: :model do
  before :each do
    # logic before each test of the file
  end

  context 'email validation'

    before :each do
      # logic before each test concerning the email validation
      # useful to factor stuff that will be used multiple time
      # e.g.
      #  @email_ok = '[email protected]'
    end

    it 'should not allow blank host' do
      # test specific setup
      # useful for stuff only used in this test
      # e.g
      #  email_no_host = 'user@'

      # expectation
    end
  end
end

In short:

What you're doing would work of course. This is not bad practice per se. It just doesn't make much sense IMHO.

EDITED

You could also add an helper inside the test scope in order to not make the model too fat:

RSpec.describe User, type: :model do
  context 'email validation'
    def generate_email(**opts)
        options = {username: 'user', subdomain: 'mail', domain_name 'example.com'}.merge(opts)
        username = options[:username]
        host = options[:host] || "#{options[:subdomain]}.#{options[:domain_name]}"

        "#{username}@#{host}"
    end

    it 'should not allow blank host' do
      is_expected.to_not allow_value(generate_email host: '').for(:email)
    end

    # Here goes 100 other use of generate_email
  end
end

Upvotes: 1

Related Questions