CODEWITHSUNDEEP
ruby-on-railsruby-on-rails-4rspec
yretuta
yretuta

Reputation: 8091

Why Does A Unit Test For destroy_all fail with let?

So I have this model code:

def self.cleanup
    Transaction.where("created_at < ?", 30.days.ago).destroy_all
  end

and this rspec unit test:

describe 'self.cleanup' do
    before(:each) do
      @transaction = Transaction.create(seller:item.user, buyer:user, item:item, created_at:6.weeks.ago)
    end

    it 'destroys all transactions more than 30 days' do
      Transaction.cleanup
      expect(@transaction).not_to exist_in_database
    end
  end

with these factories:

FactoryGirl.define do
  factory :transaction do
    association :seller, factory: :user, username: 'IAMSeller'
    association :buyer, factory: :user, username: 'IAmBuyer'
    association :item
  end

  factory :old_transaction, parent: :transaction do
    created_at 6.weeks.ago
  end
end

using this rspec custom matcher:

RSpec::Matchers.define :exist_in_database do
  match do |actual|
    actual.class.exists?(actual.id)
  end
end

When I change the spec to this:

describe 'self.cleanup' do
    let(:old_transaction){FactoryGirl.create(:old_transaction)}

    it 'destroys all transactions more than 30 days' do
      Transaction.cleanup
      expect(old_transaction).not_to exist_in_database
    end
  end

the test fails. I also tried manually creating a transaction and assigning it to :old_transaction with let() but that makes the test fail too.

Why is it that it only passes when I use an instance variable in the before(:each) block?

Thanks in advance!

EDIT: FAILED OUTPUT

1) Transaction self.cleanup destroys all transactions more than 30 days
     Failure/Error: expect(old_transaction).not_to exist_in_database
       expected #<Transaction id: 2, seller_id: 3, buyer_id: 4, item_id: 2, transaction_date: nil, created_at: "2014-02-26 10:06:30", updated_at: "2014-04-09 10:06:32", buyer_confirmed: false, seller_confirmed: false, cancelled: false> not to exist in database
     # ./spec/models/transaction_spec.rb:40:in `block (3 levels) in <top (required)>'

Upvotes: 1

Views: 1203

Answers (2)

Aaron K
Aaron K

Reputation: 6961

let is lazy loaded. So in your failing spec this is the order of events:

  1. Transaction.cleanup
  2. old_transaction = FactoryGirl.create(:old_transaction)
  3. expect(old_transaction).not_to exist_in_database

So the transaction is created after you attempt to clean up.

There are multiple options for you:

Don't use let for this

Unless you have other specs that you want to tell other devs:

I fully intend for all of these specs to reference what should be the exact same object

I personally feel, that you're better off inlining the transaction.

it do
  transaction = FactoryGirl.create(:old_transaction)

  Transaction.cleanup

  expect(transaction).not_to exist_in_database
end

Use the change matcher

This is my personal choice as it clearly demonstrates the intended behavior:

it do
  expect{
    Transaction.cleanup
  }.to change{ Transaction.exists?(old_transaction.id) }.to false
end

This works with let as the change block is run before AND after the expect block. So on the first pass the old_transaction is instantiated so it's id can be checked.

Use before or reference old_transaction before your cleanup

IMO this seems odd:

before do
  old_transaction
end

it do
  old_transaction # if you don't use the before
  Transaction.clean
  # ...
end

Use let!

The let! is not lazy loaded. Essentially, it's an alias for doing a normal let, then calling it in a before. I'm not a fan of this method (see The bang is for surprise for details why).

Upvotes: 5

gernberg
gernberg

Reputation: 2617

I think you've just accidentally typoed in a ":"

Try this spec:

describe 'self.cleanup' do
  let(:old_transaction){FactoryGirl.create(:old_transaction)}
  it 'destroys all transactions more than 30 days' do
    Transaction.cleanup
    expect(old_transaction).not_to exist_in_database
  end
end

Upvotes: 1

Related Questions