Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Cleanup Unit Tests

This commit attempts to reduce the number of calls to the DB, and
utilize RSpec's built-in 'subject' and 'described_class' methods.
  • Loading branch information...
commit b50d279859cacbe9ced0a4434a64db07325c7177 1 parent 41f4a26
Ryan Closner authored
View
6 app/models/group.rb
@@ -8,13 +8,15 @@ class Group < ActiveRecord::Base
accepts_nested_attributes_for :students, :allow_destroy=>true, :reject_if=>:all_blank
+ validates_presence_of :phone_number
+
validates_phone_number :phone_number
validates_phone_number :destination_phone_number
before_save :notify_reply_settings_changed
def send_message(message,sending_person,recipients=nil)
- begin
+ begin
recipients ||= students+[user] - [sending_person]
#send sms messages to sms recipients
@@ -52,7 +54,7 @@ def send_destination_message(message,recipient)
def notify_reply_settings_changed
if self.replies_all_changed?
message = "Reply settings for #{self.title} have changed. "
- message += if self.replies_all?
+ message += if self.replies_all?
"Your replies will now be sent to everyone in the group."
else
"Your replies will be sent to #{user.display_name} only."
View
18 app/models/student.rb
@@ -1,20 +1,26 @@
class Student < ActiveRecord::Base
belongs_to :group
+ belongs_to :active_checkin, :class_name=>'Checkin'
has_many :logged_messages, :as=>:sender, :dependent=>:nullify
has_many :answers
has_many :checkins
- belongs_to :active_checkin, :class_name=>'Checkin'
- #for the moment, we expect standard, US-style 10 digit area code/number. we assume a country code of 1.
+ validates_uniqueness_of :phone_number,
+ :scope => :group_id,
+ :allow_blank => true,
+ :allow_nil => true
+ validates_presence_of :name
+ #for the moment, we expect standard, US-style 10 digit area code/number. we assume a country code of 1.
validates_phone_number :phone_number, :allow_blank=>true
- validates_uniqueness_of :phone_number, :scope=>:group_id, :allow_blank=>true, :allow_nil=>true
-
- validates_presence_of :name
validate :notification_method_present?
+ private
+
def notification_method_present?
- errors.add(:base,"either phone_number or email must be present") unless [phone_number,email].any?(&:present?)
+ unless [phone_number, email].any?(&:present?)
+ errors.add(:base, "either phone_number or email must be present")
+ end
end
end
View
32 spec/models/destination_spec.rb
@@ -1,24 +1,28 @@
require 'spec_helper'
describe Destination do
- before(:each) do
- @destination = FactoryGirl.create(:destination)
- end
+ context "validates :hashtag" do
+ before { subject.hashtag = hashtag }
- describe "hashtag" do
- it "must be present" do
- @destination.hashtag = nil
- @destination.should_not be_valid
- @destination.errors.keys.should be_include(:hashtag)
+ context "when nil" do
+ let(:hashtag) { nil }
+ it { should have_errors_on(:hashtag).with_message("can't be blank") }
end
- it "must not start with a #, and must strip leading '#' if given" do
- #I don't know how to skip before_validation, to make sure it's actually invalid if it somehow gets through.
- @destination.hashtag = "#myhashtag"
- @destination.should be_valid
- @destination.hashtag.should_not =~ /^#/
+ context "when blank" do
+ let(:hashtag) { "" }
+ it { should have_errors_on(:hashtag).with_message("can't be blank") }
end
- end
+ context "when preceded by a '#'" do
+ let(:hashtag) { "#myhashtag" }
+
+ it { should be_valid }
+ it "strips the hashtag" do
+ subject.valid?
+ subject.hashtag.should eql("myhashtag")
+ end
+ end
+ end
end
View
51 spec/models/group_spec.rb
@@ -1,6 +1,36 @@
require 'spec_helper'
describe Group do
+
+ context "validates :phone_number" do
+ before { subject.phone_number = phone_number }
+
+ context "when nil" do
+ let(:phone_number) { nil }
+ it { should have_errors_on(:phone_number).with_message("can't be blank") }
+ end
+
+ context "when blank" do
+ let(:phone_number) { "" }
+ it { should have_errors_on(:phone_number).with_message("can't be blank") }
+ end
+
+ context "when non-numeric" do
+ let(:phone_number) { "abc123" }
+ it { should have_errors_on(:phone_number).with_message("phone number must be 10 digits, and of the form 'xxxxxxxxxx'") }
+ end
+
+ context "when formatted" do
+ let(:phone_number) { "(123) 456-7890" }
+ it { should_not have_errors_on(:phone_number) }
+ end
+
+ context "when unformatted" do
+ let(:phone_number) { "0123456789" }
+ it { should_not have_errors_on(:phone_number) }
+ end
+ end
+
before(:all) do
$outbound_flocky = '' unless $outbound_flocky
end
@@ -8,23 +38,6 @@
@group = FactoryGirl.create(:group)
end
- describe "their phone number" do
- it "may NOT be blank" do
- @group.phone_number=nil
- @group.should_not be_valid
- @group.phone_number=""
- @group.should_not be_valid
- end
- it "if present, must be valid" do
- @group.phone_number="abc123"
- @group.should_not be_valid
- @group.phone_number="(123) 456-7890"
- @group.should be_valid
- @group.phone_number="1234567891"
- @group.should be_valid
- end
- end
-
describe "send_message" do
before :each do
@email_student=FactoryGirl.create(:student,:email=>"abc@def.com", :phone_number=>nil)
@@ -34,8 +47,8 @@
end
it "should send emails to users without phone numbers" do
$outbound_flocky.stub(:message)
- mailer = mock(:mail)
- mailer.should_receive(:deliver)
+ mailer = mock(:mail)
+ mailer.should_receive(:deliver)
NotificationMailer.should_receive(:notification_email).with(/test message/,@email_student,@group).and_return(mailer)
@group.send_message("test message",@group.user)
end
View
80 spec/models/student_spec.rb
@@ -1,47 +1,71 @@
require 'spec_helper'
describe Student do
- before(:each) do
- @student = FactoryGirl.create(:student)
- end
- describe "their phone number" do
- it "must be 10 digits" do
- @student.phone_number.scan(/\d/).length.should == 10
+ context "validates :phone_number" do
+ let(:email) { nil }
+
+ before do
+ subject.phone_number = phone_number
+ subject.email = email
end
- it "must be in the canonical format" do
- @student.phone_number.should match(PhoneValidator::STORAGE_REGEX)
+
+ context "when :email is blank" do
+ context "when :phone_number is nil" do
+ let(:phone_number) { nil }
+ it { should have_errors_on(:base).with_message("either phone_number or email must be present") }
+ end
+
+ context "when :phone_number is blank" do
+ let(:phone_number) { "" }
+ it { should have_errors_on(:base).with_message("either phone_number or email must be present") }
+ end
end
- it "should automatically be converted to the canonical format, agnostic of valid-ish input" do
- FactoryGirl.create(:student, :phone_number=>"(555) 123-4567").phone_number.should == "5551234567"
+
+ context "when :email is present" do
+ let(:email) { "abc@def.com" }
+
+ context "when :phone_number is nil" do
+ let(:phone_number) { nil }
+ it { should_not have_errors_on(:phone_number) }
+ end
+
+ context "when :phone_number is blank" do
+ let(:phone_number) { "" }
+ it { should_not have_errors_on(:phone_number) }
+ end
end
- it "may not be unique, across different groups" do
- FactoryGirl.build(:student,:phone_number=>@student.phone_number,:group_id=>3).should be_valid
+
+ context "when non-numeric" do
+ let(:phone_number) { "abc123" }
+ it { should have_errors_on(:phone_number).with_message("phone number must be 10 digits, and of the form 'xxxxxxxxxx'") }
end
- it "must be unique in a group" do
- FactoryGirl.build(:student,:phone_number=>@student.phone_number,:group_id=>@student.group_id).should_not be_valid
+
+ context "when formatted" do
+ let(:phone_number) { "(123) 456-7890" }
+ it { should_not have_errors_on(:phone_number) }
end
- it "may be blank, if and only if an email is present" do
- @student.phone_number = nil
- @student.email = nil
- @student.should_not be_valid
- @student.email = "student@company.com"
- @student.should be_valid
+ context "when unformatted" do
+ let(:phone_number) { "0123456789" }
+ it { should_not have_errors_on(:phone_number) }
end
end
- describe "their name" do
- it "must be present" do
- FactoryGirl.build(:student,:name=>nil).should_not be_valid
- end
- it "can't be set to blank" do
- FactoryGirl.build(:student,:name=>"").should_not be_valid
+ context "validates :name" do
+ before { subject.name = name }
+
+ context "when nil" do
+ let(:name) { nil }
+ it { should have_errors_on(:name).with_message("can't be blank") }
end
- it "might not be unique" do
- FactoryGirl.create(:student,:name=>@student.name).should be_valid
+
+ context "when blank" do
+ let(:name) { "" }
+ it { should have_errors_on(:name).with_message("can't be blank") }
end
end
+
describe "their group memberships" do
pending "students can be members of no groups" do
end
View
2  spec/spec_helper.rb
@@ -52,5 +52,7 @@ def login(user=FactoryGirl.create(:user))
Spork.each_run do
# This code will be run each time you run your specs.
+ Dir[Rails.root.join("spec/support/**/*.rb") ].each { |f| require f }
+
end
View
48 spec/support/matchers/errors.rb
@@ -0,0 +1,48 @@
+module Classtalk
+ module Spec
+ module Matchers
+
+ RSpec::Matchers.define :have_errors_on do |field|
+ chain :with_message do |message|
+ @message = message
+ end
+
+ match do |model|
+ model.valid?
+ @has_error = has_error?(model, field)
+
+ if @message
+ @has_error && model.errors[field].include?(@message)
+ else
+ @has_error
+ end
+ end
+
+ failure_message_for_should do |model|
+ msg = "#{model} should #{description}"
+
+ if @has_error && @message
+ msg << " while it is #{model.errors[field].inspect}"
+ end
+
+ msg
+ end
+
+ failure_message_for_should_not do |model|
+ "#{model.class} should not have an error on field #{field.inspect}"
+ end
+
+ description do |model|
+ msg = "have an error on field #{field.inspect}"
+ msg << " with a message #{@message.inspect}" if @message
+ msg
+ end
+
+ def has_error?(model, field)
+ !model.errors[field].blank?
+ end
+ end
+
+ end
+ end
+end

0 comments on commit b50d279

Please sign in to comment.
Something went wrong with that request. Please try again.