Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul Spec Tests #576

Open
wants to merge 41 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
465a4d3
Test spec fix.
MatthewKennedy Dec 26, 2022
bcdad52
Update rspec syntax
MatthewKennedy Dec 26, 2022
2a8ad72
Use locale.
MatthewKennedy Dec 26, 2022
020266e
Reset locale after test.
MatthewKennedy Dec 26, 2022
a27ec8a
Ensure locale is set to :en after tests.
MatthewKennedy Dec 26, 2022
f8b0e46
Test without spec
MatthewKennedy Dec 26, 2022
8a0e2aa
Admin Test
MatthewKennedy Dec 26, 2022
9cc23ed
Lint file.
MatthewKennedy Dec 26, 2022
d8a53fd
Lint file.
MatthewKennedy Dec 26, 2022
32b4829
Store artifacts.
MatthewKennedy Dec 26, 2022
b9c4022
Remove useless variable assignment.
MatthewKennedy Dec 26, 2022
d1f78ac
Add # frozen_string_literal: true
MatthewKennedy Dec 26, 2022
10a4c19
Remove duplicate check.
MatthewKennedy Dec 26, 2022
ab9af92
Ensure I18n.locale = :en
MatthewKennedy Dec 26, 2022
f0b8c50
Ensure en local is used.
MatthewKennedy Dec 26, 2022
de426ee
Lint file
MatthewKennedy Dec 26, 2022
0d62f2e
Lint file.
MatthewKennedy Dec 26, 2022
2b1c905
Lint file.
MatthewKennedy Dec 26, 2022
5d83015
Use css id
MatthewKennedy Dec 26, 2022
655e712
Use spree.admin_login_path(locale: 'en')
MatthewKennedy Dec 26, 2022
d76835d
Add current store
MatthewKennedy Dec 26, 2022
94b383b
Fix current_store
MatthewKennedy Dec 26, 2022
1dc4594
Lint code.
MatthewKennedy Dec 26, 2022
78f9a7e
Lint files.
MatthewKennedy Dec 26, 2022
27244b2
set default store.
MatthewKennedy Dec 26, 2022
1bd4a86
use describe it
MatthewKennedy Dec 26, 2022
3c621ca
Replace it
MatthewKennedy Dec 26, 2022
6407863
Lint file.
MatthewKennedy Dec 26, 2022
f7f3f2b
Lint file.
MatthewKennedy Dec 27, 2022
4b7651a
Fix admin sign out spec.
MatthewKennedy Dec 27, 2022
dc77e6f
Use admin Path
MatthewKennedy Dec 27, 2022
401744e
More descriptive log_out method.
MatthewKennedy Dec 27, 2022
55095f8
Lint file.
MatthewKennedy Dec 27, 2022
a95279e
Lint files.
MatthewKennedy Dec 27, 2022
89aef7c
Us it not scenario.
MatthewKennedy Dec 27, 2022
c0ce112
Update Rspec syntax (Replace background with before)
MatthewKennedy Dec 27, 2022
c57960f
Update Rspec syntax (replace 'scenario' with 'it')
MatthewKennedy Dec 27, 2022
1bd2235
Update Rspec syntax (replace 'given' with 'let')
MatthewKennedy Dec 27, 2022
dcc04cd
Rubocop lint files.
MatthewKennedy Dec 27, 2022
ae20ad9
Differentiate between admin and storefront test helpers
MatthewKennedy Dec 28, 2022
a9ec228
Fix target.
MatthewKennedy Dec 28, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ run_tests: &run_tests
- store_test_results:
path: ~/rspec
- store_artifacts:
path: tmp/capybara
path: /tmp/test-artifacts

run_tests_3_0: &run_tests_3_0
<<: *defaults_3_0
Expand Down Expand Up @@ -86,7 +86,7 @@ run_tests_3_0: &run_tests_3_0
- store_test_results:
path: ~/rspec
- store_artifacts:
path: tmp/capybara
path: /tmp/test-artifacts

jobs:
bundle:
Expand Down
6 changes: 1 addition & 5 deletions lib/spree/auth/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def self.activate
Rails.configuration.cache_classes ? require(c) : load(c)
end
if Spree::Auth::Engine.backend_available?
Dir.glob(File.join(File.dirname(__FILE__), "../../controllers/backend/*/*/*_decorator*.rb")) do |c|
Dir.glob(File.join(File.dirname(__FILE__), "../../controllers/backend/**/*_decorator*.rb")) do |c|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch :)

