From b007ba148734eb523a251345beeb253b43725eec Mon Sep 17 00:00:00 2001 From: Sarah Mei Date: Tue, 19 Oct 2010 22:57:55 -0700 Subject: [PATCH 1/8] Fixed cucumber features. Backfilled tests for AspectsController#create. --- app/controllers/aspects_controller.rb | 3 +- spec/controllers/aspects_controller_spec.rb | 38 +++++++++++++++++---- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/app/controllers/aspects_controller.rb b/app/controllers/aspects_controller.rb index 35ddbaec9..53a696a47 100644 --- a/app/controllers/aspects_controller.rb +++ b/app/controllers/aspects_controller.rb @@ -20,10 +20,11 @@ class AspectsController < ApplicationController @aspect = current_user.aspect(params[:aspect]) if @aspect.valid? flash[:notice] = I18n.t('aspects.create.success') + respond_with @aspect else flash[:error] = I18n.t('aspects.create.failure') + redirect_to aspects_manage_path end - respond_with @aspect end def new diff --git a/spec/controllers/aspects_controller_spec.rb b/spec/controllers/aspects_controller_spec.rb index 19bb7bf5d..81571fb54 100644 --- a/spec/controllers/aspects_controller_spec.rb +++ b/spec/controllers/aspects_controller_spec.rb @@ -5,7 +5,8 @@ require 'spec_helper' describe AspectsController do - render_views + render_views + before do @user = Factory.create(:user) @user.aspect(:name => "lame-os") @@ -13,11 +14,36 @@ describe AspectsController do sign_in :user, @user end - it "on index sets a variable containing all a user's friends when a user is signed in" do - sign_in :user, @user - Factory.create :person - get :index - assigns[:friends].should == @user.friends + describe "#index" do + it "assigns @friends to all the user's friends" do + Factory.create :person + get :index + assigns[:friends].should == @user.friends + end end + describe "#create" do + describe "with valid params" do + it "creates an aspect" do + @user.aspects.count.should == 1 + post :create, "aspect" => {"name" => "new aspect"} + @user.reload.aspects.count.should == 2 + end + it "redirects to the aspect page" do + post :create, "aspect" => {"name" => "new aspect"} + response.should redirect_to(aspect_path(Aspect.find_by_name("new aspect"))) + end + end + describe "with invalid params" do + it "does not create an aspect" do + @user.aspects.count.should == 1 + post :create, "aspect" => {"name" => ""} + @user.reload.aspects.count.should == 1 + end + it "goes back to manage aspects" do + post :create, "aspect" => {"name" => ""} + response.should redirect_to(aspects_manage_path) + end + end + end end From 43dd95147eccaa741d930c1377cf7dfd47286efc Mon Sep 17 00:00:00 2001 From: Raphael Date: Tue, 19 Oct 2010 18:38:39 -0700 Subject: [PATCH 2/8] Starting to remove database-cleaner --- spec/lib/diaspora_parser_spec.rb | 4 ++-- spec/misc_spec.rb | 23 +---------------------- spec/models/comments_spec.rb | 6 +++--- spec/spec_helper.rb | 7 +------ 4 files changed, 7 insertions(+), 33 deletions(-) diff --git a/spec/lib/diaspora_parser_spec.rb b/spec/lib/diaspora_parser_spec.rb index 3b9e090f9..3504cfc73 100644 --- a/spec/lib/diaspora_parser_spec.rb +++ b/spec/lib/diaspora_parser_spec.rb @@ -6,7 +6,7 @@ require 'spec_helper' describe Diaspora::Parser do before do - @user = Factory.create(:user, :email => "bob@aol.com") + @user = Factory.create(:user) @aspect = @user.aspect(:name => 'spies') @user3 = Factory.create :user @@ -20,7 +20,7 @@ describe Diaspora::Parser do end it 'should be able to correctly handle comments with person in db' do - person = Factory.create(:person, :diaspora_handle => "test@testing.com") + person = Factory.create(:person) post = Factory.create(:status_message, :person => @user.person) comment = Factory.build(:comment, :post => post, :person => person, :text => "Freedom!") xml = comment.to_diaspora_xml diff --git a/spec/misc_spec.rb b/spec/misc_spec.rb index 1db3f3f82..12f7837c1 100644 --- a/spec/misc_spec.rb +++ b/spec/misc_spec.rb @@ -5,34 +5,13 @@ require 'spec_helper' describe 'making sure the spec runner works' do - - it 'should not delete the database mid-spec' do - User.count.should == 0 - Factory.create(:user) - User.count.should == 1 - end - - it 'should make sure the last user no longer exsists' do - User.count.should == 0 - end - it 'should factory create a user with a person saved' do user = Factory.create(:user) loaded_user = User.first(:id => user.id) loaded_user.person.owner_id.should == user.id end - describe 'testing a before do block' do - before do - Factory.create(:user) - end - - it 'should have cleaned before the before do block runs' do - User.count.should == 1 - end - - end - describe '#friend_users' do + describe '#friend_users' do before do @user1 = Factory.create(:user) @aspect1 = @user1.aspect(:name => "losers") diff --git a/spec/models/comments_spec.rb b/spec/models/comments_spec.rb index b2f2d3c61..08252ada2 100644 --- a/spec/models/comments_spec.rb +++ b/spec/models/comments_spec.rb @@ -18,7 +18,7 @@ describe Comment do status.comments.should == [] @user.comment "Yeah, it was great", :on => status - StatusMessage.first.comments.first.text.should == "Yeah, it was great" + status.reload.comments.first.text.should == "Yeah, it was great" end it "should be able to comment on a person's status" do @@ -26,8 +26,8 @@ describe Comment do status = Factory.create(:status_message, :person => person) @user.comment "sup dog", :on => status - StatusMessage.first.comments.first.text.should == "sup dog" - StatusMessage.first.comments.first.person.should == @user.person + status.reload.comments.first.text.should == "sup dog" + status.reload.comments.first.person.should == @user.person end it 'should not send out comments when we have no people' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 552fd884d..9bc58c2e6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -27,19 +27,14 @@ RSpec.configure do |config| config.before(:suite) do DatabaseCleaner.clean_with(:truncation) + DatabaseCleaner.clean stub_signature_verification - end config.before(:each) do - DatabaseCleaner.start stub_sockets User.stub!(:allowed_email?).and_return(:true) end - - config.after(:each) do - DatabaseCleaner.clean - end end ImageUploader.enable_processing = false From 52f7350f78bff309b9e6a2d49629d11dbc88c8e3 Mon Sep 17 00:00:00 2001 From: Raphael Date: Tue, 19 Oct 2010 18:54:18 -0700 Subject: [PATCH 3/8] Fix a couple more specs --- spec/lib/diaspora_parser_spec.rb | 4 +--- spec/models/photo_spec.rb | 7 +++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/spec/lib/diaspora_parser_spec.rb b/spec/lib/diaspora_parser_spec.rb index 3504cfc73..33efe2f0a 100644 --- a/spec/lib/diaspora_parser_spec.rb +++ b/spec/lib/diaspora_parser_spec.rb @@ -54,9 +54,7 @@ describe Diaspora::Parser do retraction = Retraction.for(message) xml = retraction.to_diaspora_xml - StatusMessage.count.should == 1 - @user.receive xml, person - StatusMessage.count.should == 0 + proc {@user.receive xml, person}.should change(StatusMessage, :count).by(-1) end it "should create a new person upon getting a person request" do diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index de70ce030..701bf079d 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -51,8 +51,7 @@ describe Photo do it 'should have a caption' do @photo.image.store! File.open(@fixture_name) @photo.caption = "cool story, bro" - @photo.save - Photo.first.caption.should == "cool story, bro" + @photo.save.should be_true end it 'should remove its reference in user profile if it is referred' do @@ -63,9 +62,9 @@ describe Photo do @user.save @user.person.save - User.first.profile.image_url.should == @photo.image.url(:thumb_medium) + @user.profile.image_url.should == @photo.image.url(:thumb_medium) @photo.destroy - User.first.profile.image_url.should be nil + @user.profile.image_url.should be nil end it 'should not use the imported filename as the url' do From 74ec629b018c9ffa8b625f8dc478e3f9bee9df16 Mon Sep 17 00:00:00 2001 From: Raphael Date: Tue, 19 Oct 2010 19:28:49 -0700 Subject: [PATCH 4/8] user_friending_spec passes again --- spec/models/photo_spec.rb | 2 +- spec/models/user/user_friending_spec.rb | 101 +++++++++++------------- 2 files changed, 46 insertions(+), 57 deletions(-) diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index 701bf079d..51b1c058d 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -64,7 +64,7 @@ describe Photo do @user.profile.image_url.should == @photo.image.url(:thumb_medium) @photo.destroy - @user.profile.image_url.should be nil + @user.reload.profile.image_url.should be nil end it 'should not use the imported filename as the url' do diff --git a/spec/models/user/user_friending_spec.rb b/spec/models/user/user_friending_spec.rb index 8bcc093f1..2756c0841 100644 --- a/spec/models/user/user_friending_spec.rb +++ b/spec/models/user/user_friending_spec.rb @@ -28,10 +28,9 @@ describe User do it "should be able to accept a pending friend request" do r = Request.instantiate(:to => user.receive_url, :from => friend) r.save - Person.all.count.should == 2 - Request.for_user(user).all.count.should == 1 - user.accept_friend_request(r.id, aspect.id) - Request.for_user(user).all.count.should == 0 + + proc {user.accept_friend_request(r.id, aspect.id)}.should change{ + Request.for_user(user).all.count}.by(-1) end it 'should be able to ignore a pending friend request' do @@ -39,12 +38,8 @@ describe User do r = Request.instantiate(:to => user.receive_url, :from => friend) r.save - Person.count.should == 2 - - user.ignore_friend_request(r.id) - - Person.count.should == 2 - Request.count.should == 0 + proc{user.ignore_friend_request(r.id)}.should change{ + Request.for_user(user).count}.by(-1) end it 'should not be able to friend request an existing friend' do @@ -80,59 +75,53 @@ describe User do @request_three.destroy end - it 'should befriend the user other user on the same pod' do - user2.receive @req_three_xml, user.person - user2.pending_requests.size.should be 1 - user2.accept_friend_request @request_three.id, aspect2.id - user2.friends.include?(user.person).should be true - Person.all.count.should be 3 + context 'request from one remote person to one local user' do + before do + user2.receive @req_three_xml, user.person + end + it 'should befriend the user other user on the same pod' do + proc{ + user2.accept_friend_request @request_three.id, aspect2.id + }.should_not change(Person, :count) + user2.friends.include?(user.person).should be true + end + + it 'should not delete the ignored user on the same pod' do + proc{ + user2.ignore_friend_request @request_three.id + }.should_not change(Person, :count) + user2.friends.include?(user.person).should be false + end end + context 'Two users receiving requests from one person' do + before do + user.receive @req_xml, person_one - it 'should not delete the ignored user on the same pod' do - user2.receive @req_three_xml, user.person - user2.pending_requests.size.should be 1 - user2.ignore_friend_request @request_three.id - user2.friends.include?(user.person).should be false - Person.all.count.should be 3 - end + user2.receive @req_two_xml, person_one + end + it 'should both users should befriend the same person' do + user.accept_friend_request @request.id, aspect.id + user.friends.include?(person_one).should be true - it 'should both users should befriend the same person' do - user.receive @req_xml, person_one - user.pending_requests.size.should be 1 - user.accept_friend_request @request.id, aspect.id - user.friends.include?(person_one).should be true + user2.accept_friend_request @request_two.id, aspect2.id + user2.friends.include?(person_one).should be true + end - user2.receive @req_two_xml, person_one - user2.pending_requests.size.should be 1 - user2.accept_friend_request @request_two.id, aspect2.id - user2.friends.include?(person_one).should be true - Person.all.count.should be 3 - end + it 'should keep the person around if one of the users rejects him' do + user.accept_friend_request @request.id, aspect.id + user.friends.include?(person_one).should be true - it 'should keep the person around if one of the users rejects him' do - user.receive @req_xml, person_one - user.pending_requests.size.should be 1 - user.accept_friend_request @request.id, aspect.id - user.friends.include?(person_one).should be true + user2.ignore_friend_request @request_two.id + user2.friends.include?(person_one).should be false + end - user2.receive @req_two_xml, person_one - user2.pending_requests.size.should be 1 - user2.ignore_friend_request @request_two.id - user2.friends.include?(person_one).should be false - Person.all.count.should be 3 - end + it 'should keep the person around if the users ignores them' do + user.ignore_friend_request user.pending_requests.first.id + user.friends.include?(person_one).should be false - it 'should keep the person around if the users ignores them' do - user.receive @req_xml, person_one - user.pending_requests.size.should be 1 - user.ignore_friend_request user.pending_requests.first.id - user.friends.include?(person_one).should be false - - user2.receive @req_two_xml, person_one - user2.pending_requests.size.should be 1 - user2.ignore_friend_request user2.pending_requests.first.id #@request_two.id - user2.friends.include?(person_one).should be false - Person.all.count.should be 3 + user2.ignore_friend_request user2.pending_requests.first.id #@request_two.id + user2.friends.include?(person_one).should be false + end end end From 197bd8eb49140352ce0ca6459db263beda18311c Mon Sep 17 00:00:00 2001 From: Raphael Date: Tue, 19 Oct 2010 22:55:11 -0700 Subject: [PATCH 5/8] make user/receive spec less repetitive --- spec/models/user/receive_spec.rb | 82 ++++++++------------------------ 1 file changed, 19 insertions(+), 63 deletions(-) diff --git a/spec/models/user/receive_spec.rb b/spec/models/user/receive_spec.rb index 1d97cdad1..779919587 100644 --- a/spec/models/user/receive_spec.rb +++ b/spec/models/user/receive_spec.rb @@ -47,96 +47,52 @@ describe User do end describe 'post refs' do - it "should add a received post to the aspect and visible_posts array" do - status_message = user.post :status_message, :message => "hi", :to =>aspect.id + before do + @status_message = user2.post :status_message, :message => "hi", :to =>aspect2.id + user.receive @status_message.to_diaspora_xml, user2.person user.reload - salmon = user.salmon(status_message).xml_for user2.person - user2.receive_salmon salmon - user2.reload - user2.raw_visible_posts.include?(status_message).should be true - aspect2.reload - aspect2.posts.include?(status_message).should be_true + end + + it "should add a received post to the aspect and visible_posts array" do + user.raw_visible_posts.include?(@status_message).should be true + aspect.reload + aspect.posts.include?(@status_message).should be_true end it 'should be removed on unfriending' do - status_message = user2.post :status_message, :message => "hi", :to => aspect2.id - user.receive status_message.to_diaspora_xml, user2.person - user.reload - - user.raw_visible_posts.count.should == 1 - user.unfriend(user2.person) - user.reload user.raw_visible_posts.count.should == 0 - - Post.count.should be 1 end it 'should be remove a post if the noone links to it' do - status_message = user2.post :status_message, :message => "hi", :to => aspect2.id - user.receive status_message.to_diaspora_xml, user2.person - user.reload - - user.raw_visible_posts.count.should == 1 - person = user2.person user2.delete - user.unfriend(person) + lambda {user.unfriend(person)}.should change(Post, :count).by(-1) user.reload user.raw_visible_posts.count.should == 0 - - Post.count.should be 0 end it 'should keep track of user references for one person ' do - status_message = user2.post :status_message, :message => "hi", :to => aspect2.id - user.receive status_message.to_diaspora_xml, user2.person - user.reload - - user.raw_visible_posts.count.should == 1 - - status_message.reload - status_message.user_refs.should == 1 + @status_message.reload + @status_message.user_refs.should == 1 user.unfriend(user2.person) - status_message.reload - - user.reload - user.raw_visible_posts.count.should == 0 - - status_message.reload - status_message.user_refs.should == 0 - - Post.count.should be 1 + @status_message.reload + @status_message.user_refs.should == 0 end it 'should not override userrefs on receive by another person' do user3.activate_friend(user2.person, aspect3) + user3.receive @status_message.to_diaspora_xml, user2.person - status_message = user2.post :status_message, :message => "hi", :to => aspect2.id - user.receive status_message.to_diaspora_xml, user2.person - - user3.receive status_message.to_diaspora_xml, user2.person - user.reload - user3.reload - - user.raw_visible_posts.count.should == 1 - - status_message.reload - status_message.user_refs.should == 2 + @status_message.reload + @status_message.user_refs.should == 2 user.unfriend(user2.person) - status_message.reload - - user.reload - user.raw_visible_posts.count.should == 0 - - status_message.reload - status_message.user_refs.should == 1 - - Post.count.should be 1 + @status_message.reload + @status_message.user_refs.should == 1 end end From d98ac7015580a16eb2fd46431fdeddf2a8f1b591 Mon Sep 17 00:00:00 2001 From: Raphael Date: Tue, 19 Oct 2010 23:29:22 -0700 Subject: [PATCH 6/8] Clean up some repetition in specs --- spec/models/person_spec.rb | 52 ++++++++---------------------- spec/models/post_spec.rb | 3 +- spec/models/status_message_spec.rb | 2 +- spec/models/user/receive_spec.rb | 7 ++-- 4 files changed, 17 insertions(+), 47 deletions(-) diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index c67e55f8a..edc5b20ee 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -77,64 +77,38 @@ describe Person do person_two.owns?(person_message).should be false end - it 'should delete all of user posts except comments upon user deletion' do + it "deletes all of a person's posts upon person deletion" do person = Factory.create(:person) - Factory.create(:status_message, :person => person) - Factory.create(:status_message, :person => person) - Factory.create(:status_message, :person => person) - Factory.create(:status_message, :person => person) + status = Factory.create(:status_message, :person => person) + Factory.create(:status_message, :person => @person) + + lambda {person.destroy}.should change(Post, :count).by(-1) + end + + it "does not delete a person's comments on person deletion" do + person = Factory.create(:person) status_message = Factory.create(:status_message, :person => @person) - Factory.create(:comment, :person_id => person.id, :text => "yes i do", :post => status_message) Factory.create(:comment, :person_id => person.id, :text => "i love you", :post => status_message) - Factory.create(:comment, :person_id => person.id, :text => "hello", :post => status_message) Factory.create(:comment, :person_id => @person.id, :text => "you are creepy", :post => status_message) - - person.destroy - - Post.count.should == 1 - Comment.all.count.should == 4 - status_message.comments.count.should == 4 + + lambda {person.destroy}.should_not change(Comment, :count) end describe "unfriending" do it 'should not delete an orphaned friend' do - request = @user.send_friend_request_to @person, @aspect - @user.activate_friend(@person, @aspect) - @user.reload - Person.all.count.should == 3 - @user.friends.count.should == 1 - @user.unfriend(@person) - @user.reload - @user.friends.count.should == 0 - Person.all.count.should == 3 + lambda {@user.unfriend(@person)}.should_not change(Person, :count) end it 'should not delete an un-orphaned friend' do - request = @user.send_friend_request_to @person, @aspect - request2 = @user2.send_friend_request_to @person, @aspect2 - @user.activate_friend(@person, @aspect) @user2.activate_friend(@person, @aspect2) - @user.reload - @user2.reload - - Person.all.count.should == 3 - @user.friends.count.should == 1 - @user2.friends.count.should == 1 - - @user.unfriend(@person) - @user.reload - @user2.reload - @user.friends.count.should == 0 - @user2.friends.count.should == 1 - - Person.all.count.should == 3 + lambda {@user.unfriend(@person)}.should_not change(Person, :count) end end diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index bc445af99..4c3daf4b6 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -6,8 +6,7 @@ require 'spec_helper' describe Post do before do - @user = Factory.create(:user, :email => "bob@aol.com") - @user.person.save + @user = Factory.create(:user) end describe 'xml' do diff --git a/spec/models/status_message_spec.rb b/spec/models/status_message_spec.rb index 2d8d0c5bf..8449b964c 100644 --- a/spec/models/status_message_spec.rb +++ b/spec/models/status_message_spec.rb @@ -6,7 +6,7 @@ require 'spec_helper' describe StatusMessage do before do - @user = Factory.create(:user, :email => "bob@aol.com") + @user = Factory.create(:user) @aspect = @user.aspect(:name => "losers") end diff --git a/spec/models/user/receive_spec.rb b/spec/models/user/receive_spec.rb index 779919587..c8e4891de 100644 --- a/spec/models/user/receive_spec.rb +++ b/spec/models/user/receive_spec.rb @@ -22,16 +22,13 @@ describe User do it 'should be able to parse and store a status message from xml' do status_message = user2.post :status_message, :message => 'store this!', :to => aspect2.id - person = user2.person xml = status_message.to_diaspora_xml user2.destroy status_message.destroy - StatusMessage.all.size.should == 0 - user.receive xml , user2.person - Post.all(:person_id => person.id).first.message.should == 'store this!' - StatusMessage.all.size.should == 1 + user + lambda {user.receive xml , user2.person}.should change (Post,:count).by(1) end it 'should not create new aspects on message receive' do From d3d01821644fcc0a1306998b2f3deda857d623b3 Mon Sep 17 00:00:00 2001 From: Raphael Date: Tue, 19 Oct 2010 23:29:36 -0700 Subject: [PATCH 7/8] Put database cleaner back in --- spec/spec_helper.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9bc58c2e6..218def68b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -26,14 +26,12 @@ RSpec.configure do |config| DatabaseCleaner.orm = "mongo_mapper" config.before(:suite) do - DatabaseCleaner.clean_with(:truncation) - DatabaseCleaner.clean stub_signature_verification end config.before(:each) do stub_sockets - User.stub!(:allowed_email?).and_return(:true) + DatabaseCleaner.clean end end From a01c1c74617f32a5315abeb3aaf2090c2a80ac98 Mon Sep 17 00:00:00 2001 From: Raphael Date: Tue, 19 Oct 2010 23:54:12 -0700 Subject: [PATCH 8/8] Remove remove_all_aspects without ill effect --- app/models/user.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index aee21b543..cc9531c8c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -64,7 +64,7 @@ class User #after_create :seed_aspects - before_destroy :unfriend_everyone, :remove_person, :remove_all_aspects + before_destroy :unfriend_everyone, :remove_person def strip_username if username.present? @@ -436,8 +436,4 @@ class User end } end - - def remove_all_aspects - aspects.destroy_all - end end