From 39aade549dd03f2efedf8f8b566d9aa03dcb8a8e Mon Sep 17 00:00:00 2001 From: Gary Traffanstedt Date: Tue, 19 Jun 2018 18:57:12 -0400 Subject: [PATCH] Removed fixtures. 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. --- .gitignore | 1 - .rspec | 9 ++-- .ruby-gemset | 1 + Gemfile | 2 +- Gemfile.lock | 4 +- app/controllers/posts_controller.rb | 2 +- app/helpers/groups_helper.rb | 4 +- app/models/user.rb | 4 +- .../groups/_members_common_links.html.haml | 2 +- app/views/groups/members.html.haml | 2 +- spec/factories/users.rb | 2 +- spec/factories_spec.rb | 22 ---------- spec/models/conversation_spec.rb | 5 ++- spec/models/user_spec.rb | 17 +------- spec/rails_helper.rb | 1 - spec/support/database_cleaner.rb | 3 +- spec/support/fixtures.rb | 41 ------------------- spec/system/user_logs_in_spec.rb | 8 ++-- spec/system/user_signs_up_spec.rb | 39 ++++++++++-------- 19 files changed, 46 insertions(+), 123 deletions(-) delete mode 100644 spec/support/fixtures.rb diff --git a/.gitignore b/.gitignore index 297111d..b48a188 100644 --- a/.gitignore +++ b/.gitignore @@ -26,4 +26,3 @@ /coverage/ /brakeman-report.html -Gemfile\.lock diff --git a/.rspec b/.rspec index 8e5b201..2d3d5d0 100644 --- a/.rspec +++ b/.rspec @@ -1,7 +1,6 @@ --color -#--order random -#--profile -#--backtrace -#--format Fuubar ---format documentation +--order random +--profile +--backtrace +--format Fuubar --require rails_helper diff --git a/.ruby-gemset b/.ruby-gemset index a0e143a..d260919 100644 --- a/.ruby-gemset +++ b/.ruby-gemset @@ -1 +1,2 @@ Bee-Connect +-global \ No newline at end of file diff --git a/Gemfile b/Gemfile index 8bc8f65..110215e 100644 --- a/Gemfile +++ b/Gemfile @@ -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 diff --git a/Gemfile.lock b/Gemfile.lock index f76372f..4ae11ed 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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) @@ -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) @@ -612,7 +611,6 @@ DEPENDENCIES simple_form simplecov (~> 0.14) simplecov-console - test-prof tzinfo-data uglifier (>= 1.3.0) web-console diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index c4ff916..5ffdc21 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -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 diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 81eefc9..9fcfbdd 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index e3293dc..1ba2863 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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) diff --git a/app/views/groups/_members_common_links.html.haml b/app/views/groups/_members_common_links.html.haml index 67812a3..ede1426 100644 --- a/app/views/groups/_members_common_links.html.haml +++ b/app/views/groups/_members_common_links.html.haml @@ -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) diff --git a/app/views/groups/members.html.haml b/app/views/groups/members.html.haml index eda5bb7..aa73afe 100644 --- a/app/views/groups/members.html.haml +++ b/app/views/groups/members.html.haml @@ -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) diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 0b8aef9..96351eb 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -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 } diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb index 908cb58..a729d22 100644 --- a/spec/factories_spec.rb +++ b/spec/factories_spec.rb @@ -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 diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 5bf821d..f42ef59 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -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) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index dc08ec4..aebc5d3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -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) } @@ -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' diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index bb395d7..600c488 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -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. diff --git a/spec/support/database_cleaner.rb b/spec/support/database_cleaner.rb index 17ce65f..cc118e0 100644 --- a/spec/support/database_cleaner.rb +++ b/spec/support/database_cleaner.rb @@ -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 diff --git a/spec/support/fixtures.rb b/spec/support/fixtures.rb deleted file mode 100644 index 84828c8..0000000 --- a/spec/support/fixtures.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -RSpec.shared_context 'user', user: true do - before(:all) do - @user = TestProf::AnyFixture.register(:user) do - FactoryBot.create(:user, first_name: 'Josh', last_name: 'Steiner') - end - end - - let(:user) { @user } -end - -RSpec.shared_context 'user1', user: true do - before(:all) do - @user1 = TestProf::AnyFixture.register(:user1) do - FactoryBot.create(:user) - end - end - - let(:user1) { @user1 } -end - -RSpec.shared_context 'user2', user: true do - before(:all) do - @user2 = TestProf::AnyFixture.register(:user2) do - FactoryBot.create(:user) - end - end - - let(:user2) { @user2 } -end - -RSpec.shared_context 'user3', user: true do - before(:all) do - @user3 = TestProf::AnyFixture.register(:user3) do - FactoryBot.create(:user) - end - end - - let(:user3) { @user3 } -end diff --git a/spec/system/user_logs_in_spec.rb b/spec/system/user_logs_in_spec.rb index a9863ea..c33c4fc 100644 --- a/spec/system/user_logs_in_spec.rb +++ b/spec/system/user_logs_in_spec.rb @@ -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' } diff --git a/spec/system/user_signs_up_spec.rb b/spec/system/user_signs_up_spec.rb index 741e8dc..96318ef 100644 --- a/spec/system/user_signs_up_spec.rb +++ b/spec/system/user_signs_up_spec.rb @@ -2,16 +2,11 @@ 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 @@ -19,26 +14,36 @@ 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