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: "!0$2-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: "!0$2-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

Note that David Chelimsky advises against the use of an explicit subject: blog.davidchelimsky.net/2012/05/13/… — Andy Waite
– Andy Waite, Commented Mar 3, 2013 at 20:26

Wayne Conrad · Accepted Answer · 2013-01-22 13:29:49Z

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: "!0$2-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: "!0$2-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

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
– Ioannis Tziligkakis, Commented Jan 23, 2013 at 12:07
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. — Ioannis Tziligkakis
– Ioannis Tziligkakis, Commented Jan 23, 2013 at 12:33
@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. — Wayne Conrad
– Wayne Conrad, Commented Jan 23, 2013 at 17:00
I see. I'm just starting to discover the ways of RSpec. Thanks a lot for the tips! — Ioannis Tziligkakis
– Ioannis Tziligkakis, Commented Jan 25, 2013 at 10:24

Stack Exchange Network

Testing a Content model

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Testing a Content model

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions