Testing a Content model

Question

I'm new to RSpec and testing in general. I've come up with a spec for testing my Content model and I need some feedback because I think there are many improvements that can be done. I don't know if the way I did it is considered over-testing, bloated/wrong code or something. This test is kinda slow but this doesn't bother me that much for now.

app/models/content.rb

class Content < ActiveRecord::Base
 extend FriendlyId
 friendly_id :title, use: [ :slugged, :history ]
 acts_as_mediumable
 delegate :title, to: :category, prefix: true
 # Associations
 belongs_to :category
 has_many :slides, dependent: :destroy
 # Accessible attributes
 attr_accessible :title, :summary, :body, :category_id,
 :seo_description, :seo_keywords, :seo_title,
 :unpublished_at, :published_at, :is_draft
 # Validations
 validates :title, presence: true
 validates :body, presence: true
 validates :category, presence: true
 validates :published_at, timeliness: { allow_nil: false, allow_blank: false }
 validates :unpublished_at, timeliness: { allow_nil: true, allow_blank: true, after: :published_at }, :if => "published_at.present?"
 scope :published, lambda { |*args|
 now = ( args.first || Time.zone.now )
 where(is_draft: false).
 where("(published_at <= ? AND unpublished_at IS NULL) OR (published_at <= ? AND ? <= unpublished_at)", now, now, now).
 order("published_at DESC")
 }
 def self.blog_posts
 joins(:category).where(categories: { acts_as_blog: true })
 end
 def self.latest_post
 blog_posts.published.first
 end
 def to_s
 title
 end
 def seo
 meta = Struct.new(:title, :keywords, :description).new
 meta.title = seo_title.presence || title.presence
 meta.description = seo_description.presence || summary.presence
 meta
 end
end

spec/models/content_spec.rb

require 'spec_helper'
describe Content do
 it "has a valid factory" do
 create(:content).should be_valid
 end
 it "is invalid without a title" do
 build(:content, title: nil).should_not be_valid
 end
 it "is invalid without a body" do
 build(:content, body: nil).should_not be_valid
 end
 it "is invalid without a category" do
 build(:content, category: nil).should_not be_valid
 end
 it "is invalid when publication date is nil" do
 build(:content, published_at: nil).should_not be_valid
 end
 it "is invalid when publication date is blank" do
 build(:content, published_at: "").should_not be_valid
 end
 it "is invalid when publication date is malformed" do
 build(:content, published_at: "!02ドル-as-#{nil}").should_not be_valid
 end
 # TODO: You shall not pass! (for now)
 # it "is invalid when expiration date is malformed" do
 # build(:content, unpublished_at: "!02ドル-as-#{nil}").should_not be_valid
 # end
 it "is invalid when publication date is nil and expiration date is set" do
 build(:content, published_at: nil, unpublished_at: 3.weeks.ago).should_not be_valid
 end
 it "is invalid when expiration date is before publication date" do
 build(:content, published_at: 1.week.ago, unpublished_at: 2.weeks.ago).should_not be_valid
 end
 it "returns a content's title as a string" do
 content = create(:content)
 content.to_s.should eq content.title
 end
 describe "filters by publication dates" do
 before :each do
 @published_three_weeks_ago = create(:published_three_weeks_ago_content)
 @expiring_in_two_weeks = create(:expiring_in_two_weeks_content)
 @publish_in_tree_weeks = create(:publish_in_tree_weeks_content)
 end
 context "with matching dates" do
 it "returns a sorted array of results that match for current time" do
 Content.published.should include @published_three_weeks_ago, @expiring_in_two_weeks
 end
 it "returns a sorted array of results that match for future time" do
 Content.published(3.weeks.from_now).should include @published_three_weeks_ago, @publish_in_tree_weeks
 end
 end
 context "without matching dates" do
 it "returns an empty array" do
 Content.published(2.months.ago).should eq [ ]
 end
 end
 end
 describe "filters contents by blog category" do
 before :each do
 @blog_category = create(:blog_category)
 end
 context "with matching contents" do
 it "returns only blog posts" do
 one_page = create(:content)
 another_page = create(:content)
 first_post = create(:content, category: @blog_category)
 second_post = create(:content, category: @blog_category)
 Content.blog_posts.should include first_post, second_post
 end
 end
 context "without matching contents" do
 it "returns an empty array" do
 one_page = create(:content)
 another_page = create(:content)
 Content.blog_posts.should eq [ ]
 end
 end
 end
 describe "retrieves latest post" do
 before :each do
 @blog_category = create(:blog_category)
 end
 context "with existing posts" do
 it "return the latest content that belongs to a blog category" do
 first_post = create(:published_three_weeks_ago_content, category: @blog_category)
 second_post = create(:content, published_at: Time.zone.now, category: @blog_category)
 Content.latest_post.should eq second_post
 end
 end
 context "without existing posts" do
 it "returns an nil object" do
 Content.latest_post.should eq nil
 end
 end
 end
 describe "uses seo attributes when present" do
 before :each do
 @it = create(:content)
 end
 context "seo title present" do
 it "returns seo title when present" do
 @it.seo.title.should eq @it.seo_title
 end
 end
 context "seo title non present" do
 it "returns title when seo title is blank" do
 @it.seo_title = ""
 @it.seo.title.should eq @it.title
 end
 it "returns title when seo title is nil" do
 @it.seo_title = nil
 @it.seo.title.should eq @it.title
 end
 end
 context "seo description present" do
 it "returns seo description when present" do
 @it.seo.description.should eq @it.seo_description
 end
 end
 context "seo description non present" do
 it "returns description when seo description is blank" do
 @it.seo_description = ""
 @it.seo.description.should eq @it.summary
 end
 it "returns description when seo description is nil" do
 @it.seo_description = nil
 @it.seo.description.should eq @it.summary
 end
 end
 end
end

spec/factories/contents.rb

FactoryGirl.define do
 factory :content do
 association :category
 title { Faker::Lorem.sentence }
 summary { Faker::Lorem.sentence(10) }
 body { Faker::Lorem.sentence(15) }
 seo_title { Faker::Lorem.sentence }
 seo_description { Faker::Lorem.sentence }
 seo_keywords { Faker::Lorem.words(8).join(", ") }
 published_at { Time.zone.now }
 is_draft { false }
 factory :published_three_weeks_ago_content do
 published_at { 3.weeks.ago }
 end
 factory :expiring_in_two_weeks_content do
 unpublished_at { 2.weeks.from_now }
 end
 factory :publish_in_tree_weeks_content do
 published_at { 3.weeks.from_now }
 end
 end
end

• Andy Waite: Note that David Chelimsky advises against the use of an explicit subject: blog.davidchelimsky.net/2012/05/13/…

Answer 1

Nice job. Your tests are nicely compartmentalized. It is indeed good to test the factory independently. Good use of describe and context.

Consider using the shoulda-matchers gem

Tests for many of the rails model associations and validations can be handled by the shoulda-matchers gem. For example, this line:

validates :title, presence: true

can be tested like so:

it {should validate_presence_of(:title)}

Consider using pending

Here's a commented-out test:

# TODO: You shall not pass! (for now)
# it "is invalid when expiration date is malformed" do
# build(:content, unpublished_at: "!02ドル-as-#{nil}").should_not be_valid
# end

Rspec has a method for documenting tests that don't (and can't yet be made to) pass:

it "is invalid when expiration date is malformed" do
 pending
 build(:content, unpublished_at: "!02ドル-as-#{nil}").should_not be_valid
end

The nice thing about pending is that it shows up in the test output, making it less easily forgotten than commented-out code. Also, you can give a reason, e.g.:

pending "Can't pass until the vendor fixes library xyz"

For clarity, Consider redoing things the factory did

This test:

context "seo description present" do
 it "returns seo description when present" do
 @it.seo.description.should eq @it.seo_description
 end
end

Relies upon the factory having having set the description, but the factory is a long way from the test. This would be clearer if explicit:

context "seo description present" do
 it "returns seo description when present" do
 @it.seo_description = 'foo bar baz'
 @it.seo.description.should eq @it.seo_description
 end
end

Consider using subject

Some of your test sets a variable in a before block and later tests that variable:

before :each do @it = create(:content) end

context "seo title present" do
 it "returns seo title when present" do
 @it.seo.title.should eq @it.seo_title
 end
end

Instead of assigning to a variable, rspec lets you declare a subject:

subject {create(:content)}

Once you've declared a subject, some snazzy syntax becomes available to you:

its('seo.title') {should == subject.title}

subject.seo_title is a little awkward, so rspec lets you name your subject:

subject(:content) {create(:content)}
its('seo.title') {should == content.title}

Consider using let along with subject

In rspec, let defines a memoized, lazily-evaluated value. When used with subject, this can DRY up a spec:

describe "seo.title" do
 let(:title) {'title'}
 subject {create :content, :title => title, :seo_title => seo_title}
 context 'seo title present' do
 let(:seo_title) {'seo title'}
 its('seo.title') {should eq seo_title}
 end
 context 'seo title missing' do
 let(:seo_title) {nil}
 its('seo.title') {should eq title}
 end
end

• Ioannis Tziligkakis: Hey Wayne thanks for the reply. You pointed out some really useful stuff. The let/subject/its seems useful, so I'll have to take a look at the documentation. Cheers ;)
• Ioannis Tziligkakis: One more to get this right. When using subject here: subject {create :content, :title => title, :seo_title => seo_title}, does the seo_title gets evaluated when let(:seo_title) {'seo title'} is executed later on? Normally it would raise an error for using an undefined variable... Also here: its('seo.title') {should eq title} is title referring to subject.title or to the variable title set in the beggining, cause every time I tried something like this its('seo.title') {should eq subject.title} just got that subject is a string.
• Wayne Conrad: @gtz, Instead of its('seo.title') {should eq subject.title} try specify {subject.seo.title.should eq subject.title}. There are... quirks... with its ability to evaluate an arbitrary string.
• Ioannis Tziligkakis: I see. I'm just starting to discover the ways of RSpec. Thanks a lot for the tips!