After learning about shoulda-matchers by answering another StackOverflow question on attribute accessibility tests (and thinking they were pretty awesome), I decided to try refactoring the model tests I did in The Rails Tutorial in an attempt to make them even more concise and thorough. I did this thanks to some inspiration from the documentation for modules Shoulda::Matchers::ActiveRecord
and Shoulda::Matchers::ActiveModel
, as well as this StackOverflow answer on structuring shoulda tests in models. However, there's still a few things I am not sure about, and I am wondering how these tests could be made better.
I will use the User spec in the Rails Tutorial as my example as it is the most detailed, and covers lots of areas that could be improved. The following code example has been changed from the original user_spec.rb, and replaces the code down until the describe "micropost associations"
line. The spec tests against the user.rb model, and its factory is defined in factories.rb.
spec/models/user_spec.rb
# == Schema Information
#
# Table name: users
#
# id :integer not null, primary key
# name :string(255)
# email :string(255)
# created_at :datetime not null
# updated_at :datetime not null
# password_digest :string(255)
# remember_token :string(255)
# admin :boolean default(FALSE)
#
# Indexes
#
# index_users_on_email (email) UNIQUE
# index_users_on_remember_token (remember_token)
#
require 'spec_helper'
describe User do
let(:user) { FactoryGirl.create(:user) }
subject { user }
describe "database schema" do
it { should have_db_column(:id).of_type(:integer)
.with_options(null: false) }
it { should have_db_column(:name).of_type(:string) }
it { should have_db_column(:email).of_type(:string) }
it { should have_db_column(:created_at).of_type(:datetime)
.with_options(null: false) }
it { should have_db_column(:updated_at).of_type(:datetime)
.with_options(null: false) }
it { should have_db_column(:password_digest).of_type(:string) }
it { should have_db_column(:remember_token).of_type(:string) }
it { should have_db_column(:admin).of_type(:boolean)
.with_options(default: false) }
it { should have_db_index(:email).unique(true) }
it { should have_db_index(:remember_token) }
end
describe "associations" do
it { should have_many(:microposts).dependent(:destroy) }
it { should have_many(:relationships).dependent(:destroy) }
it { should have_many(:followed_users).through(:relationships) }
it { should have_many(:reverse_relationships).class_name("Relationship")
.dependent(:destroy) }
it { should have_many(:followers).through(:reverse_relationships) }
end
describe "model attributes" do
it { should respond_to(:name) }
it { should respond_to(:email) }
it { should respond_to(:password_digest) }
it { should respond_to(:remember_token) }
it { should respond_to(:admin) }
it { should respond_to(:microposts) }
it { should respond_to(:relationships) }
it { should respond_to(:followed_users) }
it { should respond_to(:reverse_relationships) }
it { should respond_to(:followers) }
end
describe "virtual attributes and methods from has_secure_password" do
it { should respond_to(:password) }
it { should respond_to(:password_confirmation) }
it { should respond_to(:authenticate) }
end
describe "accessible attributes" do
it { should_not allow_mass_assignment_of(:password_digest) }
it { should_not allow_mass_assignment_of(:remember_token) }
it { should_not allow_mass_assignment_of(:admin) }
end
describe "instance methods" do
it { should respond_to(:feed) }
it { should respond_to(:following?) }
it { should respond_to(:follow!) }
it { should respond_to(:unfollow!) }
end
describe "initial state" do
it { should be_valid }
it { should_not be_admin }
its(:remember_token) { should_not be_blank }
its(:email) { should_not =~ /\p{Upper}/ }
end
describe "validations" do
context "for name" do
it { should validate_presence_of(:name) }
it { should_not allow_value(" ").for(:name) }
it { should ensure_length_of(:name).is_at_most(50) }
end
context "for email" do
it { should validate_presence_of(:email) }
it { should_not allow_value(" ").for(:email) }
it { should validate_uniqueness_of(:email).case_insensitive }
context "when email format is invalid" do
addresses = %w[user@foo,com user_at_foo.org example.user@foo.]
addresses.each do |invalid_address|
it { should_not allow_value(invalid_address).for(:email) }
end
end
context "when email format is valid" do
addresses = %w[[email protected] [email protected] [email protected] [email protected]]
addresses.each do |valid_address|
it { should allow_value(valid_address).for(:email) }
end
end
end
context "for password" do
it { should ensure_length_of(:password).is_at_least(6) }
it { should_not allow_value(" ").for(:password) }
context "when password doesn't match confirmation" do
it { should_not allow_value("mismatch").for(:password) }
end
end
context "for password_confirmation" do
it { should validate_presence_of(:password_confirmation) }
end
end
# ...
end
Some specific questions about these tests:
Shoulda::Matchers::ActiveRecord
module? Perhaps just the important indexes are worth testing...?should have_many
tests under "associations"
replace their corresponding should respond_to
tests under "model attributes"
? I can't tell whether the should have_many
test just looks for the relevant has_many
declaration in a model file or actually performs the same function as should respond_to
.1) The Shoulda::Matchers::ActiveRecord module has a lot more in it than just column and index matchers. I would dig around in the included classes a little and see what you can find. This is where the have_many
, belong_to
etc come from. For the record though, I see little value in most of what is in there.
2) Yes, macros such as have_many
test a lot more than whether or not a model responds to a method. From the source code, you can see exactly what it is testing:
def matches?(subject)
@subject = subject
association_exists? &&
macro_correct? &&
foreign_key_exists? &&
through_association_valid? &&
dependent_correct? &&
class_name_correct? &&
order_correct? &&
conditions_correct? &&
join_table_exists? &&
validate_correct?
end
3) Making the tests more readable and/or concise is definitely a subjective question to answer. Everyone will give you a different answer to this depending on their background and experience. I would personally get rid of all of the respond_to
tests and replace them with tests that have value. When someone looks at your tests, they should be able to understand the public API for that class. When I see that your objects respond_to something like "following?", I can make assumptions, but don't really know what it means. Does it take an argument? Does it return a boolean value? Is the object following something or is something following the object?
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With