HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyrailsMinor

Testing a Content model

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
contenttestingmodel

Problem

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  "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(:c

Solution

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

Code Snippets

validates :title, presence: true
it {should validate_presence_of(:title)}
# 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 expiration date is malformed" do
  pending
  build(:content, unpublished_at: "!0$2-as-#{nil}").should_not be_valid
end
pending "Can't pass until the vendor fixes library xyz"

Context

StackExchange Code Review Q#20792, answer score: 4

Revisions (0)

No revisions yet.