RSpec tests for a cash flow model

Question

This is my first model RSpec test:

require 'spec_helper'
describe CashFlow do
 context 'DB Fields' do
 it { should have_db_column :amount_cents }
 it { should have_db_column :amount_currency }
 it { should have_db_column :user_id }
 it { should have_db_column :target_date }
 it { should have_db_column :created_at }
 it { should have_db_column :updated_at }
 end
 context 'Associations' do
 it { should belong_to :user }
 it { should have_and_belong_to_many :tags }
 end
 context 'Validation' do
 it { should validate_presence_of :amount_cents }
 it { should validate_numericality_of :amount_cents }
 it { should ensure_exclusion_of(:amount_cents).in_array([0,]) }
 it { should validate_presence_of :amount_currency }
 it { should validate_presence_of :target_date }
 it { should validate_presence_of :user }
 end
 context 'Scopes' do
 subject { CashFlow }
 context 'Incoming/Outcoming flows' do
 let(:incoming_cash_flows) { 4.times.map { create :cash_flow } }
 let(:outcoming_cash_flows) { 4.times.map { create :cash_flow, negative: true } }
 it 'should be able to return all and only incomings cash_flows' do
 expect(subject.incoming).to include *incoming_cash_flows
 expect(subject.incoming).not_to include *outcoming_cash_flows
 end
 it 'should be able to return all and only outcomings cash_flows' do
 expect(subject.outcoming).to include *outcoming_cash_flows
 expect(subject.outcoming).not_to include *incoming_cash_flows
 end
 end
 context 'Past/Future flows' do
 [-1, nil, 1].each do |num|
 _threshold = num ? Date.today + num.year : nil
 let(:threshold) { _threshold }
 let(:future_cash_flows) { 4.times.map { create :cash_flow, threshold: threshold } }
 let(:past_cash_flows) { 4.times.map { create :cash_flow, past: true, threshold: threshold } }
 it "should be able to return all and only past cash_flows with #{_threshold || 'current date (default)'} threshold" do
 expect(subject.past(threshold)).to include *past_cash_flows
 expect(subject.past(threshold)).not_to include *future_cash_flows
 end
 it "should be able to return all and only future cash_flows with #{_threshold || 'current date (default)'} threshold" do
 expect(subject.future(threshold)).to include *future_cash_flows
 expect(subject.future(threshold)).not_to include *past_cash_flows
 end
 end
 end
 end
 it 'should save specified currency' do
 %w(GEL RUB USD EUR).each do |currency|
 cash_flow_id = create(:cash_flow, currency: currency).id
 cash_flow = CashFlow.find(cash_flow_id)
 expect(cash_flow.amount.currency).to eq currency
 end
 end
end

Model:

class CashFlow < ActiveRecord::Base
 belongs_to :user
 has_and_belongs_to_many :tags
 monetize :amount_cents, with_model_currency: :amount_currency
 validates_presence_of :user, :target_date, :amount_cents, :amount_currency
 validates_exclusion_of :amount_cents, in: [0, ]
 scope :incoming, -> { where 'amount_cents > 0' }
 scope :outcoming, -> { where 'amount_cents < 0' }
 scope :past, lambda { |date = nil | where 'target_date < ?', date || Date.today}
 scope :future, lambda { |date = nil | where 'target_date > ?', date || Date.today}
end

Factory:

FactoryGirl.define do
 factory :cash_flow do
 association :user
 ignore do
 negative false
 past false
 threshold nil
 currency 'USD'
 end
 after :build do |cash_flow, evaluator|
 sign = evaluator.negative ? -1 : 1
 cash_flow.amount = Money.new((rand(10000) + 1) * sign, evaluator.currency || currency)
 sign = evaluator.past ? -1 : 1
 cash_flow.target_date = (evaluator.threshold || Date.today) + ((rand(60 * 24 * 30) + 1) * sign).minutes
 end
 end
end

What can you tell me about its readability, flexibility and coverage fullness?

I use factory girl, shoulda and money gems.

Answer 1

First thing: congratulations on starting with rspec! Way to go.

My first remark would be to remove your three first contexts (Fields, Associations & Validations).

Why? Because those are not testing any parts of your code - they are testing ActiveModel functionalities. You want to test your code, not your librairies - at least, not without a very good reason to think that there is a problem there. This is time spent to write specs that do not provide any benefits.

On the other side, while your model don't have much logic yet, I like the fact that you tested your scopes - this is your logic, and it should be tested.

• Flambino: I would keep the validation tests. In terms of spec'ing a model, I'd say it's valid (no pun intended) to specify that it should perform certain validations. A model may well have an attribute that's not used anywhere in the business logic (and thus won't be tested indirectly), but its presence is required for external reasons.
• Martin: I would test the validations - but not as "is there validations" but as "should refuse an amount without currency" with an appropriate fixture. Now agree this maybe a question of preferences.
• Flambino: That's an even better way to go. But your answer didn't give an example of a spec that could replace or improve upon the shoulda tests (you did that in your comment just now, however), so it could be interpreted as simply "don't test validation"
• Martin: You are right. I'll update my answer accordingly. Thanks!
• atomAltera: Thank you guys, I myself thought that DB fields testing is excessive, but as for validation, in terms of TDD I think it should be done, especially with help of shoulda matchers, because they check is there a particular validation or not.

Answer 2

Sounds quite complete. I just do not like to have any logic in my tests that make it hard to debug, like you did when executed some test using array [-1, nil, 1] . Also make sense what first comment says, to do not test the framework, but do test your code.

• atomAltera: I used loop (iterating through array) in place of 3 quite similar tests, is this bad approach?
• jlucasps: I woundn't say bad, but just more complext. I just like my tests as simple as possible, that's why I don't like to use. Some books says that every test case should have only one assert, so when you iterate in a array you are creating multiple asserts. Let suppose you are considering a range of [-1, 0, 1], and that represents a negative, neutral and positive account balance, so I think is more clear if you create three different tests.