From 7d70e3159bd77fd6d2c5d6e51ca40ba5d46c597e Mon Sep 17 00:00:00 2001 From: Jordan Hatch Date: Sun, 10 Apr 2016 23:10:57 +1000 Subject: [PATCH 1/7] Add RSpec --- Gemfile | 11 +++++------ Gemfile.lock | 37 ++++++++++++++++++++++--------------- bin/rspec | 16 ++++++++++++++++ config/environments/test.rb | 1 - spec/rails_helper.rb | 20 ++++++++++++++++++++ spec/spec_helper.rb | 14 ++++++++++++++ 6 files changed, 77 insertions(+), 22 deletions(-) create mode 100755 bin/rspec create mode 100644 spec/rails_helper.rb create mode 100644 spec/spec_helper.rb diff --git a/Gemfile b/Gemfile index b1dc3ee..4b5d73f 100644 --- a/Gemfile +++ b/Gemfile @@ -26,12 +26,11 @@ gem 'sass-rails', '~> 4.0.3' gem 'uglifier', '~> 2.5.3' group :test do - gem "minitest-spec-rails" - gem "capybara-webkit" - gem "factory_girl_rails", '4.1.0' - gem "database_cleaner" - gem "webmock", require: false - gem "mocha", require: false + gem 'rspec-rails', '~> 3.0' + gem 'capybara', '~> 2.4.1' + gem 'factory_girl_rails' + gem 'database_cleaner' + gem 'webmock', require: false end group :production do diff --git a/Gemfile.lock b/Gemfile.lock index 079dedd..9bf0496 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -42,19 +42,16 @@ GEM multi_json arel (6.0.3) builder (3.2.2) - capybara (2.7.0) - addressable + capybara (2.4.4) mime-types (>= 1.16) nokogiri (>= 1.3.3) rack (>= 1.0.0) rack-test (>= 0.5.4) xpath (~> 2.0) - capybara-webkit (1.10.1) - capybara (>= 2.3.0, < 2.8.0) - json crack (0.4.3) safe_yaml (~> 1.0.0) database_cleaner (1.5.1) + diff-lcs (1.2.5) domain_name (0.5.20160310) unf (>= 0.0.5, < 1.0.0) erubis (2.7.0) @@ -94,15 +91,9 @@ GEM nokogiri (>= 1.5.9) mail (2.6.4) mime-types (>= 1.16, < 4) - metaclass (0.0.4) mime-types (2.99.1) mini_portile2 (2.0.0) minitest (5.8.4) - minitest-spec-rails (5.3.0) - minitest (~> 5.0) - rails (>= 4.1) - mocha (1.1.0) - metaclass (~> 0.0.1) multi_json (1.11.2) multi_xml (0.5.5) multipart-post (2.0.0) @@ -175,6 +166,23 @@ GEM http-cookie (>= 1.0.2, < 2.0) mime-types (>= 1.16, < 3.0) netrc (~> 0.7) + rspec-core (3.4.4) + rspec-support (~> 3.4.0) + rspec-expectations (3.4.0) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.4.0) + rspec-mocks (3.4.1) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.4.0) + rspec-rails (3.4.2) + actionpack (>= 3.0, < 4.3) + activesupport (>= 3.0, < 4.3) + railties (>= 3.0, < 4.3) + rspec-core (~> 3.4.0) + rspec-expectations (~> 3.4.0) + rspec-mocks (~> 3.4.0) + rspec-support (~> 3.4.0) + rspec-support (3.4.1) safe_yaml (1.0.4) sass (3.2.19) sass-rails (4.0.5) @@ -214,15 +222,13 @@ PLATFORMS DEPENDENCIES airbrake (~> 4.0.0) - capybara-webkit + capybara (~> 2.4.1) database_cleaner - factory_girl_rails (= 4.1.0) + factory_girl_rails formtastic googlebooks has_scope jquery-rails - minitest-spec-rails - mocha omniauth-google-oauth2 openlibrary paper_trail (~> 3.0.5) @@ -232,6 +238,7 @@ DEPENDENCIES rack-ssl-enforcer rails (= 4.2.6) rails_12factor + rspec-rails (~> 3.0) sass-rails (~> 4.0.3) uglifier (~> 2.5.3) webmock diff --git a/bin/rspec b/bin/rspec new file mode 100755 index 0000000..3e07513 --- /dev/null +++ b/bin/rspec @@ -0,0 +1,16 @@ +#!/usr/bin/env ruby +# +# This file was generated by Bundler. +# +# The application 'rspec' is installed as part of a gem, and +# this file is here to facilitate running it. +# + +require "pathname" +ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", + Pathname.new(__FILE__).realpath) + +require "rubygems" +require "bundler/setup" + +load Gem.bin_path("rspec-core", "rspec") diff --git a/config/environments/test.rb b/config/environments/test.rb index 251c637..2915c3d 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -32,7 +32,6 @@ config.after_initialize do PaperTrail.enabled = false end - config.minitest_spec_rails.mini_shoulda = true config.eager_load = false diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb new file mode 100644 index 0000000..2f3ed09 --- /dev/null +++ b/spec/rails_helper.rb @@ -0,0 +1,20 @@ +# This file is copied to spec/ when you run 'rails generate rspec:install' +ENV['RAILS_ENV'] ||= 'test' +require File.expand_path('../../config/environment', __FILE__) +# Prevent database truncation if the environment is production +abort("The Rails environment is running in production mode!") if Rails.env.production? +require 'spec_helper' +require 'rspec/rails' +# Add additional requires below this line. Rails is not loaded until this point! + +Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f } + +# Checks for pending migrations before tests are run. +# If you are not using ActiveRecord, you can remove this line. +ActiveRecord::Migration.maintain_test_schema! + +RSpec.configure do |config| + config.use_transactional_fixtures = true + + config.infer_spec_type_from_file_location! +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb new file mode 100644 index 0000000..5c9efca --- /dev/null +++ b/spec/spec_helper.rb @@ -0,0 +1,14 @@ +RSpec.configure do |config| + config.expect_with :rspec do |expectations| + expectations.include_chain_clauses_in_custom_matcher_descriptions = true + end + + config.mock_with :rspec do |mocks| + mocks.verify_partial_doubles = true + end + + config.disable_monkey_patching! + + config.order = :random + Kernel.srand config.seed +end From 056997421b336260f8b726e69b3c1fce8efa9255 Mon Sep 17 00:00:00 2001 From: Jordan Hatch Date: Sun, 10 Apr 2016 23:11:12 +1000 Subject: [PATCH 2/7] Add support files for stubs and factories --- spec/support/authentication_stub.rb | 36 +++++++++++++++++++++++++++++ spec/support/factory_girl.rb | 3 +++ spec/support/versioning_stub.rb | 22 ++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 spec/support/authentication_stub.rb create mode 100644 spec/support/factory_girl.rb create mode 100644 spec/support/versioning_stub.rb diff --git a/spec/support/authentication_stub.rb b/spec/support/authentication_stub.rb new file mode 100644 index 0000000..43830cc --- /dev/null +++ b/spec/support/authentication_stub.rb @@ -0,0 +1,36 @@ +module AuthenticationStub + def signed_in_user + @user ||= User.find_or_create_from_auth_hash!( + mock_auth_hash + ) + end + + def stub_omniauth + OmniAuth.config.test_mode = true + OmniAuth.config.mock_auth[:google] = OmniAuth::AuthHash.new(mock_auth_hash) + end + + def sign_in_user + visit new_session_path + click_on "Sign in with Google" + end + + def mock_auth_hash + { + provider: 'google', + uid: '12345', + info: { + name: "Stub User", + email: "stub.user@example.org" + } + } + end +end + +RSpec.configure do |config| + config.include AuthenticationStub + + config.before(:each) do + stub_omniauth + end +end diff --git a/spec/support/factory_girl.rb b/spec/support/factory_girl.rb new file mode 100644 index 0000000..eec437f --- /dev/null +++ b/spec/support/factory_girl.rb @@ -0,0 +1,3 @@ +RSpec.configure do |config| + config.include FactoryGirl::Syntax::Methods +end diff --git a/spec/support/versioning_stub.rb b/spec/support/versioning_stub.rb new file mode 100644 index 0000000..fd8073b --- /dev/null +++ b/spec/support/versioning_stub.rb @@ -0,0 +1,22 @@ +module VersioningStub + # https://github.com/airblade/paper_trail#globally + # and https://github.com/airblade/paper_trail/issues/341 + def with_versioning + was_enabled = PaperTrail.enabled? + controller_was_enabled = PaperTrail.enabled_for_controller? + + PaperTrail.enabled = true + PaperTrail.enabled_for_controller = true + + begin + yield + ensure + PaperTrail.enabled = was_enabled + PaperTrail.enabled_for_controller = PaperTrail.enabled_for_controller? + end + end +end + +RSpec.configure do |config| + config.include VersioningStub +end From f43f2500f060e966d196b837a420b80ffdead855 Mon Sep 17 00:00:00 2001 From: Jordan Hatch Date: Sun, 10 Apr 2016 23:11:32 +1000 Subject: [PATCH 3/7] Move and expand factories from minitest suite --- {test => spec}/factories/book.rb | 0 {test => spec}/factories/copy.rb | 0 spec/factories/loan.rb | 20 ++++++++++++++++++++ {test => spec}/factories/user.rb | 4 +++- test/factories/loan.rb | 9 --------- 5 files changed, 23 insertions(+), 10 deletions(-) rename {test => spec}/factories/book.rb (100%) rename {test => spec}/factories/copy.rb (100%) create mode 100644 spec/factories/loan.rb rename {test => spec}/factories/user.rb (83%) delete mode 100644 test/factories/loan.rb diff --git a/test/factories/book.rb b/spec/factories/book.rb similarity index 100% rename from test/factories/book.rb rename to spec/factories/book.rb diff --git a/test/factories/copy.rb b/spec/factories/copy.rb similarity index 100% rename from test/factories/copy.rb rename to spec/factories/copy.rb diff --git a/spec/factories/loan.rb b/spec/factories/loan.rb new file mode 100644 index 0000000..7e82c95 --- /dev/null +++ b/spec/factories/loan.rb @@ -0,0 +1,20 @@ +FactoryGirl.define do + factory :loan do + user + copy + state "on_loan" + sequence(:loan_date) {|n| + Date.parse('2000-01-01') + n.week + } + return_date nil + + trait :returned do + state :returned + sequence(:return_date) {|n| + Date.parse('2001-01-01') + n.week + } + end + + factory :returned_loan, traits: [:returned] + end +end diff --git a/test/factories/user.rb b/spec/factories/user.rb similarity index 83% rename from test/factories/user.rb rename to spec/factories/user.rb index 662941e..72bf088 100644 --- a/test/factories/user.rb +++ b/spec/factories/user.rb @@ -1,6 +1,8 @@ FactoryGirl.define do factory :user do - name 'Winston Smith-Churchill' + sequence(:name) {|n| + "User#{n}" + } sequence(:email) {|n| "user#{n}@example.org" } diff --git a/test/factories/loan.rb b/test/factories/loan.rb deleted file mode 100644 index 1a6c6e0..0000000 --- a/test/factories/loan.rb +++ /dev/null @@ -1,9 +0,0 @@ -FactoryGirl.define do - factory :loan do - user - copy - state "on_loan" - loan_date { Time.now } - return_date nil - end -end From fa90bd3a03d751c12b05894073a71a82fa6268cf Mon Sep 17 00:00:00 2001 From: Jordan Hatch Date: Sun, 10 Apr 2016 23:12:17 +1000 Subject: [PATCH 4/7] Add feature spec for browsing books --- spec/features/browsing_books_spec.rb | 62 ++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 spec/features/browsing_books_spec.rb diff --git a/spec/features/browsing_books_spec.rb b/spec/features/browsing_books_spec.rb new file mode 100644 index 0000000..eb65975 --- /dev/null +++ b/spec/features/browsing_books_spec.rb @@ -0,0 +1,62 @@ +require 'rails_helper' + +RSpec.describe 'browsing books', type: :feature do + let!(:book) { create(:book) } + before(:each) { sign_in_user } + + it 'displays the book details' do + visit "/books/#{book.id}" + + within ".cover" do + expect(page).to have_selector("img[src='http://bks0.books.google.co.uk/books?id=#{book.google_id}&printsec=frontcover&img=1&zoom=1&edge=none&source=gbs_api']") + end + + within ".title" do + expect(page).to have_content(book.title) + expect(page).to have_content("by #{book.author}") + end + end + + it 'lists the copies of the book' do + visit "/books/#{book.id}" + + expect(page).to have_content("1 copy") + within ".copies li" do + expect(page).to have_link("#1", :href => "/copy/#{book.copies.first.to_param}") + expect(page).to have_content("Available to borrow") + expect(page).to have_link("Borrow", :href => "/copy/#{book.copies.first.to_param}/borrow") + end + end + + it 'lists previous versions of the book' do + previous_editor = create(:user) + + with_versioning do + PaperTrail.whodunnit = previous_editor.id.to_s + book.update_attributes!(title: 'Goodnight Mister Tom') + + visit "/books/#{book.id}" + click_on 'See revision history' + + expect(page).to have_selector('table.history') + + entries = page.all('table.history tr').map {|row| + cells = row.all('td') + cells.map(&:text).map(&:strip) + } + + expect(entries).to contain_exactly( + [ previous_editor.name, "less than a minute ago", "Title: Goodnight Mister Tom" ] + ) + end + end + + it 'shows a message when no previous book versions exist' do + visit "/books/#{book.id}" + click_on "See revision history" + + assert page.has_content?(book.title) + assert page.has_content?("No changes have been made to this book yet.") + end + +end From 69922fff4f7c028ce98285f7739503f096fbe192 Mon Sep 17 00:00:00 2001 From: Jordan Hatch Date: Sun, 10 Apr 2016 23:12:29 +1000 Subject: [PATCH 5/7] Add feature spec for the start page --- spec/features/start_page_spec.rb | 41 ++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 spec/features/start_page_spec.rb diff --git a/spec/features/start_page_spec.rb b/spec/features/start_page_spec.rb new file mode 100644 index 0000000..2040ab8 --- /dev/null +++ b/spec/features/start_page_spec.rb @@ -0,0 +1,41 @@ +require 'rails_helper' + +RSpec.describe 'start page', type: :feature do + before(:each) { sign_in_user } + + it 'tells the user how many books they have on loan' do + create_list(:loan, 5, user: signed_in_user) + + visit root_path + + expect(page).to have_content("Welcome, #{signed_in_user.name}!") + expect(page).to have_content('You have 5 books on loan') + end + + it 'lists recently added books' do + create_list(:copy, 10) + recent_copy = create(:copy) + + visit root_path + + within '.recently-added' do + expect(page).to have_link(recent_copy.book.title, + href: copy_path(recent_copy)) + end + end + + it 'list recent loans' do + loans = create_list(:loan, 5) + + visit root_path + + within '.recent-activity' do + loans.reverse.each_with_index do |loan, i| + within "li:nth-of-type(#{i+1})" do + expect(page).to have_link(loan.user.name, href: user_path(loan.user)) + expect(page).to have_link("##{loan.copy.book_reference}: #{loan.book.title}", href: copy_path(loan.copy)) + end + end + end + end +end From 5625050936e36ef3ac967f629c476f0e6a774397 Mon Sep 17 00:00:00 2001 From: Jordan Hatch Date: Sun, 10 Apr 2016 23:12:54 +1000 Subject: [PATCH 6/7] Add feature spec for borrowing books This also refactors the date formatting to use locale files to reduce repetition. --- app/views/copies/show.html.erb | 2 +- config/locales/en.yml | 4 +- spec/features/borrowing_books_spec.rb | 73 +++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 spec/features/borrowing_books_spec.rb diff --git a/app/views/copies/show.html.erb b/app/views/copies/show.html.erb index 873ab1c..92a8cd8 100644 --- a/app/views/copies/show.html.erb +++ b/app/views/copies/show.html.erb @@ -19,7 +19,7 @@ <% @previous_loans.each do |loan| %> - +
<%= loan.loan_date.strftime("%e %B %Y") if loan.loan_date %> - <%= loan.return_date.strftime("%e %B %Y") if loan.return_date %><%= l loan.loan_date, format: :short_date if loan.loan_date %> - <%= l loan.return_date, format: :short_date if loan.return_date %> <%= link_to loan.user.name, user_path(loan.user) %> <% if loan.returned_by_another_user? %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 179c14c..55b35c9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2,4 +2,6 @@ # See https://github.com/svenfuchs/rails-i18n/tree/master/rails%2Flocale for starting points. en: - hello: "Hello world" + time: + formats: + short_date: '%-d %B %Y' diff --git a/spec/features/borrowing_books_spec.rb b/spec/features/borrowing_books_spec.rb new file mode 100644 index 0000000..9f2eed1 --- /dev/null +++ b/spec/features/borrowing_books_spec.rb @@ -0,0 +1,73 @@ +require 'rails_helper' + +RSpec.describe 'borrowing books', type: :feature do + let(:book) { create(:book) } + let(:copy) { create(:copy, book: book) } + + before(:each) { sign_in_user } + + it 'borrows a copy that is available' do + visit copy_path(copy) + + expect(page).to have_content('Available') + + click_on 'Borrow' + + expect(page).to have_content('On loan to you') + end + + it 'cannot borrow a copy that is on loan' do + copy.borrow(create(:user)) + visit copy_path(copy) + + expect(page).to have_content('On loan') + expect(page).to_not have_link('Borrow') + end + + it 'returns a copy on loan to the current user' do + copy.borrow(signed_in_user) + visit copy_path(copy) + + expect(page).to have_content('On loan to you') + + click_on 'Return' + + expect(page).to have_content('Available') + end + + it 'returns a copy on loan to another user' do + other_user = create(:user) + copy.borrow(other_user) + visit copy_path(copy) + + expect(page).to have_content("On loan to #{other_user.name}") + + click_on 'Return' + + expect(page).to have_content('Available') + expect(page).to have_content("returned by #{signed_in_user.name}") + end + + it 'lists previous loans of a copy' do + previous_user = create(:user) + + loans = create_list(:returned_loan, 5, copy: copy, user: previous_user) + + visit copy_path(copy) + + entries = page.all('table.history tr').map {|row| + cells = row.all('th, td') + cells.map(&:text).map(&:strip) + } + + expect(entries).to contain_exactly( + *loans.map {|loan| + [ + "#{I18n.localize loan.loan_date, format: :short_date} - #{I18n.localize loan.return_date, format: :short_date}", + loan.user.name, + ] + } + ) + end + +end From cfd2f364d3053337d0708a05fca16e538cc5b92a Mon Sep 17 00:00:00 2001 From: Jordan Hatch Date: Sun, 10 Apr 2016 23:13:21 +1000 Subject: [PATCH 7/7] Update Omniauth dev strategy to ask for email --- config/initializers/omniauth.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index e7fbf4a..5c43428 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -1,5 +1,5 @@ Rails.application.config.middleware.use OmniAuth::Builder do - provider :developer, { :fields => [:id, :name, :nickname], :uid_field => :id } if Rails.env.development? + provider :developer, { :fields => [:id, :name, :email], :uid_field => :id } if Rails.env.development? provider :google_oauth2, ENV["GOOGLE_CLIENT_ID"], ENV["GOOGLE_CLIENT_SECRET"], { name: "google",