Rails.configuration.cache_classes ? require(c) : load(c)
end
end
Expand Down Expand Up @@ -60,10 +60,6 @@ def self.frontend_available?
@@frontend_available ||= Gem::Specification.find_all_by_name('spree_frontend').any?
end

def self.api_available?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is safe to be removed, it's used e.g. in lib/spree/auth/engine.rb:43, to determine whether to load decorators for API classes.

@@api_available ||= ::Rails::Engine.subclasses.map(&:instance).map{ |e| e.class.to_s }.include?('Spree::Api::Engine')
end

def self.emails_available?
@@emails_available ||= ::Rails::Engine.subclasses.map(&:instance).map{ |e| e.class.to_s }.include?('Spree::Emails::Engine')
end
Expand Down
27 changes: 23 additions & 4 deletions lib/spree/testing_support/auth_helpers.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Spree
module TestingSupport
module AuthHelpers
Expand All @@ -9,6 +11,23 @@ def logout_button
Spree.version.to_f == 4.1 ? Spree.t('nav_bar.log_out') : Spree.t(:logout).upcase
end

def admin_login(email:, password:, remember_me: true, locale: nil)
visit spree.admin_path(locale: locale)

fill_in id: 'spree_user_email', with: email
fill_in id: 'spree_user_password', with: password

first('label', text: Spree.t(:remember_me)).click if remember_me

click_button login_button
end

def assert_admin_login_success(locale = :en)
expect(page).to have_css('.admin')
expect(current_path).to eq '/admin'
expect(page).to have_css("html[lang='#{locale.to_s}']")
end

def log_in(email:, password:, remember_me: true, locale: nil)
visit spree.login_path(locale: locale)

Expand All @@ -22,20 +41,20 @@ def log_in(email:, password:, remember_me: true, locale: nil)
expect(page).to have_content Spree.t(:logged_in_successfully)
end

def log_out
show_user_menu
def log_out_via_frontend_user_menu
show_frontend_user_menu
click_link logout_button

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

def show_user_menu
def show_frontend_user_menu
find("button[aria-label='#{Spree.t('nav_bar.show_user_menu')}']").click
end

def show_user_account
within '#nav-bar' do
show_user_menu
show_frontend_user_menu
click_link Spree.t(:my_account).upcase
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/spree/testing_support/checkout_helpers.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Spree
module TestingSupport
module CheckoutHelpers
Expand Down
2 changes: 2 additions & 0 deletions spec/controllers/spree/admin/orders_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Spree
module Admin
RSpec.describe OrdersController, type: :controller do
Expand Down
2 changes: 2 additions & 0 deletions spec/controllers/spree/admin/user_sessions_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

RSpec.describe Spree::Admin::UserSessionsController, type: :controller do
before { @request.env['devise.mapping'] = Devise.mappings[:spree_user] }

Expand Down
18 changes: 10 additions & 8 deletions spec/controllers/spree/checkout_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
Spree::Store.default.update(default_locale: 'en', supported_locales: 'en,fr') if Spree.version.to_f >= 4.2
end

after { I18n.locale = :en }

context '#edit' do
context 'when registration step enabled' do
before do
Expand All @@ -33,8 +35,8 @@

context 'non default locale' do
it 'redirects to registration step with non default locale' do
get :edit, params: { state: 'address', locale: 'fr' }
expect(response).to redirect_to spree.checkout_registration_path(locale: 'fr')
get :edit, params: { state: 'address', locale: :fr }
expect(response).to redirect_to spree.checkout_registration_path(locale: :fr)
end
end
end
Expand Down Expand Up @@ -97,8 +99,8 @@
it 'redirects to the tokenized order view with a non default locale' do
# spree version higher than 3.6 required for test to work correctly
request.cookie_jar.signed[:token] = 'ABC'
post :update, params: { state: 'confirm', locale: 'fr' }
expect(response).to redirect_to spree.order_path(order, locale: 'fr')
post :update, params: { state: 'confirm', locale: :fr }
expect(response).to redirect_to spree.order_path(order, locale: :fr)
end
end
end
Expand All @@ -121,8 +123,8 @@

