From 8823bb01a2128c90f517cdfc8dfab6a5a3093534 Mon Sep 17 00:00:00 2001 From: realtin Date: Wed, 15 Jul 2015 10:03:17 +0200 Subject: [PATCH] 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