Skip to content

Commit

Permalink
Removed fixtures.
Browse files Browse the repository at this point in the history
I originally setup the tests to use AnyFixture and share factories across specs. This proved to be very problematic. RSpec isn't really designed to setup data and use it repeatedly so I removed the fixtures and used factories in their place. This means the tests will hit the database a bit more and will be a bit slower but really it's a negligible difference.
  • Loading branch information
blimey85 committed Jun 20, 2018
1 parent 4d446cb commit 39aade5
Show file tree
Hide file tree
Showing 19 changed files with 46 additions and 123 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,3 @@
/coverage/
/brakeman-report.html

Gemfile\.lock
9 changes: 4 additions & 5 deletions .rspec
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
--color
#--order random
#--profile
#--backtrace
#--format Fuubar
--format documentation
--order random
--profile
--backtrace
--format Fuubar
--require rails_helper
1 change: 1 addition & 0 deletions .ruby-gemset
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
Bee-Connect
-global
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,5 @@ group :test do
gem 'shoulda-matchers', github: 'thoughtbot/shoulda-matchers', branch: 'master'
gem 'simplecov', '~> 0.14', require: false
gem 'simplecov-console', require: false
gem 'test-prof'
# gem 'test-prof'
end
4 changes: 1 addition & 3 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ GEM
simplecov-html (0.10.2)
spoon (0.0.6)
ffi
sprockets (3.7.1)
sprockets (3.7.2)
concurrent-ruby (~> 1.0)
rack (> 1, < 3)
sprockets-rails (3.2.1)
Expand All @@ -525,7 +525,6 @@ GEM
temple (0.8.0)
terrapin (0.6.0)
climate_control (>= 0.0.3, < 1.0)
test-prof (0.5.0)
thor (0.20.0)
thread_safe (0.3.6)
tilt (2.0.8)
Expand Down Expand Up @@ -612,7 +611,6 @@ DEPENDENCIES
simple_form
simplecov (~> 0.14)
simplecov-console
test-prof
tzinfo-data
uglifier (>= 1.3.0)
web-console
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/posts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def load_postable

def owned_post
if @klass.name == 'Group'
unless current_user == @post.user || current_user.role.name == 'admin' || (current_user.university_moderator?(@post.postable.id) && !@post.user.university_moderator?(@post.postable.id))
unless current_user == @post.user || current_user.role.name == 'admin' || (current_user.group_moderator?(@post.postable.id) && !@post.user.group_moderator?(@post.postable.id))
flash[:alert] = "That post doesn't belong to you!"
redirect_to root_path
end
Expand Down
4 changes: 2 additions & 2 deletions app/helpers/groups_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ def current_user_is_joined(current_user_id, group_id)
end

def moderation_links_and_power(post)
current_user == post.user || current_user.role.name == 'admin' || (current_user.university_moderator?(post.postable.id) && !post.user.university_moderator?(post.postable.id))
current_user == post.user || current_user.role.name == 'admin' || (current_user.group_moderator?(post.postable.id) && !post.user.group_moderator?(post.postable.id))
end

def moderation_power(_user, group)
return true if current_user.role.name == 'admin' || current_user.university_moderator?(group.id)
return true if current_user.role.name == 'admin' || current_user.group_moderator?(group.id)
false
end
end
4 changes: 2 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ def group_admin?(_group_id)
(role.name == 'admin') || (role.name == 'moderator')
end

def university_moderator?(group_id)
def group_moderator?(group_id)
(group_role_name(group_id) == 'moderator') || group_admin?(group_id)
end

def group_approved?(group_id)
university_moderator?(group_id) || (group_role_name(group_id) == 'group_approved')
group_moderator?(group_id) || (group_role_name(group_id) == 'group_approved')
end

def group_role_update(group_id, role_name)
Expand Down
2 changes: 1 addition & 1 deletion app/views/groups/_members_common_links.html.haml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
%div{class: "padding20 bglgray sb1 dib w250 text-center" ,id: "user_box_#{member.user.id}"}
= profile_avatar_select(member.user)
%h6= link_to member.user.full_name,profile_path(member.user.user_name)
- if current_user.university_moderator?(@group.id)
- if current_user.group_moderator?(@group.id)
- if type == 'pending'
= link_to 'Approve',approve_group_path(group.id,member.user.id), class: 'btn btn-success', id:"approve_user_#{member.user.id}", remote: true, method: :post
- if (type == 'pending' or type =='moderators' or type =='members' or type =='banned') && moderation_power(member.user,group)
Expand Down
2 changes: 1 addition & 1 deletion app/views/groups/members.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
= render 'members_common_links',group: @group , member: member,type: 'members'
%hr
- if current_user.group_approved?(@group.id)
- if(current_user.university_moderator?(@group.id))
- if(current_user.group_moderator?(@group.id))
%h4 Pending To Be Approved
-@members.each do |member|
-if(member.role_id == @pending_role_id)
Expand Down
2 changes: 1 addition & 1 deletion spec/factories/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

FactoryBot.define do
factory :user do
user_name { Faker::Internet.user_name(2..20) }
sequence(:user_name) { |n| Faker::Internet.user_name(2..20) + n.to_s }
email { Faker::Internet.email(user_name) }
bio { Faker::Hipster.paragraph(3) }
first_name { Faker::Name.first_name }
Expand Down
22 changes: 0 additions & 22 deletions spec/factories_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,3 @@
end
end
end