context 'non default locale' do
it 'redirects to the standard order view with a non default locale' do
post :update, params: { state: 'confirm', locale: 'fr' }
expect(response).to redirect_to spree.order_path(order, locale: 'fr')
post :update, params: { state: 'confirm', locale: :fr }
expect(response).to redirect_to spree.order_path(order, locale: :fr)
end
end
end
Expand Down Expand Up @@ -174,8 +176,8 @@
context 'non default locale' do
it 'redirects to the checkout_path after saving with non default locale' do
allow(controller).to receive(:check_authorization)
put :update_registration, params: { order: { email: 'jobs@spreecommerce.com' }, locale: 'fr' }
expect(response).to redirect_to spree.checkout_state_path(:address, locale: 'fr')
put :update_registration, params: { order: { email: 'jobs@spreecommerce.com' }, locale: :fr }
expect(response).to redirect_to spree.checkout_state_path(:address, locale: :fr)
end
end

Expand Down
12 changes: 5 additions & 7 deletions spec/controllers/spree/user_registrations_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
RSpec.describe Spree::UserRegistrationsController, type: :controller do
before { @request.env['devise.mapping'] = Devise.mappings[:spree_user] }

after { I18n.locale = :en }

context '#create' do
before { allow(controller).to receive(:after_sign_up_path_for).and_return(spree.account_path) }

Expand All @@ -14,15 +16,13 @@
Spree::Store.default.update(default_locale: 'en', supported_locales: 'en,fr')
end

after { I18n.locale = :en }

it 'redirects to account_path with locale' do
post :create, params: { spree_user: { email: 'foobar@example.com', password: 'foobar123', password_confirmation: 'foobar123' }, locale: 'fr'}
expect(response).to redirect_to spree.account_path(locale: 'fr')
post :create, params: { spree_user: { email: 'foobar@example.com', password: 'foobar123', password_confirmation: 'foobar123' }, locale: :fr}
expect(response).to redirect_to spree.account_path(locale: :fr)
end

it 'saves locale in user' do
post :create, params: { spree_user: { email: 'foobar@example.com', password: 'foobar123', password_confirmation: 'foobar123' }, locale: 'fr'}
post :create, params: { spree_user: { email: 'foobar@example.com', password: 'foobar123', password_confirmation: 'foobar123' }, locale: :fr}
user = Spree.user_class.find_by_email('foobar@example.com')
expect(user.selected_locale).to eq('fr')
end
Expand Down Expand Up @@ -83,8 +83,6 @@
allow(Devise::Mapping).to receive(:find_scope!).and_return(:spree_user)
end

after { I18n.locale = :en }

it 'redirects to sign in after timeout' do
expect(controller.send(:after_inactive_sign_up_path_for, :user)).to eq(spree.login_path)
end
Expand Down
50 changes: 26 additions & 24 deletions spec/controllers/spree/user_sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

before { @request.env['devise.mapping'] = Devise.mappings[:spree_user] }

after { I18n.locale = :en }

context "#create" do
context "using correct login information" do
if Gem.loaded_specs['spree_core'].version >= Gem::Version.create('3.7.0')
Expand All @@ -14,7 +16,7 @@

it 'assigns orders with the correct token and no user present' do
order = create(:order, email: user.email, token: 'ABC', user_id: nil, created_by_id: nil)
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }

order.reload
expect(order.user_id).to eq user.id
Expand All @@ -23,7 +25,7 @@

it 'assigns orders with the correct token and no user or email present' do
order = create(:order, token: 'ABC', user_id: nil, created_by_id: nil)
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }

order.reload
expect(order.user_id).to eq user.id
Expand All @@ -34,7 +36,7 @@
order = create(:order, email: user.email, token: 'ABC',
user_id: nil, created_by_id: nil,
completed_at: 1.minute.ago)
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }

order.reload
expect(order.user_id).to be_nil
Expand All @@ -43,14 +45,14 @@

it 'does not assign orders with an existing user' do
order = create(:order, token: 'ABC', user_id: 200)
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }

expect(order.reload.user_id).to eq 200
end

it 'does not assign orders with a different token' do
order = create(:order, token: 'DEF', user_id: nil, created_by_id: nil)
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }

expect(order.reload.user_id).to be_nil
end
Expand All @@ -72,7 +74,7 @@
else
order = create(:order, email: user.email, guest_token: 'ABC', user_id: nil, created_by_id: nil)
end
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }

order.reload
expect(order.user_id).to eq user.id
Expand All @@ -85,7 +87,7 @@
else
order = create(:order, guest_token: 'ABC', user_id: nil, created_by_id: nil)
end
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }

order.reload
expect(order.user_id).to eq user.id
Expand All @@ -111,22 +113,22 @@

