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

Enable branch coverage and add more tests to get 100% coverage #139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .simplecov
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
if ENV['COVERAGE']
SimpleCov.start do
enable_coverage :branch # available in ruby >= 2.5, already required by actionpack 6
add_filter '/spec/'
end
end
1 change: 1 addition & 0 deletions AUTHORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Redis Session Store authors
- Olek Poplavsky
- Tim Lossen
- Todd Bealmear
- Vasily Fedoseyev
- Aleksey Dashkevych
- Olle Jonsson
- Nicolas Rodriguez
Expand Down
2 changes: 2 additions & 0 deletions lib/redis-session-store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# the MemCacheStore code, simply dropping in Redis instead.
class RedisSessionStore < ActionDispatch::Session::AbstractSecureStore
VERSION = '0.11.5'.freeze
# :nocov:
# Rails 3.1 and beyond defines the constant elsewhere
unless defined?(ENV_SESSION_OPTIONS_KEY)
ENV_SESSION_OPTIONS_KEY = if Rack.release.split('.').first.to_i > 1
Expand All @@ -12,6 +13,7 @@ class RedisSessionStore < ActionDispatch::Session::AbstractSecureStore
Rack::Session::Abstract::ENV_SESSION_OPTIONS_KEY
end
end
# :nocov:

USE_INDIFFERENT_ACCESS = defined?(ActiveSupport).freeze
# ==== Options
Expand Down
82 changes: 76 additions & 6 deletions spec/redis_session_store_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -252,21 +252,54 @@

context 'when session id does not exist in redis' do
it 'returns false' do
expect(redis).to receive(:exists).with('foo').and_return(false)
expect(redis).to receive(:exists?).with('foo').and_return(false)
expect(store.send(:session_exists?, :env)).to eq(false)
end
end

context 'when session id exists in redis' do
it 'returns true' do
expect(redis).to receive(:exists).with('foo').and_return(true)
expect(redis).to receive(:exists?).with('foo').and_return(true)
expect(store.send(:session_exists?, :env)).to eq(true)
end
end

context 'when redis is down' do
it 'returns true (fallback to old behavior)' do
before do
allow(store).to receive(:redis).and_raise(Redis::CannotConnectError)
end

it 'returns true (fallback to old behavior)' do
expect(store.send(:session_exists?, :env)).to eq(true)
end

context 'when :on_redis_down re-raises' do
before { store.on_redis_down = ->(e, *) { raise e } }

it 'explodes' do
expect do
store.send(:session_exists?, :env)
end.to raise_error(Redis::CannotConnectError)
end
end
end

context 'when redis does suport exists?' do
it 'calls it' do
expect(redis).to receive(:exists?).and_return(false)
expect(redis).not_to receive(:exists)
store.send(:session_exists?, :env)
end
end

context 'when redis does not support #exists?' do
before do
allow(redis).to receive(:respond_to?).with(:exists?).and_return(false)
allow(redis).to receive(:exists?).and_raise('shouhld not be called')
end

it 'uses old method exist' do
expect(redis).to receive(:exists).with('foo').and_return(true)
expect(store.send(:session_exists?, :env)).to eq(true)
end
end
Expand Down Expand Up @@ -345,6 +378,15 @@
store.send(:destroy, env)
end

context 'when no cookie hash in request' do
let(:env) { {} }

it 'does nothing' do
allow(store).to receive(:redis).and_return(double('redis'))
store.send(:destroy, env)
end
end

context 'when redis is down' do
before do
allow(store).to receive(:redis).and_raise(Redis::CannotConnectError)
Expand Down Expand Up @@ -546,9 +588,10 @@ def self.dump(_value)
end

describe 'setting the session' do
let(:env) { { 'rack.session.options' => {} } }
let(:sid) { 1234 }

it 'allows changing the session' do
env = { 'rack.session.options' => {} }
sid = 1234
allow(store).to receive(:redis).and_return(Redis.new)
data1 = { 'foo' => 'bar' }
store.send(:set_session, env, sid, data1)
Expand All @@ -560,7 +603,6 @@ def self.dump(_value)

it 'allows changing the session when the session has an expiry' do
env = { 'rack.session.options' => { expire_after: 60 } }
sid = 1234
allow(store).to receive(:redis).and_return(Redis.new)
data1 = { 'foo' => 'bar' }
store.send(:set_session, env, sid, data1)
Expand All @@ -569,5 +611,33 @@ def self.dump(_value)
_, session = store.send(:get_session, env, sid)
expect(session).to eq(data2)
end

context 'when redis is down' do
let(:redis_exception_type) { Redis::CannotConnectError }

before { allow(store).to receive(:redis).and_raise(redis_exception_type) }

it 'returns false' do
expect(store.send(:set_session, env, sid, { 'foo' => 'bar' })).to eq false
end

it 'calls on_redis_down' do
store.on_redis_down = double('on_redis_down')
expect(store.on_redis_down).to receive(:call).with(
be_kind_of(redis_exception_type), env, sid
)
store.send(:set_session, env, sid, { 'foo' => 'bar' })
end

context 'when :on_redis_down re-raises' do
before { store.on_redis_down = ->(e, *) { raise e } }

it 'explodes' do
expect do
store.send(:set_session, env, sid, { 'foo' => 'bar' })
end.to raise_error(Redis::CannotConnectError)
end
end
end
end
end