# Test user fixtures
describe 'The user1 fixture' do
include_context 'user1'
it 'is valid' do
user1.should be_valid
end
end

describe 'The user2 fixture' do
include_context 'user2'
it 'is valid' do
user2.should be_valid
end
end

describe 'The user3 fixture' do
include_context 'user3'
it 'is valid' do
user3.should be_valid
end
end
5 changes: 3 additions & 2 deletions spec/models/conversation_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# frozen_string_literal: true

RSpec.describe Conversation, type: :model do
# Include three user fixtures
%w[user1 user2 user3].each { |context| include_context context }
let!(:user1) { FactoryBot.create(:user) }
let!(:user2) { FactoryBot.create(:user) }
let!(:user3) { FactoryBot.create(:user) }

# user1 -> user2
let!(:conversation1) { FactoryBot.create(:conversation, author_id: user1.id, receiver_id: user2.id) }
Expand Down
17 changes: 1 addition & 16 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

RSpec.describe User, type: :model do
include_context 'user'
let!(:user) { FactoryBot.create(:user, first_name: 'Josh', last_name: 'Steiner') }

context 'Model Associations' do
it { is_expected.to belong_to(:role) }
Expand Down Expand Up @@ -32,21 +32,6 @@
# it { is_expected.to validate_format_of(:user_name).case_insensitive }
end

# TODO(Gary): these work locally and fail remotely - will investigate and fix
# context 'Scope Tests' do
# describe ':users_to_be_followed' do
# it 'returns users to be followed for given user', :aggregate_failures do
# expect(User.users_to_be_followed(user.id)).to include(user)
# end
# end
#
# describe ':search_name' do
# it 'returns users that match the search term', :aggregate_failures do
# expect(User.search_name(user.user_name)).to include(user)
# end
# end
# end

describe User, '#full_name' do
it 'returns the concatenated first and last names' do
expect(user.full_name).to eq 'Josh Steiner'
Expand Down
1 change: 0 additions & 1 deletion spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
# require only the support files necessary.
#
Dir[Rails.root.join('spec', 'support', '**', '*.rb')].each { |f| require f }
require 'test_prof/recipes/rspec/any_fixture'

# Checks for pending migration and applies them before tests are run.
# If you are not using ActiveRecord, you can remove this line.
Expand Down
3 changes: 1 addition & 2 deletions spec/support/database_cleaner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
DatabaseCleaner.strategy = :transaction
end

# config.before(:each, js: true) do
config.before(:each, type: :system) do
config.before(:each, js: true) do
DatabaseCleaner.strategy = :truncation
end

Expand Down
41 changes: 0 additions & 41 deletions spec/support/fixtures.rb

This file was deleted.

8 changes: 4 additions & 4 deletions spec/system/user_logs_in_spec.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# frozen_string_literal: true

RSpec.describe 'User logs in' do
include_context 'user'
RSpec.describe 'User logs in', type: :system do
let!(:new_user2) { FactoryBot.create(:user) }

let!(:valid_user_name) { user.user_name }
let!(:valid_password) { user.password }
let!(:valid_user_name) { new_user2.user_name }
let!(:valid_password) { new_user2.password }

let!(:invalid_user_name) { 'tester1' }
let!(:invalid_password) { '11111111' }
Expand Down
39 changes: 22 additions & 17 deletions spec/system/user_signs_up_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,43 +2,48 @@

require 'rails_helper'

RSpec.feature 'User signs Up', type: :system do
# before(:all) do
# # Ensure no emails are left from previous tests
# clear_emails
# end

RSpec.describe 'User signs Up', type: :system, js: true do
# Build stubbed user
let(:user) { FactoryBot.build_stubbed(:user) }
let!(:new_user) { FactoryBot.build_stubbed(:user) }

scenario 'open confirmation email' do
scenario 'open confirmation email', :aggregate_failures do
# Open home page
visit root_path

# Clink link to go to sign up page
click_link 'Sign up'

# Complete sign up form
fill_in 'user[email]', with: user.email
fill_in 'user[user_name]', with: user.user_name
fill_in 'user[first_name]', with: user.first_name
fill_in 'user[last_name]', with: user.last_name
fill_in 'user[password]', with: user.password
fill_in 'user[password_confirmation]', with: user.password
fill_in 'user[email]', with: new_user.email
fill_in 'user[user_name]', with: new_user.user_name
fill_in 'user[first_name]', with: new_user.first_name
fill_in 'user[last_name]', with: new_user.last_name
fill_in 'user[password]', with: new_user.password
fill_in 'user[password_confirmation]', with: new_user.password

# Submit sign up form
click_button 'Sign up'

# Open email address confirmation email
open_email(user.email)
open_email(new_user.email)

# Test that the welcome message is found in the email
expect(current_email).to have_content "Welcome #{user.email}!"
# Test that we received the email and it has the correct subject and message
expect(current_email.subject).to eq('Confirmation instructions')
expect(current_email).to have_content "Welcome #{new_user.email}!"

# Click link to confirm email address
current_email.click_link 'Confirm my account'

# Test that the email address is confirmed and correct success message is present
expect(page).to have_content 'Your email address has been successfully confirmed.'

# Test logging in
visit new_user_session_path
fill_in 'user_login', with: new_user.user_name
fill_in 'user_password', with: new_user.password

click_button 'Log in'

expect(page).to have_content('Signed in successfully.')
end
end

0 comments on commit 39aade5

Please sign in to comment.