it 'does not assign orders with an existing user' do
if Spree.version.to_f > 3.6
order = create(:order, token: 'ABC', user_id: 200)
order = create(:order, token: 'ABC', user_id: 200)
else
order = create(:order, guest_token: 'ABC', user_id: 200)
order = create(:order, guest_token: 'ABC', user_id: 200)
end
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }

expect(order.reload.user_id).to eq 200
end

it 'does not assign orders with a different token' do
if Spree.version.to_f > 3.6
order = create(:order, token: 'DEF', user_id: nil, created_by_id: nil)
order = create(:order, token: 'DEF', user_id: nil, created_by_id: nil)
else
order = create(:order, guest_token: 'DEF', user_id: nil, created_by_id: nil)
order = create(:order, guest_token: 'DEF', user_id: nil, created_by_id: nil)
end
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }

expect(order.reload.user_id).to be_nil
end
Expand All @@ -144,17 +146,17 @@
else
order = create(:order, email: user.email, guest_token: 'ABC', user_id: nil, created_by_id: nil)
end
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }

order.reload
expect(order.user_id).to eq user.id
expect(order.created_by_id).to eq user.id
end
end
end

context "and html format is used" do
it "redirects to account path after signing in" do
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }
expect(response).to redirect_to spree.account_path
end

Expand All @@ -165,8 +167,8 @@

it 'redirects to localized account path after signing in' do
skip if Spree.version.to_f < 4.2
post :create, params: { spree_user: { email: user.email, password: 'secret' }, locale: 'fr' }
expect(response).to redirect_to spree.account_path(locale: 'fr')
post :create, params: { spree_user: { email: user.email, password: 'secret' }, locale: :fr }
expect(response).to redirect_to spree.account_path(locale: :fr)
end
end
end
Expand All @@ -185,7 +187,7 @@
context "using incorrect login information" do
context "and html format is used" do
it "renders new template again with errors" do
post :create, params: { spree_user: { email: user.email, password: 'wrong' }}
post :create, params: { spree_user: { email: user.email, password: 'wrong' } }
expect(response).to render_template('new')
expect(flash[:error]).to eq I18n.t(:'devise.failure.invalid')
end
Expand All @@ -207,15 +209,15 @@
end

it "redirects to login page after signing out with default locale" do
post :create, params: { spree_user: { email: user.email, password: 'secret' }}
post :create, params: { spree_user: { email: user.email, password: 'secret' } }
delete :destroy
expect(response).to redirect_to(spree.login_path)
end

it "persists fr locale when redirecting to login page after signing out" do
post :create, params: { spree_user: { email: user.email, password: 'secret' }, locale: 'fr' }
delete :destroy, params: { locale: 'fr' }
expect(response).to redirect_to spree.login_path(locale: 'fr')
post :create, params: { spree_user: { email: user.email, password: 'secret' }, locale: :fr }
delete :destroy, params: { locale: :fr }
expect(response).to redirect_to spree.login_path(locale: :fr)
end
end
end
end
10 changes: 6 additions & 4 deletions spec/controllers/spree/users_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
Spree::Store.default.update(default_locale: 'en', supported_locales: 'en,fr')
end

after { I18n.locale = :en }

context '#load_object' do
it 'redirects to signup path if user is not found' do
allow(controller).to receive(:spree_current_user) { nil }
Expand All @@ -18,8 +20,8 @@
context "non default locale" do
it 'redirects to signup path with non default locale if user is not found' do
allow(controller).to receive(:spree_current_user) { nil }
put :update, params: { user: { email: 'foobar@example.com' }, locale: 'fr' }
expect(response).to redirect_to spree.login_path(locale: 'fr')
put :update, params: { user: { email: 'foobar@example.com' }, locale: :fr }
expect(response).to redirect_to spree.login_path(locale: :fr)
end
end
end
Expand All @@ -46,14 +48,14 @@
end

context 'non default locale' do
before { put :update, params: { user: { email: 'mynew@email-address.com' }, locale: 'fr' } }
before { put :update, params: { user: { email: 'mynew@email-address.com' }, locale: :fr } }

it 'performs update of email' do
expect(assigns[:user].email).to eq 'mynew@email-address.com'
end

it 'persists locale when redirecting to account' do
expect(response).to redirect_to spree.account_path(locale: 'fr')
expect(response).to redirect_to spree.account_path(locale: :fr)
end
end

Expand Down
Loading