From abf87889a0235fa3bdd7cc5a1ee3c6b1471f07a5 Mon Sep 17 00:00:00 2001 From: zaziemo Date: Mon, 13 Jul 2015 11:35:15 +0200 Subject: [PATCH 01/17] refactor test by using let instead of before block when creating instance variables --- spec/models/aspect_membership_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/models/aspect_membership_spec.rb b/spec/models/aspect_membership_spec.rb index 12b01fe24..1f9694670 100644 --- a/spec/models/aspect_membership_spec.rb +++ b/spec/models/aspect_membership_spec.rb @@ -7,25 +7,25 @@ require 'spec_helper' describe AspectMembership, :type => :model do describe '#before_destroy' do - before do - @aspect = alice.aspects.create(:name => "two") - @contact = alice.contact_for(bob.person) + let(:aspect) { alice.aspects.create(:name => "two") } + let(:contact) { alice.contact_for(bob.person) } + let(:am) { alice.aspects.where(:name => "generic").first.aspect_memberships.first } - @am = alice.aspects.where(:name => "generic").first.aspect_memberships.first - allow(@am).to receive(:user).and_return(alice) + before do + allow(am).to receive(:user).and_return(alice) end it 'calls disconnect if its the last aspect for the contact' do - expect(alice).to receive(:disconnect).with(@contact) + expect(alice).to receive(:disconnect).with(contact) - @am.destroy + am.destroy end it 'does not call disconnect if its not the last aspect for the contact' do expect(alice).not_to receive(:disconnect) - alice.add_contact_to_aspect(@contact, @aspect) - @am.destroy + alice.add_contact_to_aspect(contact, aspect) + am.destroy end end From 7f3737b13fff3c30fc6e049fbb5835ad458f6eb4 Mon Sep 17 00:00:00 2001 From: realtin Date: Mon, 13 Jul 2015 12:35:21 +0200 Subject: [PATCH 02/17] refactor test to use let rather then instance variables --- spec/models/account_deletion_spec.rb | 52 +++++++++++----------------- 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb index 576d58c57..a4a4c0d56 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deletion_spec.rb @@ -5,30 +5,28 @@ require 'spec_helper' describe AccountDeletion, :type => :model do + let(:ad_new) { AccountDeletion.new(:person => alice.person) } + let(:ad_create) { AccountDeletion.create(:person => alice.person) } + it 'assigns the diaspora_handle from the person object' do - a = AccountDeletion.new(:person => alice.person) - expect(a.diaspora_handle).to eq(alice.person.diaspora_handle) + expect(ad_new.diaspora_handle).to eq(alice.person.diaspora_handle) end it 'fires a job after creation'do expect(Workers::DeleteAccount).to receive(:perform_async).with(anything) - - AccountDeletion.create(:person => alice.person) + ad_create end describe "#perform!" do - before do - @ad = AccountDeletion.new(:person => alice.person) - end it 'creates a deleter' do expect(AccountDeleter).to receive(:new).with(alice.person.diaspora_handle).and_return(double(:perform! => true)) - @ad.perform! + ad_new.perform! end - + it 'dispatches the account deletion if the user exists' do - expect(@ad).to receive(:dispatch) - @ad.perform! + expect(ad_new).to receive(:dispatch) + ad_new.perform! end it 'does not dispatch an account deletion for non-local people' do @@ -38,55 +36,47 @@ describe AccountDeletion, :type => :model do end it 'marks an AccountDeletion as completed when successful' do - ad = AccountDeletion.create(:person => alice.person) - ad.perform! - expect(ad.reload.completed_at).not_to be_nil + ad_create.perform! + expect(ad_create.reload.completed_at).not_to be_nil end end describe '#dispatch' do it "sends the account deletion xml" do - @ad = AccountDeletion.new(:person => alice.person) - @ad.send(:dispatch) + ad_new.send(:dispatch) end it 'creates a public postzord' do expect(Postzord::Dispatcher::Public).to receive(:new).and_return(double.as_null_object) - @ad = AccountDeletion.new(:person => alice.person) - @ad.send(:dispatch) + ad_new.send(:dispatch) end end describe "#subscribers" do it 'includes all remote contacts' do - @ad = AccountDeletion.new(:person => alice.person) alice.share_with(remote_raphael, alice.aspects.first) - expect(@ad.subscribers(alice)).to eq([remote_raphael]) + expect(ad_new.subscribers(alice)).to eq([remote_raphael]) end it 'includes remote resharers' do - @ad = AccountDeletion.new(:person => alice.person) sm = FactoryGirl.create( :status_message, :public => true, :author => alice.person) - r1 = FactoryGirl.create( :reshare, :author => remote_raphael, :root => sm) - r2 = FactoryGirl.create( :reshare, :author => local_luke.person, :root => sm) + FactoryGirl.create( :reshare, :author => remote_raphael, :root => sm) + FactoryGirl.create( :reshare, :author => local_luke.person, :root => sm) - expect(@ad.subscribers(alice)).to eq([remote_raphael]) + expect(ad_new.subscribers(alice)).to eq([remote_raphael]) end end describe 'serialization' do - before do - account_deletion = AccountDeletion.new(:person => alice.person) - @xml = account_deletion.to_xml.to_s - end + let(:xml) { ad_new.to_xml.to_s } it 'should have a diaspora_handle' do - expect(@xml.include?(alice.person.diaspora_handle)).to eq(true) + expect(xml.include?(alice.person.diaspora_handle)).to eq(true) end - + it 'marshals the xml' do - expect(AccountDeletion.from_xml(@xml)).to be_valid + expect(AccountDeletion.from_xml(xml)).to be_valid end end end From d86a768590d928387d7af8e26d8969a428ddb3ee Mon Sep 17 00:00:00 2001 From: zaziemo Date: Mon, 13 Jul 2015 12:56:28 +0200 Subject: [PATCH 03/17] refactor test using let and subject instead of before --- spec/models/acts_as_taggable_on_tag_spec.rb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/spec/models/acts_as_taggable_on_tag_spec.rb b/spec/models/acts_as_taggable_on_tag_spec.rb index 159c7c035..259027d94 100644 --- a/spec/models/acts_as_taggable_on_tag_spec.rb +++ b/spec/models/acts_as_taggable_on_tag_spec.rb @@ -1,23 +1,25 @@ require 'spec_helper' describe ActsAsTaggableOn::Tag, :type => :model do + + subject(:tag) { ActsAsTaggableOn::Tag } + describe '.autocomplete' do - before do - @tag = ActsAsTaggableOn::Tag.create(:name => "cats") - end + let!(:tag_cats) { tag.create(:name => "cats") } + it 'downcases the tag name' do - expect(ActsAsTaggableOn::Tag.autocomplete("CATS")).to eq([@tag]) + expect(tag.autocomplete("CATS")).to eq([tag_cats]) end it 'does an end where on tags' do - expect(ActsAsTaggableOn::Tag.autocomplete("CAT")).to eq([@tag]) + expect(tag.autocomplete("CAT")).to eq([tag_cats]) end end describe ".normalize" do it "removes leading hash symbols" do - expect(ActsAsTaggableOn::Tag.normalize("#mytag")).to eq("mytag") + expect(tag.normalize("#mytag")).to eq("mytag") end it "removes punctuation and whitespace" do @@ -33,13 +35,13 @@ describe ActsAsTaggableOn::Tag, :type => :model do 'hash#inside' => 'hashinside', 'f!u@n#k$y%-^h&a*r(a)c{t}e[r]s' => 'funky-characters' }.each do |invalid, normalized| - expect(ActsAsTaggableOn::Tag.normalize(invalid)).to eq(normalized) + expect(tag.normalize(invalid)).to eq(normalized) end end it 'allows for love' do - expect(ActsAsTaggableOn::Tag.normalize("<3")).to eq("<3") - expect(ActsAsTaggableOn::Tag.normalize("#<3")).to eq("<3") + expect(tag.normalize("<3")).to eq("<3") + expect(tag.normalize("#<3")).to eq("<3") end end end From a9919fabd18875117aac7b6de1a8ca55d31e2ddf Mon Sep 17 00:00:00 2001 From: realtin Date: Mon, 13 Jul 2015 14:23:32 +0200 Subject: [PATCH 04/17] refactor test to use let and rm unused variables --- spec/models/aspect_spec.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb index 4d85a2b1f..301a7d3a4 100644 --- a/spec/models/aspect_spec.rb +++ b/spec/models/aspect_spec.rb @@ -6,20 +6,14 @@ require 'spec_helper' describe Aspect, :type => :model do describe 'creation' do - before do - @name = alice.aspects.first.name - end + let(:name) { alice.aspects.first.name } it 'does not allow duplicate names' do - expect { - invalid_aspect = alice.aspects.create(:name => @name) - }.not_to change(Aspect, :count) + expect { alice.aspects.create(:name => name) }.not_to change(Aspect, :count) end it 'validates case insensitiveness on names' do - expect { - invalid_aspect = alice.aspects.create(:name => @name.titleize) - }.not_to change(Aspect, :count) + expect { alice.aspects.create(:name => name.titleize) }.not_to change(Aspect, :count) end it 'has a 20 character limit on names' do From fccb5dae23b799b8e881814c49c2dd5fde69c452 Mon Sep 17 00:00:00 2001 From: zaziemo Date: Mon, 13 Jul 2015 16:52:46 +0200 Subject: [PATCH 05/17] refactor test: replacing instance variables in before blocks, renaming variables for easier understanding, simplify test set up with less different comments, correct two tests --- spec/models/comment_spec.rb | 49 +++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 2e2a91205..9a843c4d5 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -6,58 +6,57 @@ require 'spec_helper' require Rails.root.join("spec", "shared_behaviors", "relayable") describe Comment, :type => :model do - before do - @alices_aspect = alice.aspects.first - @status = bob.post(:status_message, :text => "hello", :to => bob.aspects.first.id) - end + + let(:alices_aspect) { alice.aspects.first } + let(:status_bob) { bob.post(:status_message, :text => "hello", :to => bob.aspects.first.id) } + let(:comment_alice) { alice.comment!(status_bob, "why so formal?") } describe 'comment#notification_type' do - let (:comment) { alice.comment!(@status, "why so formal?") } it "returns 'comment_on_post' if the comment is on a post you own" do - expect(comment.notification_type(bob, alice.person)).to eq(Notifications::CommentOnPost) + expect(comment_alice.notification_type(bob, alice.person)).to eq(Notifications::CommentOnPost) end it "returns 'also_commented' if the comment is on a post you participate to" do - eve.participate! @status - expect(comment.notification_type(eve, alice.person)).to eq(Notifications::AlsoCommented) + eve.participate! status_bob + expect(comment_alice.notification_type(eve, alice.person)).to eq(Notifications::AlsoCommented) end it 'returns false if the comment is not on a post you own and no one "also_commented"' do - comment = alice.comment!(@status, "I simply felt like issuing a greeting. Do step off.") - expect(comment.notification_type(eve, alice.person)).to be false + expect(comment_alice.notification_type(eve, alice.person)).to be false end context "also commented" do + let(:comment_eve) { eve.comment!(status_bob, "I also commented on the first user's post") } + before do - alice.comment!(@status, "a-commenta commenta") - @comment = eve.comment!(@status, "I also commented on the first user's post") + comment_alice end it 'does not return also commented if the user commented' do - expect(@comment.notification_type(eve, alice.person)).to eq(false) + expect(comment_eve.notification_type(eve, alice.person)).to eq(false) end it "returns 'also_commented' if another person commented on a post you commented on" do - expect(@comment.notification_type(alice, alice.person)).to eq(Notifications::AlsoCommented) + expect(comment_eve.notification_type(alice, alice.person)).to eq(Notifications::AlsoCommented) end end end describe 'User#comment' do it "should be able to comment on one's own status" do - alice.comment!(@status, "Yeah, it was great") - expect(@status.reload.comments.first.text).to eq("Yeah, it was great") + bob.comment!(status_bob, "sup dog") + expect(status_bob.reload.comments.first.text).to eq("sup dog") end it "should be able to comment on a contact's status" do - bob.comment!(@status, "sup dog") - expect(@status.reload.comments.first.text).to eq("sup dog") + comment_alice + expect(status_bob.reload.comments.first.text).to eq("why so formal?") end it 'does not multi-post a comment' do expect { - alice.comment!(@status, 'hello') + comment_alice }.to change { Comment.count }.by(1) end end @@ -65,19 +64,21 @@ describe Comment, :type => :model do describe 'counter cache' do it 'increments the counter cache on its post' do expect { - alice.comment!(@status, "oh yeah") + comment_alice }.to change{ - @status.reload.comments_count + status_bob.reload.comments_count }.by(1) end end +# hier weiter + describe 'xml' do before do @commenter = FactoryGirl.create(:user) @commenter_aspect = @commenter.aspects.create(:name => "bruisers") - connect_users(alice, @alices_aspect, @commenter, @commenter_aspect) - @post = alice.post :status_message, :text => "hello", :to => @alices_aspect.id + connect_users(alice, alices_aspect, @commenter, @commenter_aspect) + @post = alice.post :status_message, :text => "hello", :to => alices_aspect.id @comment = @commenter.comment!(@post, "Fool!") @xml = @comment.to_xml.to_s end @@ -125,7 +126,7 @@ describe Comment, :type => :model do @object_on_remote_parent = @local_luke.comment!(@remote_parent, "Yeah, it was great") end - let(:build_object) { alice.build_comment(:post => @status, :text => "why so formal?") } + let(:build_object) { alice.build_comment(:post => status_bob, :text => "why so formal?") } it_should_behave_like 'it is relayable' end From e3fe37584967341a81f766d5403aa3c5f5cff5a3 Mon Sep 17 00:00:00 2001 From: zaziemo Date: Mon, 13 Jul 2015 17:41:48 +0200 Subject: [PATCH 06/17] correct indentation and use new hash syntax --- spec/models/account_deletion_spec.rb | 15 +++++++-------- spec/models/acts_as_taggable_on_tag_spec.rb | 3 +-- spec/models/aspect_membership_spec.rb | 4 ++-- spec/models/aspect_spec.rb | 18 +++++++++--------- spec/models/block_spec.rb | 4 ++-- spec/models/comment_spec.rb | 4 +--- 6 files changed, 22 insertions(+), 26 deletions(-) diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb index a4a4c0d56..2a3d682a8 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deletion_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' describe AccountDeletion, :type => :model do - let(:ad_new) { AccountDeletion.new(:person => alice.person) } - let(:ad_create) { AccountDeletion.create(:person => alice.person) } + let(:ad_new) { AccountDeletion.new(person: alice.person) } + let(:ad_create) { AccountDeletion.create(person: alice.person) } it 'assigns the diaspora_handle from the person object' do expect(ad_new.diaspora_handle).to eq(alice.person.diaspora_handle) @@ -18,9 +18,8 @@ describe AccountDeletion, :type => :model do end describe "#perform!" do - it 'creates a deleter' do - expect(AccountDeleter).to receive(:new).with(alice.person.diaspora_handle).and_return(double(:perform! => true)) + expect(AccountDeleter).to receive(:new).with(alice.person.diaspora_handle).and_return(double(perform!: true)) ad_new.perform! end @@ -30,7 +29,7 @@ describe AccountDeletion, :type => :model do end it 'does not dispatch an account deletion for non-local people' do - deletion = AccountDeletion.new(:person => remote_raphael) + deletion = AccountDeletion.new(person: remote_raphael) expect(deletion).not_to receive(:dispatch) deletion.perform! end @@ -60,9 +59,9 @@ describe AccountDeletion, :type => :model do end it 'includes remote resharers' do - sm = FactoryGirl.create( :status_message, :public => true, :author => alice.person) - FactoryGirl.create( :reshare, :author => remote_raphael, :root => sm) - FactoryGirl.create( :reshare, :author => local_luke.person, :root => sm) + sm = FactoryGirl.create( :status_message, public: true, author: alice.person) + FactoryGirl.create( :reshare, author: remote_raphael, root: sm) + FactoryGirl.create( :reshare, author: local_luke.person, root: sm) expect(ad_new.subscribers(alice)).to eq([remote_raphael]) end diff --git a/spec/models/acts_as_taggable_on_tag_spec.rb b/spec/models/acts_as_taggable_on_tag_spec.rb index 259027d94..0d6d47981 100644 --- a/spec/models/acts_as_taggable_on_tag_spec.rb +++ b/spec/models/acts_as_taggable_on_tag_spec.rb @@ -1,11 +1,10 @@ require 'spec_helper' describe ActsAsTaggableOn::Tag, :type => :model do - subject(:tag) { ActsAsTaggableOn::Tag } describe '.autocomplete' do - let!(:tag_cats) { tag.create(:name => "cats") } + let!(:tag_cats) { tag.create(name: "cats") } it 'downcases the tag name' do expect(tag.autocomplete("CATS")).to eq([tag_cats]) diff --git a/spec/models/aspect_membership_spec.rb b/spec/models/aspect_membership_spec.rb index 1f9694670..97c5f7c77 100644 --- a/spec/models/aspect_membership_spec.rb +++ b/spec/models/aspect_membership_spec.rb @@ -7,9 +7,9 @@ require 'spec_helper' describe AspectMembership, :type => :model do describe '#before_destroy' do - let(:aspect) { alice.aspects.create(:name => "two") } + let(:aspect) { alice.aspects.create(name: "two") } let(:contact) { alice.contact_for(bob.person) } - let(:am) { alice.aspects.where(:name => "generic").first.aspect_memberships.first } + let(:am) { alice.aspects.where(name: "generic").first.aspect_memberships.first } before do allow(am).to receive(:user).and_return(alice) diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb index 301a7d3a4..7e247d405 100644 --- a/spec/models/aspect_spec.rb +++ b/spec/models/aspect_spec.rb @@ -9,24 +9,24 @@ describe Aspect, :type => :model do let(:name) { alice.aspects.first.name } it 'does not allow duplicate names' do - expect { alice.aspects.create(:name => name) }.not_to change(Aspect, :count) + expect { alice.aspects.create(name: name) }.not_to change(Aspect, :count) end it 'validates case insensitiveness on names' do - expect { alice.aspects.create(:name => name.titleize) }.not_to change(Aspect, :count) + expect { alice.aspects.create(name: name.titleize) }.not_to change(Aspect, :count) end it 'has a 20 character limit on names' do - aspect = Aspect.new(:name => "this name is really too too too too too long") + aspect = Aspect.new(name: "this name is really too too too too too long") expect(aspect.valid?).to eq(false) end it 'is able to have other users as contacts' do - aspect = alice.aspects.create(:name => 'losers') + aspect = alice.aspects.create(name: 'losers') - Contact.create(:user => alice, :person => eve.person, :aspects => [aspect]) - expect(aspect.contacts.where(:person_id => alice.person.id)).to be_empty - expect(aspect.contacts.where(:person_id => eve.person.id)).not_to be_empty + Contact.create(user: alice, person: eve.person, aspects: [aspect]) + expect(aspect.contacts.where(person_id: alice.person.id)).to be_empty + expect(aspect.contacts.where(person_id: eve.person.id)).not_to be_empty expect(aspect.contacts.size).to eq(1) end @@ -44,8 +44,8 @@ describe Aspect, :type => :model do describe 'validation' do it 'has no uniqueness of name between users' do - aspect = alice.aspects.create(:name => "New Aspect") - aspect2 = eve.aspects.create(:name => aspect.name) + aspect = alice.aspects.create(name: "New Aspect") + aspect2 = eve.aspects.create(name: aspect.name) expect(aspect2).to be_valid end end diff --git a/spec/models/block_spec.rb b/spec/models/block_spec.rb index e8b3683a6..2721ea1df 100644 --- a/spec/models/block_spec.rb +++ b/spec/models/block_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Block, :type => :model do describe 'validations' do it 'doesnt allow you to block yourself' do - block = alice.blocks.create(:person => alice.person) + block = alice.blocks.create(person: alice.person) expect(block.errors[:person_id].size).to eq(1) end end -end \ No newline at end of file +end diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 9a843c4d5..26ce4f7d9 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -6,13 +6,11 @@ require 'spec_helper' require Rails.root.join("spec", "shared_behaviors", "relayable") describe Comment, :type => :model do - let(:alices_aspect) { alice.aspects.first } - let(:status_bob) { bob.post(:status_message, :text => "hello", :to => bob.aspects.first.id) } + let(:status_bob) { bob.post(:status_message, text: "hello", to: bob.aspects.first.id) } let(:comment_alice) { alice.comment!(status_bob, "why so formal?") } describe 'comment#notification_type' do - it "returns 'comment_on_post' if the comment is on a post you own" do expect(comment_alice.notification_type(bob, alice.person)).to eq(Notifications::CommentOnPost) end From e1e8856e8d33528ff368506f4784ee0ce2389f78 Mon Sep 17 00:00:00 2001 From: zaziemo Date: Tue, 14 Jul 2015 15:41:53 +0200 Subject: [PATCH 07/17] refactor more specs --- spec/models/comment_spec.rb | 64 +++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 26ce4f7d9..afeab06e4 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -69,59 +69,65 @@ describe Comment, :type => :model do end end -# hier weiter - describe 'xml' do + let(:commenter) { create(:user) } + let(:commenter_aspect) { commenter.aspects.create(name: "bruisers") } + let(:post) { alice.post :status_message, text: "hello", to: alices_aspect.id } + let(:comment) { commenter.comment!(post, "Fool!") } + let(:xml) { comment.to_xml.to_s } + before do - @commenter = FactoryGirl.create(:user) - @commenter_aspect = @commenter.aspects.create(:name => "bruisers") - connect_users(alice, alices_aspect, @commenter, @commenter_aspect) - @post = alice.post :status_message, :text => "hello", :to => alices_aspect.id - @comment = @commenter.comment!(@post, "Fool!") - @xml = @comment.to_xml.to_s + connect_users(alice, alices_aspect, commenter, commenter_aspect) end it 'serializes the sender handle' do - expect(@xml.include?(@commenter.diaspora_handle)).to be true + expect(xml.include?(commenter.diaspora_handle)).to be true end it 'serializes the post_guid' do - expect(@xml).to include(@post.guid) + expect(xml).to include(post.guid) end describe 'marshalling' do - before do - @marshalled_comment = Comment.from_xml(@xml) - end + let(:marshalled_comment) { Comment.from_xml(xml) } it 'marshals the author' do - expect(@marshalled_comment.author).to eq(@commenter.person) + expect(marshalled_comment.author).to eq(commenter.person) end it 'marshals the post' do - expect(@marshalled_comment.post).to eq(@post) + expect(marshalled_comment.post).to eq(post) end it 'tries to fetch a missing parent' do - guid = @post.guid - @post.destroy + guid = post.guid + marshalled_comment + post.destroy expect_any_instance_of(Comment).to receive(:fetch_parent).with(guid).and_return(nil) - Comment.from_xml(@xml) + Comment.from_xml(xml) end end end + #Andy fragen - shared behaviour describe 'it is relayable' do + let(:remote_parent) { build(:status_message, author: @remote_raphael) } + let(:local_parent) { @local_luke.post :status_message, text: "hi", to: @local_luke.aspects.first } + let(:object_by_parent_author) { @local_luke.comment!(text: "yo!", post: local_parent) } + let(:object_by_recipient) { @local_leia.build_comment(text: "yo", post: local_parent) } + let(:dup_object_by_parent_author) { object_by_parent_author.dup } + let(:object_on_remote_parent) { @local_luke.comment!(remote_parent, "Yeah, it was great") } + before do - @local_luke, @local_leia, @remote_raphael = set_up_friends - @remote_parent = FactoryGirl.build(:status_message, :author => @remote_raphael) - @local_parent = @local_luke.post :status_message, :text => "hi", :to => @local_luke.aspects.first + local_luke, @local_leia, @remote_raphael = set_up_friends + # @remote_parent = FactoryGirl.build(:status_message, :author => @remote_raphael) + # @local_parent = @local_luke.post :status_message, :text => "hi", :to => @local_luke.aspects.first - @object_by_parent_author = @local_luke.comment!(@local_parent, "yo") - @object_by_recipient = @local_leia.build_comment(:text => "yo", :post => @local_parent) - @dup_object_by_parent_author = @object_by_parent_author.dup + # @object_by_parent_author = @local_luke.comment!(@local_parent, "yo") + # @object_by_recipient = @local_leia.build_comment(:text => "yo", :post => @local_parent) + # @dup_object_by_parent_author = @object_by_parent_author.dup - @object_on_remote_parent = @local_luke.comment!(@remote_parent, "Yeah, it was great") + # @object_on_remote_parent = @local_luke.comment!(@remote_parent, "Yeah, it was great") end let(:build_object) { alice.build_comment(:post => status_bob, :text => "why so formal?") } @@ -129,9 +135,11 @@ describe Comment, :type => :model do end describe 'tags' do - before do - @object = FactoryGirl.build(:comment) - end + let(:object) { build(:comment) } + + # before do + # @object = FactoryGirl.build(:comment) + # end it_should_behave_like 'it is taggable' end From 3ff33d355f8565deb78170df885dc046ca22553f Mon Sep 17 00:00:00 2001 From: realtin Date: Tue, 14 Jul 2015 16:07:13 +0200 Subject: [PATCH 08/17] refactor test to use let --- spec/models/contact_spec.rb | 81 ++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 42 deletions(-) diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 9f8370952..6bed1b92a 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -14,7 +14,7 @@ describe Contact, :type => :model do end context 'validations' do - let(:contact){Contact.new} + let(:contact) { Contact.new } it 'requires a user' do contact.valid? @@ -37,7 +37,7 @@ describe Contact, :type => :model do it 'validates uniqueness' do person = FactoryGirl.create(:person) - contact2 = alice.contacts.create(:person=>person) + contact2 = alice.contacts.create(person: person) expect(contact2).to be_valid contact.user = alice @@ -48,7 +48,7 @@ describe Contact, :type => :model do it "validates that the person's account is not closed" do person = FactoryGirl.create(:person, :closed_account => true) - contact = alice.contacts.new(:person=>person) + contact = alice.contacts.new(person: person) expect(contact).not_to be_valid expect(contact.errors.full_messages).to include "Cannot be in contact with a closed account" @@ -59,8 +59,8 @@ describe Contact, :type => :model do describe 'sharing' do it 'returns contacts with sharing true' do expect { - alice.contacts.create!(:sharing => true, :person => FactoryGirl.create(:person)) - alice.contacts.create!(:sharing => false, :person => FactoryGirl.create(:person)) + alice.contacts.create!(sharing: true, person: FactoryGirl.create(:person)) + alice.contacts.create!(sharing: false, person: FactoryGirl.create(:person)) }.to change{ Contact.sharing.count }.by(1) @@ -70,8 +70,8 @@ describe Contact, :type => :model do describe 'receiving' do it 'returns contacts with sharing true' do expect { - alice.contacts.create!(:receiving => true, :person => FactoryGirl.build(:person)) - alice.contacts.create!(:receiving => false, :person => FactoryGirl.build(:person)) + alice.contacts.create!(receiving: true, person: FactoryGirl.build(:person)) + alice.contacts.create!(receiving: false, person: FactoryGirl.build(:person)) }.to change{ Contact.receiving.count }.by(1) @@ -81,20 +81,20 @@ describe Contact, :type => :model do describe 'only_sharing' do it 'returns contacts with sharing true and receiving false' do expect { - alice.contacts.create!(:receiving => true, :sharing => true, :person => FactoryGirl.build(:person)) - alice.contacts.create!(:receiving => false, :sharing => true, :person => FactoryGirl.build(:person)) - alice.contacts.create!(:receiving => false, :sharing => true, :person => FactoryGirl.build(:person)) - alice.contacts.create!(:receiving => true, :sharing => false, :person => FactoryGirl.build(:person)) + alice.contacts.create!(receiving: true, sharing: true, person: FactoryGirl.build(:person)) + alice.contacts.create!(receiving: false, sharing: true, person: FactoryGirl.build(:person)) + alice.contacts.create!(receiving: false, sharing: true, person: FactoryGirl.build(:person)) + alice.contacts.create!(receiving: true, sharing: false, person: FactoryGirl.build(:person)) }.to change{ Contact.receiving.count }.by(2) end end - + describe "all_contacts_of_person" do it 'returns all contacts where the person is the passed in person' do person = FactoryGirl.create(:person) - contact1 = FactoryGirl.create(:contact, :person => person) + contact1 = FactoryGirl.create(:contact, person: person) contact2 = FactoryGirl.create(:contact) contacts = Contact.all_contacts_of_person(person) expect(contacts).to eq([contact1]) @@ -107,23 +107,23 @@ describe Contact, :type => :model do @alice = alice @bob = bob @eve = eve - @bob.aspects.create(:name => 'next') + @bob.aspects.create(name: 'next') @bob.aspects(true) - @original_aspect = @bob.aspects.where(:name => "generic").first - @new_aspect = @bob.aspects.where(:name => "next").first + @original_aspect = @bob.aspects.where(name: "generic").first + @new_aspect = @bob.aspects.where(name: "next").first @people1 = [] @people2 = [] 1.upto(5) do person = FactoryGirl.build(:person) - @bob.contacts.create(:person => person, :aspects => [@original_aspect]) + @bob.contacts.create(person: person, aspects: [@original_aspect]) @people1 << person end 1.upto(5) do person = FactoryGirl.build(:person) - @bob.contacts.create(:person => person, :aspects => [@new_aspect]) + @bob.contacts.create(person: person, aspects: [@new_aspect]) @people2 << person end #eve <-> bob <-> alice @@ -154,66 +154,63 @@ describe Contact, :type => :model do end context 'on a contact for a remote user' do - before do - @contact = @bob.contact_for @people1.first - end + let(:contact) { @bob.contact_for @people1.first } + it 'returns an empty array' do - expect(@contact.contacts).to eq([]) + expect(contact.contacts).to eq([]) end end end context 'requesting' do - before do - @contact = Contact.new - @user = FactoryGirl.build(:user) - @person = FactoryGirl.build(:person) + let(:contact) { Contact.new } + let(:user) { build(:user) } + let(:person) { build(:person) } - @contact.user = @user - @contact.person = @person + before do + contact.user = user + contact.person = person end describe '#generate_request' do it 'makes a request' do - allow(@contact).to receive(:user).and_return(@user) - request = @contact.generate_request + allow(contact).to receive(:user).and_return(user) + request = contact.generate_request - expect(request.sender).to eq(@user.person) - expect(request.recipient).to eq(@person) + expect(request.sender).to eq(user.person) + expect(request.recipient).to eq(person) end end describe '#dispatch_request' do it 'pushes to people' do - allow(@contact).to receive(:user).and_return(@user) + allow(contact).to receive(:user).and_return(user) m = double() expect(m).to receive(:post) expect(Postzord::Dispatcher).to receive(:build).and_return(m) - @contact.dispatch_request + contact.dispatch_request end end end describe "#not_blocked_user" do - before do - @contact = alice.contact_for(bob.person) - end + let(:contact) { alice.contact_for(bob.person) } it "is called on validate" do - expect(@contact).to receive(:not_blocked_user) - @contact.valid? + expect(contact).to receive(:not_blocked_user) + contact.valid? end it "adds to errors if potential contact is blocked by user" do person = eve.person - block = alice.blocks.create(:person => person) - bad_contact = alice.contacts.create(:person => person) + block = alice.blocks.create(person: person) + bad_contact = alice.contacts.create(person: person) expect(bad_contact.send(:not_blocked_user)).to be false end it "does not add to errors" do - expect(@contact.send(:not_blocked_user)).to be true + expect(contact.send(:not_blocked_user)).to be true end end end From 5fba53a1056b2589e6f49a18d4ef1f142d0b24d9 Mon Sep 17 00:00:00 2001 From: zaziemo Date: Tue, 14 Jul 2015 16:57:01 +0200 Subject: [PATCH 09/17] refactor even more tests --- spec/models/conversation_spec.rb | 125 +++++++++++++------------------ 1 file changed, 51 insertions(+), 74 deletions(-) diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 690df1bba..35b79f412 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -5,101 +5,91 @@ require 'spec_helper' describe Conversation, :type => :model do - before do - @user1 = alice - @user2 = bob - @participant_ids = [@user1.contacts.first.person.id, @user1.person.id] - - @create_hash = { - :author => @user1.person, - :participant_ids => @participant_ids, - :subject => "cool stuff", - :messages_attributes => [ {:author => @user1.person, :text => 'hey'} ] - } - end + let(:user1) { alice } + let(:user2) { bob } + let(:participant_ids) { [user1.contacts.first.person.id, user1.person.id] } + let(:create_hash) { {author: user1.person, participant_ids: participant_ids, subject: "cool stuff", + messages_attributes: [ {author: user1.person, text: "hey"} ]} } + let(:conversation) { Conversation.create(create_hash) } + let(:message_last) { Message.create(author: user2.person, created_at: Time.now + 100, text: "last", conversation_id: conversation.id) } + let(:message_first) { Message.create(author: user2.person, created_at: Time.now + 100, text: "first", conversation_id: conversation.id) } it 'creates a message on create' do - expect{ - Conversation.create(@create_hash) - }.to change(Message, :count).by(1) + expect{ conversation }.to change(Message, :count).by(1) end describe '#last_author' do it 'returns the last author to a conversation' do - cnv = Conversation.create(@create_hash) - Message.create(:author => @user2.person, :created_at => Time.now + 100, :text => "last", :conversation_id => cnv.id) - expect(cnv.reload.last_author.id).to eq(@user2.person.id) + message_last + expect(conversation.reload.last_author.id).to eq(user2.person.id) end end describe "#ordered_participants" do it "returns the ordered participants" do - cnv = Conversation.create(@create_hash) - Message.create(author: @user2.person, created_at: Time.now + 100, text: "last", conversation_id: cnv.id) - expect(cnv.ordered_participants.first).to eq(@user2.person) - expect(cnv.ordered_participants.last).to eq(@user1.person) + message_last + expect(conversation.ordered_participants.first).to eq(user2.person) + expect(conversation.ordered_participants.last).to eq(user1.person) end end describe '#first_unread_message' do before do - @cnv = Conversation.create(@create_hash) - @message = Message.create(:author => @user2.person, :created_at => Time.now + 100, :text => "last", :conversation_id => @cnv.id) - @message.increase_unread(@user1) + message_last.increase_unread(user1) end it 'returns the first unread message if there are unread messages in a conversation' do - @cnv.first_unread_message(@user1) == @message + conversation.first_unread_message(user1) == message_last end it 'returns nil if there are no unread messages in a conversation' do - @cnv.conversation_visibilities.where(:person_id => @user1.person.id).first.tap { |cv| cv.unread = 0 }.save - expect(@cnv.first_unread_message(@user1)).to be_nil + conversation.conversation_visibilities.where(person_id: user1.person.id).first.tap { |cv| cv.unread = 0 }.save + expect(conversation.first_unread_message(user1)).to be_nil end end describe '#set_read' do + before do - @cnv = Conversation.create(@create_hash) - Message.create(:author => @user2.person, :created_at => Time.now + 100, :text => "first", :conversation_id => @cnv.id) - .increase_unread(@user1) - Message.create(:author => @user2.person, :created_at => Time.now + 200, :text => "last", :conversation_id => @cnv.id) - .increase_unread(@user1) + conversation + message_first.increase_unread(user1) + message_last.increase_unread(user1) end it 'sets the unread counter to 0' do - expect(@cnv.conversation_visibilities.where(:person_id => @user1.person.id).first.unread).to eq(2) - @cnv.set_read(@user1) - expect(@cnv.conversation_visibilities.where(:person_id => @user1.person.id).first.unread).to eq(0) + expect(conversation.conversation_visibilities.where(person_id: user1.person.id).first.unread).to eq(2) + conversation.set_read(user1) + expect(conversation.conversation_visibilities.where(person_id: user1.person.id).first.unread).to eq(0) end end context 'transport' do + let(:conversation_message) { conversation.messages.first } + let(:xml) { conversation.to_diaspora_xml } + before do - @cnv = Conversation.create(@create_hash) - @message = @cnv.messages.first - @xml = @cnv.to_diaspora_xml + conversation end describe 'serialization' do it 'serializes the message' do - expect(@xml.gsub(/\s/, '')).to include(@message.to_xml.to_s.gsub(/\s/, '')) + expect(xml.gsub(/\s/, '')).to include(conversation_message.to_xml.to_s.gsub(/\s/, '')) end it 'serializes the participants' do - @create_hash[:participant_ids].each{|id| - expect(@xml).to include(Person.find(id).diaspora_handle) + create_hash[:participant_ids].each{|id| + expect(xml).to include(Person.find(id).diaspora_handle) } end it 'serializes the created_at time' do - expect(@xml).to include(@message.created_at.to_s) + expect(xml).to include(conversation_message.created_at.to_s) end end describe '#subscribers' do it 'returns the recipients for the post owner' do - expect(@cnv.subscribers(@user1)).to eq(@user1.contacts.map{|c| c.person}) + expect(conversation.subscribers(user1)).to eq(user1.contacts.map{|c| c.person}) end end @@ -111,63 +101,50 @@ describe Conversation, :type => :model do it 'creates a message' do expect{ - Diaspora::Parser.from_xml(@xml).receive(@user1, @user2.person) + Diaspora::Parser.from_xml(xml).receive(user1, user2.person) }.to change(Message, :count).by(1) end it 'creates a conversation' do expect{ - Diaspora::Parser.from_xml(@xml).receive(@user1, @user2.person) + Diaspora::Parser.from_xml(xml).receive(user1, user2.person) }.to change(Conversation, :count).by(1) end it 'creates appropriate visibilities' do expect{ - Diaspora::Parser.from_xml(@xml).receive(@user1, @user2.person) - }.to change(ConversationVisibility, :count).by(@participant_ids.size) + Diaspora::Parser.from_xml(xml).receive(user1, user2.person) + }.to change(ConversationVisibility, :count).by(participant_ids.size) end it 'does not save before receive' do - expect(Diaspora::Parser.from_xml(@xml).persisted?).to be false + expect(Diaspora::Parser.from_xml(xml).persisted?).to be false end it 'notifies for the message' do expect(Notification).to receive(:notify).once - Diaspora::Parser.from_xml(@xml).receive(@user1, @user2.person) + Diaspora::Parser.from_xml(xml).receive(user1, user2.person) end end end describe "#invalid parameters" do context "local author" do - before do - @invalid_hash = { - author: peter.person, - participant_ids: [peter.person.id, @user1.person.id], - subject: "cool stuff", - messages_attributes: [{author: peter.person, text: "hey"}] - } - end + let(:invalid_hash) { {author: peter.person, participant_ids: [peter.person.id, user1.person.id], + subject: "cool stuff", messages_attributes: [{author: peter.person, text: "hey"}]} } it "is invalid with invalid recipient" do - conversation = Conversation.create(@invalid_hash) - expect(conversation).to be_invalid + invalid_conversation = Conversation.create(invalid_hash) + expect(invalid_conversation).to be_invalid end end context "remote author" do - before do - @remote_person = remote_raphael - @local_user = alice - @participant_ids = [@remote_person.id, @local_user.person.id] - - @invalid_hash_remote = { - author: @remote_person, - participant_ids: @participant_ids, - subject: "cool stuff", - messages_attributes: [{author: @remote_person, text: "hey"}] - } - end + let(:remote_person) { remote_raphael } + let(:local_user) { alice } + let(:participant_ids) { [remote_person.id, local_user.person.id] } + let(:invalid_hash_remote) { {author: remote_person, participant_ids: participant_ids, + subject: "cool stuff", messages_attributes: [{author: remote_person, text: "hey"}]} } it "is invalid with invalid recipient" do - conversation = Conversation.create(@invalid_hash_remote) - expect(conversation).to be_invalid + invalid_conversation_remote = Conversation.create(invalid_hash_remote) + expect(invalid_conversation_remote).to be_invalid end end end From 880886bbadde1da465a8c8298f939ed7d9ab0a63 Mon Sep 17 00:00:00 2001 From: realtin Date: Tue, 14 Jul 2015 17:08:54 +0200 Subject: [PATCH 10/17] refactor test to use let instead of instance variables --- spec/models/conversation_visibilities_spec.rb | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/spec/models/conversation_visibilities_spec.rb b/spec/models/conversation_visibilities_spec.rb index da15b8743..a8a0e2708 100644 --- a/spec/models/conversation_visibilities_spec.rb +++ b/spec/models/conversation_visibilities_spec.rb @@ -4,25 +4,22 @@ require 'spec_helper' -describe ConversationVisibility, :type => :model do - before do - @user1 = alice - @participant_ids = [@user1.contacts.first.person.id, @user1.person.id] - - @create_hash = { - :author => @user1.person, - :participant_ids => @participant_ids, - :subject => "cool stuff", - :messages_attributes => [ {:author => @user1.person, :text => 'hey'} ] +describe ConversationVisibility, type: :model do + let(:user1) { alice } + let(:participant_ids) { [user1.contacts.first.person.id, user1.person.id] } + let(:create_hash) do + { + author: user1.person, + participant_ids: participant_ids, + subject: 'cool stuff', + messages_attributes: [{ author: user1.person, text: 'hey' }] } - @conversation = Conversation.create(@create_hash) end + let(:conversation) { Conversation.create(create_hash) } it 'destroy conversation when no participant' do - @conversation.conversation_visibilities.each do |visibility| - visibility.destroy - end - - expect(Conversation).not_to exist(@conversation.id) + conversation.conversation_visibilities.each(&:destroy) + + expect(Conversation).not_to exist(conversation.id) end end From 1503376fa632d5efd16652c6b0358ba5ab1e09d7 Mon Sep 17 00:00:00 2001 From: realtin Date: Tue, 14 Jul 2015 18:10:56 +0200 Subject: [PATCH 11/17] refactor test to not use send method since dispatch method is not private (#6192) --- spec/models/account_deletion_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb index 2a3d682a8..1424a731b 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deletion_spec.rb @@ -42,12 +42,12 @@ describe AccountDeletion, :type => :model do describe '#dispatch' do it "sends the account deletion xml" do - ad_new.send(:dispatch) + ad_new.dispatch end it 'creates a public postzord' do expect(Postzord::Dispatcher::Public).to receive(:new).and_return(double.as_null_object) - ad_new.send(:dispatch) + ad_new.dispatch end end From 62b375bb8ea35ad96181d226c5a381ab36eb3969 Mon Sep 17 00:00:00 2001 From: realtin Date: Tue, 14 Jul 2015 18:59:57 +0200 Subject: [PATCH 12/17] refactor test to use let instead of instance variables (#6192) --- spec/models/comment_spec.rb | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index afeab06e4..8d5e12eff 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -111,23 +111,20 @@ describe Comment, :type => :model do #Andy fragen - shared behaviour describe 'it is relayable' do - let(:remote_parent) { build(:status_message, author: @remote_raphael) } - let(:local_parent) { @local_luke.post :status_message, text: "hi", to: @local_luke.aspects.first } - let(:object_by_parent_author) { @local_luke.comment!(text: "yo!", post: local_parent) } - let(:object_by_recipient) { @local_leia.build_comment(text: "yo", post: local_parent) } + let(:remote_parent) { build(:status_message, author: remote_raphael) } + let(:local_parent) { local_luke.post :status_message, text: "hi", to: local_luke.aspects.first } + let(:object_by_parent_author) { local_luke.comment!(local_parent, "yo!") } + let(:object_by_recipient) { local_leia.build_comment(text: "yo", post: local_parent) } let(:dup_object_by_parent_author) { object_by_parent_author.dup } - let(:object_on_remote_parent) { @local_luke.comment!(remote_parent, "Yeah, it was great") } + let(:object_on_remote_parent) { local_luke.comment!(remote_parent, "Yeah, it was great") } before do - local_luke, @local_leia, @remote_raphael = set_up_friends - # @remote_parent = FactoryGirl.build(:status_message, :author => @remote_raphael) - # @local_parent = @local_luke.post :status_message, :text => "hi", :to => @local_luke.aspects.first - - # @object_by_parent_author = @local_luke.comment!(@local_parent, "yo") - # @object_by_recipient = @local_leia.build_comment(:text => "yo", :post => @local_parent) - # @dup_object_by_parent_author = @object_by_parent_author.dup - - # @object_on_remote_parent = @local_luke.comment!(@remote_parent, "Yeah, it was great") + # shared_behaviors/relayable.rb is still using instance variables, so we need to define them here. + # Suggestion: refactor all specs using shared_behaviors/relayable.rb to use 'let' + @object_by_parent_author = object_by_parent_author + @object_by_recipient = object_by_recipient + @dup_object_by_parent_author = dup_object_by_parent_author + @object_on_remote_parent = object_on_remote_parent end let(:build_object) { alice.build_comment(:post => status_bob, :text => "why so formal?") } @@ -137,9 +134,11 @@ describe Comment, :type => :model do describe 'tags' do let(:object) { build(:comment) } - # before do - # @object = FactoryGirl.build(:comment) - # end + before do + # shared_behaviors/taggable.rb is still using instance variables, so we need to define them here. + # Suggestion: refactor all specs using shared_behaviors/taggable.rb to use 'let' + @object = object + end it_should_behave_like 'it is taggable' end From f52260ec687a2a7ba1f242637e576decbefe6d2a Mon Sep 17 00:00:00 2001 From: zaziemo Date: Tue, 14 Jul 2015 19:10:05 +0200 Subject: [PATCH 13/17] Delete test because it didn't have an expectation and dipatch is also called in the following test #6192 --- spec/models/account_deletion_spec.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb index 1424a731b..ad528bcc2 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deletion_spec.rb @@ -41,10 +41,6 @@ describe AccountDeletion, :type => :model do end describe '#dispatch' do - it "sends the account deletion xml" do - ad_new.dispatch - end - it 'creates a public postzord' do expect(Postzord::Dispatcher::Public).to receive(:new).and_return(double.as_null_object) ad_new.dispatch From 56e0d3d57d4b4e0e1f2d3b82de2d304b065ad221 Mon Sep 17 00:00:00 2001 From: zaziemo Date: Tue, 14 Jul 2015 19:33:40 +0200 Subject: [PATCH 14/17] Remove one comment and change hash syntax #6192 --- spec/models/comment_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 8d5e12eff..8b26ab871 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -109,7 +109,6 @@ describe Comment, :type => :model do end end - #Andy fragen - shared behaviour describe 'it is relayable' do let(:remote_parent) { build(:status_message, author: remote_raphael) } let(:local_parent) { local_luke.post :status_message, text: "hi", to: local_luke.aspects.first } @@ -127,7 +126,7 @@ describe Comment, :type => :model do @object_on_remote_parent = object_on_remote_parent end - let(:build_object) { alice.build_comment(:post => status_bob, :text => "why so formal?") } + let(:build_object) { alice.build_comment(post: status_bob, text: "why so formal?") } it_should_behave_like 'it is relayable' end From 8823bb01a2128c90f517cdfc8dfab6a5a3093534 Mon Sep 17 00:00:00 2001 From: realtin Date: Wed, 15 Jul 2015 10:03:17 +0200 Subject: [PATCH 15/17] refactor test to use let and better indentation (#6192) --- spec/models/acts_as_taggable_on_tag_spec.rb | 21 ++++---- spec/models/aspect_spec.rb | 2 +- spec/models/contact_spec.rb | 34 ++++++------- spec/models/conversation_spec.rb | 50 ++++++++++++------- spec/models/conversation_visibilities_spec.rb | 6 +-- 5 files changed, 61 insertions(+), 52 deletions(-) diff --git a/spec/models/acts_as_taggable_on_tag_spec.rb b/spec/models/acts_as_taggable_on_tag_spec.rb index 0d6d47981..4e5b21a26 100644 --- a/spec/models/acts_as_taggable_on_tag_spec.rb +++ b/spec/models/acts_as_taggable_on_tag_spec.rb @@ -8,7 +8,6 @@ describe ActsAsTaggableOn::Tag, :type => :model do it 'downcases the tag name' do expect(tag.autocomplete("CATS")).to eq([tag_cats]) - end it 'does an end where on tags' do @@ -23,16 +22,16 @@ describe ActsAsTaggableOn::Tag, :type => :model do it "removes punctuation and whitespace" do { - 'node.js' => 'nodejs', - '.dotatstart' => 'dotatstart', - 'you,inside' => 'youinside', - 'iam(parenthetical)' => 'iamparenthetical', - 'imeanit?maybe' => 'imeanitmaybe', - 'imeanit!' => 'imeanit', - 'how about spaces' => 'howaboutspaces', - "other\twhitespace\n" => 'otherwhitespace', - 'hash#inside' => 'hashinside', - 'f!u@n#k$y%-^h&a*r(a)c{t}e[r]s' => 'funky-characters' + 'node.js' => 'nodejs', + '.dotatstart' => 'dotatstart', + 'you,inside' => 'youinside', + 'iam(parenthetical)' => 'iamparenthetical', + 'imeanit?maybe' => 'imeanitmaybe', + 'imeanit!' => 'imeanit', + 'how about spaces' => 'howaboutspaces', + "other\twhitespace\n" => 'otherwhitespace', + 'hash#inside' => 'hashinside', + 'f!u@n#k$y%-^h&a*r(a)c{t}e[r]s' => 'funky-characters' }.each do |invalid, normalized| expect(tag.normalize(invalid)).to eq(normalized) end diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb index 7e247d405..770bf48a3 100644 --- a/spec/models/aspect_spec.rb +++ b/spec/models/aspect_spec.rb @@ -23,8 +23,8 @@ describe Aspect, :type => :model do it 'is able to have other users as contacts' do aspect = alice.aspects.create(name: 'losers') - Contact.create(user: alice, person: eve.person, aspects: [aspect]) + expect(aspect.contacts.where(person_id: alice.person.id)).to be_empty expect(aspect.contacts.where(person_id: eve.person.id)).not_to be_empty expect(aspect.contacts.size).to eq(1) diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 6bed1b92a..8732ff2fd 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -47,7 +47,6 @@ describe Contact, :type => :model do it "validates that the person's account is not closed" do person = FactoryGirl.create(:person, :closed_account => true) - contact = alice.contacts.new(person: person) expect(contact).not_to be_valid @@ -104,26 +103,23 @@ describe Contact, :type => :model do describe '#contacts' do before do - @alice = alice - @bob = bob - @eve = eve - @bob.aspects.create(name: 'next') - @bob.aspects(true) + bob.aspects.create(name: 'next') + bob.aspects(true) - @original_aspect = @bob.aspects.where(name: "generic").first - @new_aspect = @bob.aspects.where(name: "next").first + @original_aspect = bob.aspects.where(name: "generic").first + @new_aspect = bob.aspects.where(name: "next").first @people1 = [] @people2 = [] 1.upto(5) do person = FactoryGirl.build(:person) - @bob.contacts.create(person: person, aspects: [@original_aspect]) + bob.contacts.create(person: person, aspects: [@original_aspect]) @people1 << person end 1.upto(5) do person = FactoryGirl.build(:person) - @bob.contacts.create(person: person, aspects: [@new_aspect]) + bob.contacts.create(person: person, aspects: [@new_aspect]) @people2 << person end #eve <-> bob <-> alice @@ -131,13 +127,13 @@ describe Contact, :type => :model do context 'on a contact for a local user' do before do - @alice.reload - @alice.aspects.reload - @contact = @alice.contact_for(@bob.person) + alice.reload + alice.aspects.reload + @contact = alice.contact_for(bob.person) end it "returns the target local user's contacts that are in the same aspect" do - expect(@contact.contacts.map{|p| p.id}).to match_array([@eve.person].concat(@people1).map{|p| p.id}) + expect(@contact.contacts.map{|p| p.id}).to match_array([eve.person].concat(@people1).map{|p| p.id}) end it 'returns nothing if contacts_visible is false in that aspect' do @@ -147,14 +143,14 @@ describe Contact, :type => :model do end it 'returns no duplicate contacts' do - [@alice, @eve].each {|c| @bob.add_contact_to_aspect(@bob.contact_for(c.person), @bob.aspects.last)} + [alice, eve].each {|c| bob.add_contact_to_aspect(bob.contact_for(c.person), bob.aspects.last)} contact_ids = @contact.contacts.map{|p| p.id} expect(contact_ids.uniq).to eq(contact_ids) end end context 'on a contact for a remote user' do - let(:contact) { @bob.contact_for @people1.first } + let(:contact) { bob.contact_for @people1.first } it 'returns an empty array' do expect(contact.contacts).to eq([]) @@ -164,8 +160,8 @@ describe Contact, :type => :model do context 'requesting' do let(:contact) { Contact.new } - let(:user) { build(:user) } - let(:person) { build(:person) } + let(:user) { build(:user) } + let(:person) { build(:person) } before do contact.user = user @@ -203,7 +199,7 @@ describe Contact, :type => :model do it "adds to errors if potential contact is blocked by user" do person = eve.person - block = alice.blocks.create(person: person) + alice.blocks.create(person: person) bad_contact = alice.contacts.create(person: person) expect(bad_contact.send(:not_blocked_user)).to be false diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 35b79f412..6fd7feb8a 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -5,14 +5,20 @@ require 'spec_helper' describe Conversation, :type => :model do - let(:user1) { alice } - let(:user2) { bob } + let(:user1) { alice } + let(:user2) { bob } let(:participant_ids) { [user1.contacts.first.person.id, user1.person.id] } - let(:create_hash) { {author: user1.person, participant_ids: participant_ids, subject: "cool stuff", - messages_attributes: [ {author: user1.person, text: "hey"} ]} } - let(:conversation) { Conversation.create(create_hash) } - let(:message_last) { Message.create(author: user2.person, created_at: Time.now + 100, text: "last", conversation_id: conversation.id) } - let(:message_first) { Message.create(author: user2.person, created_at: Time.now + 100, text: "first", conversation_id: conversation.id) } + let(:create_hash) do + { + author: user1.person, + participant_ids: participant_ids, + subject: "cool stuff", + messages_attributes: [ {author: user1.person, text: "hey"} ] + } + end + let(:conversation) { Conversation.create(create_hash) } + let(:message_last) { Message.create(author: user2.person, created_at: Time.now + 100, text: "last", conversation_id: conversation.id) } + let(:message_first) { Message.create(author: user2.person, created_at: Time.now + 100, text: "first", conversation_id: conversation.id) } it 'creates a message on create' do expect{ conversation }.to change(Message, :count).by(1) @@ -65,7 +71,7 @@ describe Conversation, :type => :model do context 'transport' do let(:conversation_message) { conversation.messages.first } - let(:xml) { conversation.to_diaspora_xml } + let(:xml) { conversation.to_diaspora_xml } before do conversation @@ -77,7 +83,7 @@ describe Conversation, :type => :model do end it 'serializes the participants' do - create_hash[:participant_ids].each{|id| + create_hash[:participant_ids].each{ |id| expect(xml).to include(Person.find(id).diaspora_handle) } end @@ -126,9 +132,13 @@ describe Conversation, :type => :model do describe "#invalid parameters" do context "local author" do - let(:invalid_hash) { {author: peter.person, participant_ids: [peter.person.id, user1.person.id], - subject: "cool stuff", messages_attributes: [{author: peter.person, text: "hey"}]} } - + let(:invalid_hash) do + { + author: peter.person, + participant_ids: [peter.person.id, user1.person.id], + subject: "cool stuff", messages_attributes: [{author: peter.person, text: "hey"}] + } + end it "is invalid with invalid recipient" do invalid_conversation = Conversation.create(invalid_hash) expect(invalid_conversation).to be_invalid @@ -136,12 +146,16 @@ describe Conversation, :type => :model do end context "remote author" do - let(:remote_person) { remote_raphael } - let(:local_user) { alice } - let(:participant_ids) { [remote_person.id, local_user.person.id] } - let(:invalid_hash_remote) { {author: remote_person, participant_ids: participant_ids, - subject: "cool stuff", messages_attributes: [{author: remote_person, text: "hey"}]} } - + let(:remote_person) { remote_raphael } + let(:local_user) { alice } + let(:participant_ids) { [remote_person.id, local_user.person.id] } + let(:invalid_hash_remote) do + { + author: remote_person, + participant_ids: participant_ids, + subject: "cool stuff", messages_attributes: [{author: remote_person, text: "hey"}] + } + end it "is invalid with invalid recipient" do invalid_conversation_remote = Conversation.create(invalid_hash_remote) expect(invalid_conversation_remote).to be_invalid diff --git a/spec/models/conversation_visibilities_spec.rb b/spec/models/conversation_visibilities_spec.rb index a8a0e2708..cb3011fbc 100644 --- a/spec/models/conversation_visibilities_spec.rb +++ b/spec/models/conversation_visibilities_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe ConversationVisibility, type: :model do - let(:user1) { alice } + let(:user1) { alice } let(:participant_ids) { [user1.contacts.first.person.id, user1.person.id] } let(:create_hash) do { @@ -15,11 +15,11 @@ describe ConversationVisibility, type: :model do messages_attributes: [{ author: user1.person, text: 'hey' }] } end - let(:conversation) { Conversation.create(create_hash) } + let(:conversation) { Conversation.create(create_hash) } it 'destroy conversation when no participant' do conversation.conversation_visibilities.each(&:destroy) - + expect(Conversation).not_to exist(conversation.id) end end From c5a2334fbebc8d44c6a69eaa9292345544670883 Mon Sep 17 00:00:00 2001 From: zaziemo Date: Wed, 15 Jul 2015 13:10:54 +0200 Subject: [PATCH 16/17] refactor tests to use let instead of before blocks creating instance variables pronto checked #6192 --- spec/models/account_deletion_spec.rb | 32 ++++----- spec/models/acts_as_taggable_on_tag_spec.rb | 20 +++--- spec/models/aspect_membership_spec.rb | 12 ++-- spec/models/aspect_spec.rb | 2 +- spec/models/comment_spec.rb | 25 ++++--- spec/models/contact_spec.rb | 21 +++--- spec/models/conversation_spec.rb | 69 ++++++++----------- spec/models/conversation_visibilities_spec.rb | 14 ++-- 8 files changed, 84 insertions(+), 111 deletions(-) diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb index ad528bcc2..2d052dd66 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deletion_spec.rb @@ -5,27 +5,27 @@ require 'spec_helper' describe AccountDeletion, :type => :model do - let(:ad_new) { AccountDeletion.new(person: alice.person) } - let(:ad_create) { AccountDeletion.create(person: alice.person) } + let(:account_deletion_new) { AccountDeletion.new(person: alice.person) } + let(:account_deletion_create) { AccountDeletion.create(person: alice.person) } it 'assigns the diaspora_handle from the person object' do - expect(ad_new.diaspora_handle).to eq(alice.person.diaspora_handle) + expect(account_deletion_new.diaspora_handle).to eq(alice.person.diaspora_handle) end it 'fires a job after creation'do expect(Workers::DeleteAccount).to receive(:perform_async).with(anything) - ad_create + account_deletion_create end describe "#perform!" do it 'creates a deleter' do expect(AccountDeleter).to receive(:new).with(alice.person.diaspora_handle).and_return(double(perform!: true)) - ad_new.perform! + account_deletion_new.perform! end it 'dispatches the account deletion if the user exists' do - expect(ad_new).to receive(:dispatch) - ad_new.perform! + expect(account_deletion_new).to receive(:dispatch) + account_deletion_new.perform! end it 'does not dispatch an account deletion for non-local people' do @@ -35,15 +35,15 @@ describe AccountDeletion, :type => :model do end it 'marks an AccountDeletion as completed when successful' do - ad_create.perform! - expect(ad_create.reload.completed_at).not_to be_nil + account_deletion_create.perform! + expect(account_deletion_create.reload.completed_at).not_to be_nil end end describe '#dispatch' do it 'creates a public postzord' do expect(Postzord::Dispatcher::Public).to receive(:new).and_return(double.as_null_object) - ad_new.dispatch + account_deletion_new.dispatch end end @@ -51,20 +51,20 @@ describe AccountDeletion, :type => :model do it 'includes all remote contacts' do alice.share_with(remote_raphael, alice.aspects.first) - expect(ad_new.subscribers(alice)).to eq([remote_raphael]) + expect(account_deletion_new.subscribers(alice)).to eq([remote_raphael]) end it 'includes remote resharers' do - sm = FactoryGirl.create( :status_message, public: true, author: alice.person) - FactoryGirl.create( :reshare, author: remote_raphael, root: sm) - FactoryGirl.create( :reshare, author: local_luke.person, root: sm) + status_message = FactoryGirl.create(:status_message, public: true, author: alice.person) + FactoryGirl.create(:reshare, author: remote_raphael, root: status_message) + FactoryGirl.create(:reshare, author: local_luke.person, root: status_message) - expect(ad_new.subscribers(alice)).to eq([remote_raphael]) + expect(account_deletion_new.subscribers(alice)).to eq([remote_raphael]) end end describe 'serialization' do - let(:xml) { ad_new.to_xml.to_s } + let(:xml) { account_deletion_new.to_xml.to_s } it 'should have a diaspora_handle' do expect(xml.include?(alice.person.diaspora_handle)).to eq(true) diff --git a/spec/models/acts_as_taggable_on_tag_spec.rb b/spec/models/acts_as_taggable_on_tag_spec.rb index 4e5b21a26..214a57371 100644 --- a/spec/models/acts_as_taggable_on_tag_spec.rb +++ b/spec/models/acts_as_taggable_on_tag_spec.rb @@ -22,16 +22,16 @@ describe ActsAsTaggableOn::Tag, :type => :model do it "removes punctuation and whitespace" do { - 'node.js' => 'nodejs', - '.dotatstart' => 'dotatstart', - 'you,inside' => 'youinside', - 'iam(parenthetical)' => 'iamparenthetical', - 'imeanit?maybe' => 'imeanitmaybe', - 'imeanit!' => 'imeanit', - 'how about spaces' => 'howaboutspaces', - "other\twhitespace\n" => 'otherwhitespace', - 'hash#inside' => 'hashinside', - 'f!u@n#k$y%-^h&a*r(a)c{t}e[r]s' => 'funky-characters' + "node.js" => "nodejs", + ".dotatstart" => "dotatstart", + "you,inside" => "youinside", + "iam(parenthetical)" => "iamparenthetical", + "imeanit?maybe" => "imeanitmaybe", + "imeanit!" => "imeanit", + "how about spaces" => "howaboutspaces", + "other\twhitespace\n" => "otherwhitespace", + "hash#inside" => "hashinside", + "f!u@n#k$y%-^h&a*r(a)c{t}e[r]s" => "funky-characters" }.each do |invalid, normalized| expect(tag.normalize(invalid)).to eq(normalized) end diff --git a/spec/models/aspect_membership_spec.rb b/spec/models/aspect_membership_spec.rb index 97c5f7c77..6e33d3f37 100644 --- a/spec/models/aspect_membership_spec.rb +++ b/spec/models/aspect_membership_spec.rb @@ -5,28 +5,26 @@ require 'spec_helper' describe AspectMembership, :type => :model do - describe '#before_destroy' do - let(:aspect) { alice.aspects.create(name: "two") } + let(:aspect) { alice.aspects.create(name: "two") } let(:contact) { alice.contact_for(bob.person) } - let(:am) { alice.aspects.where(name: "generic").first.aspect_memberships.first } + let(:aspect_membership) { alice.aspects.where(name: "generic").first.aspect_memberships.first } before do - allow(am).to receive(:user).and_return(alice) + allow(aspect_membership).to receive(:user).and_return(alice) end it 'calls disconnect if its the last aspect for the contact' do expect(alice).to receive(:disconnect).with(contact) - am.destroy + aspect_membership.destroy end it 'does not call disconnect if its not the last aspect for the contact' do expect(alice).not_to receive(:disconnect) alice.add_contact_to_aspect(contact, aspect) - am.destroy + aspect_membership.destroy end end - end diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb index 770bf48a3..469ba313d 100644 --- a/spec/models/aspect_spec.rb +++ b/spec/models/aspect_spec.rb @@ -22,7 +22,7 @@ describe Aspect, :type => :model do end it 'is able to have other users as contacts' do - aspect = alice.aspects.create(name: 'losers') + aspect = alice.aspects.create(name: "losers") Contact.create(user: alice, person: eve.person, aspects: [aspect]) expect(aspect.contacts.where(person_id: alice.person.id)).to be_empty diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 8b26ab871..056998d43 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -6,9 +6,9 @@ require 'spec_helper' require Rails.root.join("spec", "shared_behaviors", "relayable") describe Comment, :type => :model do - let(:alices_aspect) { alice.aspects.first } - let(:status_bob) { bob.post(:status_message, text: "hello", to: bob.aspects.first.id) } - let(:comment_alice) { alice.comment!(status_bob, "why so formal?") } + let(:alices_aspect) { alice.aspects.first } + let(:status_bob) { bob.post(:status_message, text: "hello", to: bob.aspects.first.id) } + let(:comment_alice) { alice.comment!(status_bob, "why so formal?") } describe 'comment#notification_type' do it "returns 'comment_on_post' if the comment is on a post you own" do @@ -70,11 +70,11 @@ describe Comment, :type => :model do end describe 'xml' do - let(:commenter) { create(:user) } + let(:commenter) { create(:user) } let(:commenter_aspect) { commenter.aspects.create(name: "bruisers") } - let(:post) { alice.post :status_message, text: "hello", to: alices_aspect.id } - let(:comment) { commenter.comment!(post, "Fool!") } - let(:xml) { comment.to_xml.to_s } + let(:post) { alice.post :status_message, text: "hello", to: alices_aspect.id } + let(:comment) { commenter.comment!(post, "Fool!") } + let(:xml) { comment.to_xml.to_s } before do connect_users(alice, alices_aspect, commenter, commenter_aspect) @@ -110,12 +110,12 @@ describe Comment, :type => :model do end describe 'it is relayable' do - let(:remote_parent) { build(:status_message, author: remote_raphael) } - let(:local_parent) { local_luke.post :status_message, text: "hi", to: local_luke.aspects.first } - let(:object_by_parent_author) { local_luke.comment!(local_parent, "yo!") } - let(:object_by_recipient) { local_leia.build_comment(text: "yo", post: local_parent) } + let(:remote_parent) { build(:status_message, author: remote_raphael) } + let(:local_parent) { local_luke.post :status_message, text: "hi", to: local_luke.aspects.first } + let(:object_by_parent_author) { local_luke.comment!(local_parent, "yo!") } + let(:object_by_recipient) { local_leia.build_comment(text: "yo", post: local_parent) } let(:dup_object_by_parent_author) { object_by_parent_author.dup } - let(:object_on_remote_parent) { local_luke.comment!(remote_parent, "Yeah, it was great") } + let(:object_on_remote_parent) { local_luke.comment!(remote_parent, "Yeah, it was great") } before do # shared_behaviors/relayable.rb is still using instance variables, so we need to define them here. @@ -140,5 +140,4 @@ describe Comment, :type => :model do end it_should_behave_like 'it is taggable' end - end diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 8732ff2fd..736530e61 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -7,7 +7,7 @@ require 'spec_helper' describe Contact, :type => :model do describe 'aspect_memberships' do it 'deletes dependent aspect memberships' do - expect{ + expect { alice.contact_for(bob.person).destroy }.to change(AspectMembership, :count).by(-1) end @@ -103,7 +103,7 @@ describe Contact, :type => :model do describe '#contacts' do before do - bob.aspects.create(name: 'next') + bob.aspects.create(name: "next") bob.aspects(true) @original_aspect = bob.aspects.where(name: "generic").first @@ -133,7 +133,7 @@ describe Contact, :type => :model do end it "returns the target local user's contacts that are in the same aspect" do - expect(@contact.contacts.map{|p| p.id}).to match_array([eve.person].concat(@people1).map{|p| p.id}) + expect(@contact.contacts.map(&:id)).to match_array([eve.person].concat(@people1).map(&:id)) end it 'returns nothing if contacts_visible is false in that aspect' do @@ -143,8 +143,8 @@ describe Contact, :type => :model do end it 'returns no duplicate contacts' do - [alice, eve].each {|c| bob.add_contact_to_aspect(bob.contact_for(c.person), bob.aspects.last)} - contact_ids = @contact.contacts.map{|p| p.id} + [alice, eve].each {|c| bob.add_contact_to_aspect(bob.contact_for(c.person), bob.aspects.last) } + contact_ids = @contact.contacts.map(&:id) expect(contact_ids.uniq).to eq(contact_ids) end end @@ -159,14 +159,9 @@ describe Contact, :type => :model do end context 'requesting' do - let(:contact) { Contact.new } - let(:user) { build(:user) } - let(:person) { build(:person) } - - before do - contact.user = user - contact.person = person - end + let(:contact) { Contact.new user: user, person: person } + let(:user) { build(:user) } + let(:person) { build(:person) } describe '#generate_request' do it 'makes a request' do diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 6fd7feb8a..ad4abb9ec 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -5,23 +5,19 @@ require 'spec_helper' describe Conversation, :type => :model do - let(:user1) { alice } - let(:user2) { bob } + let(:user1) { alice } + let(:user2) { bob } let(:participant_ids) { [user1.contacts.first.person.id, user1.person.id] } - let(:create_hash) do - { - author: user1.person, - participant_ids: participant_ids, - subject: "cool stuff", - messages_attributes: [ {author: user1.person, text: "hey"} ] - } - end - let(:conversation) { Conversation.create(create_hash) } - let(:message_last) { Message.create(author: user2.person, created_at: Time.now + 100, text: "last", conversation_id: conversation.id) } - let(:message_first) { Message.create(author: user2.person, created_at: Time.now + 100, text: "first", conversation_id: conversation.id) } + let(:create_hash) {{author: user1.person, participant_ids: participant_ids, + subject: "cool stuff", messages_attributes: [{author: user1.person, text: "hey"}]}} + let(:conversation) { Conversation.create(create_hash) } + let(:message_last) { Message.create(author: user2.person, created_at: Time.now + 100, + text: "last", conversation_id: conversation.id) } + let(:message_first) { Message.create(author: user2.person, created_at: Time.now + 100, + text: "first", conversation_id: conversation.id) } it 'creates a message on create' do - expect{ conversation }.to change(Message, :count).by(1) + expect { conversation }.to change(Message, :count).by(1) end describe '#last_author' do @@ -49,13 +45,12 @@ describe Conversation, :type => :model do end it 'returns nil if there are no unread messages in a conversation' do - conversation.conversation_visibilities.where(person_id: user1.person.id).first.tap { |cv| cv.unread = 0 }.save + conversation.conversation_visibilities.where(person_id: user1.person.id).first.tap {|cv| cv.unread = 0 }.save expect(conversation.first_unread_message(user1)).to be_nil end end describe '#set_read' do - before do conversation message_first.increase_unread(user1) @@ -71,7 +66,7 @@ describe Conversation, :type => :model do context 'transport' do let(:conversation_message) { conversation.messages.first } - let(:xml) { conversation.to_diaspora_xml } + let(:xml) { conversation.to_diaspora_xml } before do conversation @@ -79,13 +74,13 @@ describe Conversation, :type => :model do describe 'serialization' do it 'serializes the message' do - expect(xml.gsub(/\s/, '')).to include(conversation_message.to_xml.to_s.gsub(/\s/, '')) + expect(xml.gsub(/\s/, "")).to include(conversation_message.to_xml.to_s.gsub(/\s/, "")) end it 'serializes the participants' do - create_hash[:participant_ids].each{ |id| + create_hash[:participant_ids].each do |id| expect(xml).to include(Person.find(id).diaspora_handle) - } + end end it 'serializes the created_at time' do @@ -95,7 +90,7 @@ describe Conversation, :type => :model do describe '#subscribers' do it 'returns the recipients for the post owner' do - expect(conversation.subscribers(user1)).to eq(user1.contacts.map{|c| c.person}) + expect(conversation.subscribers(user1)).to eq(user1.contacts.map(&:person)) end end @@ -106,17 +101,17 @@ describe Conversation, :type => :model do end it 'creates a message' do - expect{ + expect { Diaspora::Parser.from_xml(xml).receive(user1, user2.person) }.to change(Message, :count).by(1) end it 'creates a conversation' do - expect{ + expect { Diaspora::Parser.from_xml(xml).receive(user1, user2.person) }.to change(Conversation, :count).by(1) end it 'creates appropriate visibilities' do - expect{ + expect { Diaspora::Parser.from_xml(xml).receive(user1, user2.person) }.to change(ConversationVisibility, :count).by(participant_ids.size) end @@ -132,13 +127,9 @@ describe Conversation, :type => :model do describe "#invalid parameters" do context "local author" do - let(:invalid_hash) do - { - author: peter.person, - participant_ids: [peter.person.id, user1.person.id], - subject: "cool stuff", messages_attributes: [{author: peter.person, text: "hey"}] - } - end + let(:invalid_hash) {{author: peter.person, participant_ids: [peter.person.id, user1.person.id], + subject: "cool stuff", messages_attributes: [{author: peter.person, text: "hey"}]}} + it "is invalid with invalid recipient" do invalid_conversation = Conversation.create(invalid_hash) expect(invalid_conversation).to be_invalid @@ -146,16 +137,12 @@ describe Conversation, :type => :model do end context "remote author" do - let(:remote_person) { remote_raphael } - let(:local_user) { alice } - let(:participant_ids) { [remote_person.id, local_user.person.id] } - let(:invalid_hash_remote) do - { - author: remote_person, - participant_ids: participant_ids, - subject: "cool stuff", messages_attributes: [{author: remote_person, text: "hey"}] - } - end + let(:remote_person) { remote_raphael } + let(:local_user) { alice } + let(:participant_ids) { [remote_person.id, local_user.person.id] } + let(:invalid_hash_remote) {{author: remote_person, participant_ids: participant_ids, + subject: "cool stuff", messages_attributes: [{author: remote_person, text: "hey"}]}} + it "is invalid with invalid recipient" do invalid_conversation_remote = Conversation.create(invalid_hash_remote) expect(invalid_conversation_remote).to be_invalid diff --git a/spec/models/conversation_visibilities_spec.rb b/spec/models/conversation_visibilities_spec.rb index cb3011fbc..4a2bfed0f 100644 --- a/spec/models/conversation_visibilities_spec.rb +++ b/spec/models/conversation_visibilities_spec.rb @@ -5,17 +5,11 @@ require 'spec_helper' describe ConversationVisibility, type: :model do - let(:user1) { alice } + let(:user1) { alice } let(:participant_ids) { [user1.contacts.first.person.id, user1.person.id] } - let(:create_hash) do - { - author: user1.person, - participant_ids: participant_ids, - subject: 'cool stuff', - messages_attributes: [{ author: user1.person, text: 'hey' }] - } - end - let(:conversation) { Conversation.create(create_hash) } + let(:create_hash) {{author: user1.person, participant_ids: participant_ids, subject: "cool stuff", + messages_attributes: [{author: user1.person, text: "hey"}]}} + let(:conversation) { Conversation.create(create_hash) } it 'destroy conversation when no participant' do conversation.conversation_visibilities.each(&:destroy) From d0b290ea4dafc4cc82a920f13dff78e6383e3ee4 Mon Sep 17 00:00:00 2001 From: realtin Date: Wed, 15 Jul 2015 15:19:16 +0200 Subject: [PATCH 17/17] refactor hash indentation and add double quotes this is the suggested styling according to hound/pronto/rubocop (#6192) --- Changelog.md | 1 + spec/models/account_deletion_spec.rb | 28 +++---- spec/models/acts_as_taggable_on_tag_spec.rb | 10 +-- spec/models/aspect_membership_spec.rb | 8 +- spec/models/aspect_spec.rb | 18 ++--- spec/models/block_spec.rb | 6 +- spec/models/comment_spec.rb | 40 +++++----- spec/models/contact_spec.rb | 54 +++++++------- spec/models/conversation_spec.rb | 74 +++++++++++-------- spec/models/conversation_visibilities_spec.rb | 10 ++- 10 files changed, 131 insertions(+), 118 deletions(-) diff --git a/Changelog.md b/Changelog.md index 843dfcf4d..47f86cccd 100644 --- a/Changelog.md +++ b/Changelog.md @@ -6,6 +6,7 @@ * Add link to pod statistics in right navigation [#6117](https://github.com/diaspora/diaspora/pull/6117) * Refactor person related URL generation [#6168](https://github.com/diaspora/diaspora/pull/6168) * Move webfinger and HCard generation out of the core and embed the `diaspora_federation-rails` gem [#6151](https://github.com/diaspora/diaspora/pull/6151/) +* Refactor rspec tests to to use `let` instead of before blocks [#6199](https://github.com/diaspora/diaspora/pull/6199) ## Bug fixes * Precompile facebox images [#6105](https://github.com/diaspora/diaspora/pull/6105) diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb index 2d052dd66..0126edb77 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deletion_spec.rb @@ -2,59 +2,59 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -require 'spec_helper' +require "spec_helper" describe AccountDeletion, :type => :model do let(:account_deletion_new) { AccountDeletion.new(person: alice.person) } let(:account_deletion_create) { AccountDeletion.create(person: alice.person) } - it 'assigns the diaspora_handle from the person object' do + it "assigns the diaspora_handle from the person object" do expect(account_deletion_new.diaspora_handle).to eq(alice.person.diaspora_handle) end - it 'fires a job after creation'do + it "fires a job after creation"do expect(Workers::DeleteAccount).to receive(:perform_async).with(anything) account_deletion_create end describe "#perform!" do - it 'creates a deleter' do + it "creates a deleter" do expect(AccountDeleter).to receive(:new).with(alice.person.diaspora_handle).and_return(double(perform!: true)) account_deletion_new.perform! end - it 'dispatches the account deletion if the user exists' do + it "dispatches the account deletion if the user exists" do expect(account_deletion_new).to receive(:dispatch) account_deletion_new.perform! end - it 'does not dispatch an account deletion for non-local people' do + it "does not dispatch an account deletion for non-local people" do deletion = AccountDeletion.new(person: remote_raphael) expect(deletion).not_to receive(:dispatch) deletion.perform! end - it 'marks an AccountDeletion as completed when successful' do + it "marks an AccountDeletion as completed when successful" do account_deletion_create.perform! expect(account_deletion_create.reload.completed_at).not_to be_nil end end - describe '#dispatch' do - it 'creates a public postzord' do + describe "#dispatch" do + it "creates a public postzord" do expect(Postzord::Dispatcher::Public).to receive(:new).and_return(double.as_null_object) account_deletion_new.dispatch end end describe "#subscribers" do - it 'includes all remote contacts' do + it "includes all remote contacts" do alice.share_with(remote_raphael, alice.aspects.first) expect(account_deletion_new.subscribers(alice)).to eq([remote_raphael]) end - it 'includes remote resharers' do + it "includes remote resharers" do status_message = FactoryGirl.create(:status_message, public: true, author: alice.person) FactoryGirl.create(:reshare, author: remote_raphael, root: status_message) FactoryGirl.create(:reshare, author: local_luke.person, root: status_message) @@ -63,14 +63,14 @@ describe AccountDeletion, :type => :model do end end - describe 'serialization' do + describe "serialization" do let(:xml) { account_deletion_new.to_xml.to_s } - it 'should have a diaspora_handle' do + it "should have a diaspora_handle" do expect(xml.include?(alice.person.diaspora_handle)).to eq(true) end - it 'marshals the xml' do + it "marshals the xml" do expect(AccountDeletion.from_xml(xml)).to be_valid end end diff --git a/spec/models/acts_as_taggable_on_tag_spec.rb b/spec/models/acts_as_taggable_on_tag_spec.rb index 214a57371..84f1fd982 100644 --- a/spec/models/acts_as_taggable_on_tag_spec.rb +++ b/spec/models/acts_as_taggable_on_tag_spec.rb @@ -1,16 +1,16 @@ -require 'spec_helper' +require "spec_helper" describe ActsAsTaggableOn::Tag, :type => :model do subject(:tag) { ActsAsTaggableOn::Tag } - describe '.autocomplete' do + describe ".autocomplete" do let!(:tag_cats) { tag.create(name: "cats") } - it 'downcases the tag name' do + it "downcases the tag name" do expect(tag.autocomplete("CATS")).to eq([tag_cats]) end - it 'does an end where on tags' do + it "does an end where on tags" do expect(tag.autocomplete("CAT")).to eq([tag_cats]) end end @@ -37,7 +37,7 @@ describe ActsAsTaggableOn::Tag, :type => :model do end end - it 'allows for love' do + it "allows for love" do expect(tag.normalize("<3")).to eq("<3") expect(tag.normalize("#<3")).to eq("<3") end diff --git a/spec/models/aspect_membership_spec.rb b/spec/models/aspect_membership_spec.rb index 6e33d3f37..f57950794 100644 --- a/spec/models/aspect_membership_spec.rb +++ b/spec/models/aspect_membership_spec.rb @@ -2,10 +2,10 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. # -require 'spec_helper' +require "spec_helper" describe AspectMembership, :type => :model do - describe '#before_destroy' do + describe "#before_destroy" do let(:aspect) { alice.aspects.create(name: "two") } let(:contact) { alice.contact_for(bob.person) } let(:aspect_membership) { alice.aspects.where(name: "generic").first.aspect_memberships.first } @@ -14,13 +14,13 @@ describe AspectMembership, :type => :model do allow(aspect_membership).to receive(:user).and_return(alice) end - it 'calls disconnect if its the last aspect for the contact' do + it "calls disconnect if its the last aspect for the contact" do expect(alice).to receive(:disconnect).with(contact) aspect_membership.destroy end - it 'does not call disconnect if its not the last aspect for the contact' do + it "does not call disconnect if its not the last aspect for the contact" do expect(alice).not_to receive(:disconnect) alice.add_contact_to_aspect(contact, aspect) diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb index 469ba313d..3ccae9515 100644 --- a/spec/models/aspect_spec.rb +++ b/spec/models/aspect_spec.rb @@ -2,26 +2,26 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -require 'spec_helper' +require "spec_helper" describe Aspect, :type => :model do - describe 'creation' do + describe "creation" do let(:name) { alice.aspects.first.name } - it 'does not allow duplicate names' do + it "does not allow duplicate names" do expect { alice.aspects.create(name: name) }.not_to change(Aspect, :count) end - it 'validates case insensitiveness on names' do + it "validates case insensitiveness on names" do expect { alice.aspects.create(name: name.titleize) }.not_to change(Aspect, :count) end - it 'has a 20 character limit on names' do + it "has a 20 character limit on names" do aspect = Aspect.new(name: "this name is really too too too too too long") expect(aspect.valid?).to eq(false) end - it 'is able to have other users as contacts' do + it "is able to have other users as contacts" do aspect = alice.aspects.create(name: "losers") Contact.create(user: alice, person: eve.person, aspects: [aspect]) @@ -30,7 +30,7 @@ describe Aspect, :type => :model do expect(aspect.contacts.size).to eq(1) end - it 'has a contacts_visible? method' do + it "has a contacts_visible? method" do expect(alice.aspects.first.contacts_visible?).to be true end @@ -42,8 +42,8 @@ describe Aspect, :type => :model do end end - describe 'validation' do - it 'has no uniqueness of name between users' do + describe "validation" do + it "has no uniqueness of name between users" do aspect = alice.aspects.create(name: "New Aspect") aspect2 = eve.aspects.create(name: aspect.name) expect(aspect2).to be_valid diff --git a/spec/models/block_spec.rb b/spec/models/block_spec.rb index 2721ea1df..54f7317fd 100644 --- a/spec/models/block_spec.rb +++ b/spec/models/block_spec.rb @@ -1,8 +1,8 @@ -require 'spec_helper' +require "spec_helper" describe Block, :type => :model do - describe 'validations' do - it 'doesnt allow you to block yourself' do + describe "validations" do + it "doesnt allow you to block yourself" do block = alice.blocks.create(person: alice.person) expect(block.errors[:person_id].size).to eq(1) end diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 056998d43..b6a546706 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -2,7 +2,7 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -require 'spec_helper' +require "spec_helper" require Rails.root.join("spec", "shared_behaviors", "relayable") describe Comment, :type => :model do @@ -20,7 +20,7 @@ describe Comment, :type => :model do expect(comment_alice.notification_type(eve, alice.person)).to eq(Notifications::AlsoCommented) end - it 'returns false if the comment is not on a post you own and no one "also_commented"' do + it "returns false if the comment is not on a post you own and no one 'also_commented'" do expect(comment_alice.notification_type(eve, alice.person)).to be false end @@ -31,7 +31,7 @@ describe Comment, :type => :model do comment_alice end - it 'does not return also commented if the user commented' do + it "does not return also commented if the user commented" do expect(comment_eve.notification_type(eve, alice.person)).to eq(false) end @@ -41,7 +41,7 @@ describe Comment, :type => :model do end end - describe 'User#comment' do + describe "User#comment" do it "should be able to comment on one's own status" do bob.comment!(status_bob, "sup dog") expect(status_bob.reload.comments.first.text).to eq("sup dog") @@ -52,15 +52,15 @@ describe Comment, :type => :model do expect(status_bob.reload.comments.first.text).to eq("why so formal?") end - it 'does not multi-post a comment' do + it "does not multi-post a comment" do expect { comment_alice }.to change { Comment.count }.by(1) end end - describe 'counter cache' do - it 'increments the counter cache on its post' do + describe "counter cache" do + it "increments the counter cache on its post" do expect { comment_alice }.to change{ @@ -69,7 +69,7 @@ describe Comment, :type => :model do end end - describe 'xml' do + describe "xml" do let(:commenter) { create(:user) } let(:commenter_aspect) { commenter.aspects.create(name: "bruisers") } let(:post) { alice.post :status_message, text: "hello", to: alices_aspect.id } @@ -80,26 +80,26 @@ describe Comment, :type => :model do connect_users(alice, alices_aspect, commenter, commenter_aspect) end - it 'serializes the sender handle' do + it "serializes the sender handle" do expect(xml.include?(commenter.diaspora_handle)).to be true end - it 'serializes the post_guid' do + it "serializes the post_guid" do expect(xml).to include(post.guid) end - describe 'marshalling' do + describe "marshalling" do let(:marshalled_comment) { Comment.from_xml(xml) } - it 'marshals the author' do + it "marshals the author" do expect(marshalled_comment.author).to eq(commenter.person) end - it 'marshals the post' do + it "marshals the post" do expect(marshalled_comment.post).to eq(post) end - it 'tries to fetch a missing parent' do + it "tries to fetch a missing parent" do guid = post.guid marshalled_comment post.destroy @@ -109,7 +109,7 @@ describe Comment, :type => :model do end end - describe 'it is relayable' do + describe "it is relayable" do let(:remote_parent) { build(:status_message, author: remote_raphael) } let(:local_parent) { local_luke.post :status_message, text: "hi", to: local_luke.aspects.first } let(:object_by_parent_author) { local_luke.comment!(local_parent, "yo!") } @@ -119,7 +119,7 @@ describe Comment, :type => :model do before do # shared_behaviors/relayable.rb is still using instance variables, so we need to define them here. - # Suggestion: refactor all specs using shared_behaviors/relayable.rb to use 'let' + # Suggestion: refactor all specs using shared_behaviors/relayable.rb to use "let" @object_by_parent_author = object_by_parent_author @object_by_recipient = object_by_recipient @dup_object_by_parent_author = dup_object_by_parent_author @@ -127,17 +127,17 @@ describe Comment, :type => :model do end let(:build_object) { alice.build_comment(post: status_bob, text: "why so formal?") } - it_should_behave_like 'it is relayable' + it_should_behave_like "it is relayable" end - describe 'tags' do + describe "tags" do let(:object) { build(:comment) } before do # shared_behaviors/taggable.rb is still using instance variables, so we need to define them here. - # Suggestion: refactor all specs using shared_behaviors/taggable.rb to use 'let' + # Suggestion: refactor all specs using shared_behaviors/taggable.rb to use "let" @object = object end - it_should_behave_like 'it is taggable' + it_should_behave_like "it is taggable" end end diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 736530e61..b5efa37e1 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -2,31 +2,31 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -require 'spec_helper' +require "spec_helper" describe Contact, :type => :model do - describe 'aspect_memberships' do - it 'deletes dependent aspect memberships' do + describe "aspect_memberships" do + it "deletes dependent aspect memberships" do expect { alice.contact_for(bob.person).destroy }.to change(AspectMembership, :count).by(-1) end end - context 'validations' do + context "validations" do let(:contact) { Contact.new } - it 'requires a user' do + it "requires a user" do contact.valid? expect(contact.errors.full_messages).to include "User can't be blank" end - it 'requires a person' do + it "requires a person" do contact.valid? expect(contact.errors.full_messages).to include "Person can't be blank" end - it 'ensures user is not making a contact for himself' do + it "ensures user is not making a contact for himself" do contact.person = alice.person contact.user = alice @@ -34,7 +34,7 @@ describe Contact, :type => :model do expect(contact.errors.full_messages).to include "Cannot create self-contact" end - it 'validates uniqueness' do + it "validates uniqueness" do person = FactoryGirl.create(:person) contact2 = alice.contacts.create(person: person) @@ -54,9 +54,9 @@ describe Contact, :type => :model do end end - context 'scope' do - describe 'sharing' do - it 'returns contacts with sharing true' do + context "scope" do + describe "sharing" do + it "returns contacts with sharing true" do expect { alice.contacts.create!(sharing: true, person: FactoryGirl.create(:person)) alice.contacts.create!(sharing: false, person: FactoryGirl.create(:person)) @@ -66,8 +66,8 @@ describe Contact, :type => :model do end end - describe 'receiving' do - it 'returns contacts with sharing true' do + describe "receiving" do + it "returns contacts with sharing true" do expect { alice.contacts.create!(receiving: true, person: FactoryGirl.build(:person)) alice.contacts.create!(receiving: false, person: FactoryGirl.build(:person)) @@ -77,8 +77,8 @@ describe Contact, :type => :model do end end - describe 'only_sharing' do - it 'returns contacts with sharing true and receiving false' do + describe "only_sharing" do + it "returns contacts with sharing true and receiving false" do expect { alice.contacts.create!(receiving: true, sharing: true, person: FactoryGirl.build(:person)) alice.contacts.create!(receiving: false, sharing: true, person: FactoryGirl.build(:person)) @@ -91,7 +91,7 @@ describe Contact, :type => :model do end describe "all_contacts_of_person" do - it 'returns all contacts where the person is the passed in person' do + it "returns all contacts where the person is the passed in person" do person = FactoryGirl.create(:person) contact1 = FactoryGirl.create(:contact, person: person) contact2 = FactoryGirl.create(:contact) @@ -101,7 +101,7 @@ describe Contact, :type => :model do end end - describe '#contacts' do + describe "#contacts" do before do bob.aspects.create(name: "next") bob.aspects(true) @@ -125,7 +125,7 @@ describe Contact, :type => :model do #eve <-> bob <-> alice end - context 'on a contact for a local user' do + context "on a contact for a local user" do before do alice.reload alice.aspects.reload @@ -136,35 +136,35 @@ describe Contact, :type => :model do expect(@contact.contacts.map(&:id)).to match_array([eve.person].concat(@people1).map(&:id)) end - it 'returns nothing if contacts_visible is false in that aspect' do + it "returns nothing if contacts_visible is false in that aspect" do @original_aspect.contacts_visible = false @original_aspect.save expect(@contact.contacts).to eq([]) end - it 'returns no duplicate contacts' do + it "returns no duplicate contacts" do [alice, eve].each {|c| bob.add_contact_to_aspect(bob.contact_for(c.person), bob.aspects.last) } contact_ids = @contact.contacts.map(&:id) expect(contact_ids.uniq).to eq(contact_ids) end end - context 'on a contact for a remote user' do + context "on a contact for a remote user" do let(:contact) { bob.contact_for @people1.first } - it 'returns an empty array' do + it "returns an empty array" do expect(contact.contacts).to eq([]) end end end - context 'requesting' do + context "requesting" do let(:contact) { Contact.new user: user, person: person } let(:user) { build(:user) } let(:person) { build(:person) } - describe '#generate_request' do - it 'makes a request' do + describe "#generate_request" do + it "makes a request" do allow(contact).to receive(:user).and_return(user) request = contact.generate_request @@ -173,8 +173,8 @@ describe Contact, :type => :model do end end - describe '#dispatch_request' do - it 'pushes to people' do + describe "#dispatch_request" do + it "pushes to people" do allow(contact).to receive(:user).and_return(user) m = double() expect(m).to receive(:post) diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index ad4abb9ec..fe44e463d 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -2,26 +2,32 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -require 'spec_helper' +require "spec_helper" describe Conversation, :type => :model do let(:user1) { alice } let(:user2) { bob } let(:participant_ids) { [user1.contacts.first.person.id, user1.person.id] } - let(:create_hash) {{author: user1.person, participant_ids: participant_ids, - subject: "cool stuff", messages_attributes: [{author: user1.person, text: "hey"}]}} + let(:create_hash) { + {author: user1.person, participant_ids: participant_ids, + subject: "cool stuff", messages_attributes: [{author: user1.person, text: "hey"}]} + } let(:conversation) { Conversation.create(create_hash) } - let(:message_last) { Message.create(author: user2.person, created_at: Time.now + 100, - text: "last", conversation_id: conversation.id) } - let(:message_first) { Message.create(author: user2.person, created_at: Time.now + 100, - text: "first", conversation_id: conversation.id) } + let(:message_last) { + Message.create(author: user2.person, created_at: Time.now + 100, + text: "last", conversation_id: conversation.id) + } + let(:message_first) { + Message.create(author: user2.person, created_at: Time.now + 100, + text: "first", conversation_id: conversation.id) + } - it 'creates a message on create' do + it "creates a message on create" do expect { conversation }.to change(Message, :count).by(1) end - describe '#last_author' do - it 'returns the last author to a conversation' do + describe "#last_author" do + it "returns the last author to a conversation" do message_last expect(conversation.reload.last_author.id).to eq(user2.person.id) end @@ -35,36 +41,36 @@ describe Conversation, :type => :model do end end - describe '#first_unread_message' do + describe "#first_unread_message" do before do message_last.increase_unread(user1) end - it 'returns the first unread message if there are unread messages in a conversation' do + it "returns the first unread message if there are unread messages in a conversation" do conversation.first_unread_message(user1) == message_last end - it 'returns nil if there are no unread messages in a conversation' do + it "returns nil if there are no unread messages in a conversation" do conversation.conversation_visibilities.where(person_id: user1.person.id).first.tap {|cv| cv.unread = 0 }.save expect(conversation.first_unread_message(user1)).to be_nil end end - describe '#set_read' do + describe "#set_read" do before do conversation message_first.increase_unread(user1) message_last.increase_unread(user1) end - it 'sets the unread counter to 0' do + it "sets the unread counter to 0" do expect(conversation.conversation_visibilities.where(person_id: user1.person.id).first.unread).to eq(2) conversation.set_read(user1) expect(conversation.conversation_visibilities.where(person_id: user1.person.id).first.unread).to eq(0) end end - context 'transport' do + context "transport" do let(:conversation_message) { conversation.messages.first } let(:xml) { conversation.to_diaspora_xml } @@ -72,53 +78,53 @@ describe Conversation, :type => :model do conversation end - describe 'serialization' do - it 'serializes the message' do + describe "serialization" do + it "serializes the message" do expect(xml.gsub(/\s/, "")).to include(conversation_message.to_xml.to_s.gsub(/\s/, "")) end - it 'serializes the participants' do + it "serializes the participants" do create_hash[:participant_ids].each do |id| expect(xml).to include(Person.find(id).diaspora_handle) end end - it 'serializes the created_at time' do + it "serializes the created_at time" do expect(xml).to include(conversation_message.created_at.to_s) end end - describe '#subscribers' do - it 'returns the recipients for the post owner' do + describe "#subscribers" do + it "returns the recipients for the post owner" do expect(conversation.subscribers(user1)).to eq(user1.contacts.map(&:person)) end end - describe '#receive' do + describe "#receive" do before do Message.destroy_all Conversation.destroy_all end - it 'creates a message' do + it "creates a message" do expect { Diaspora::Parser.from_xml(xml).receive(user1, user2.person) }.to change(Message, :count).by(1) end - it 'creates a conversation' do + it "creates a conversation" do expect { Diaspora::Parser.from_xml(xml).receive(user1, user2.person) }.to change(Conversation, :count).by(1) end - it 'creates appropriate visibilities' do + it "creates appropriate visibilities" do expect { Diaspora::Parser.from_xml(xml).receive(user1, user2.person) }.to change(ConversationVisibility, :count).by(participant_ids.size) end - it 'does not save before receive' do + it "does not save before receive" do expect(Diaspora::Parser.from_xml(xml).persisted?).to be false end - it 'notifies for the message' do + it "notifies for the message" do expect(Notification).to receive(:notify).once Diaspora::Parser.from_xml(xml).receive(user1, user2.person) end @@ -127,8 +133,10 @@ describe Conversation, :type => :model do describe "#invalid parameters" do context "local author" do - let(:invalid_hash) {{author: peter.person, participant_ids: [peter.person.id, user1.person.id], - subject: "cool stuff", messages_attributes: [{author: peter.person, text: "hey"}]}} + let(:invalid_hash) { + {author: peter.person, participant_ids: [peter.person.id, user1.person.id], + subject: "cool stuff", messages_attributes: [{author: peter.person, text: "hey"}]} + } it "is invalid with invalid recipient" do invalid_conversation = Conversation.create(invalid_hash) @@ -140,8 +148,10 @@ describe Conversation, :type => :model do let(:remote_person) { remote_raphael } let(:local_user) { alice } let(:participant_ids) { [remote_person.id, local_user.person.id] } - let(:invalid_hash_remote) {{author: remote_person, participant_ids: participant_ids, - subject: "cool stuff", messages_attributes: [{author: remote_person, text: "hey"}]}} + let(:invalid_hash_remote) { + {author: remote_person, participant_ids: participant_ids, + subject: "cool stuff", messages_attributes: [{author: remote_person, text: "hey"}]} + } it "is invalid with invalid recipient" do invalid_conversation_remote = Conversation.create(invalid_hash_remote) diff --git a/spec/models/conversation_visibilities_spec.rb b/spec/models/conversation_visibilities_spec.rb index 4a2bfed0f..68a77a2f9 100644 --- a/spec/models/conversation_visibilities_spec.rb +++ b/spec/models/conversation_visibilities_spec.rb @@ -2,16 +2,18 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -require 'spec_helper' +require "spec_helper" describe ConversationVisibility, type: :model do let(:user1) { alice } let(:participant_ids) { [user1.contacts.first.person.id, user1.person.id] } - let(:create_hash) {{author: user1.person, participant_ids: participant_ids, subject: "cool stuff", - messages_attributes: [{author: user1.person, text: "hey"}]}} + let(:create_hash) { + {author: user1.person, participant_ids: participant_ids, subject: "cool stuff", + messages_attributes: [{author: user1.person, text: "hey"}]} + } let(:conversation) { Conversation.create(create_hash) } - it 'destroy conversation when no participant' do + it "destroy conversation when no participant" do conversation.conversation_visibilities.each(&:destroy) expect(Conversation).not_to exist(conversation.id)