Tests for session handling #7568

Merged
merged 2 commits into from Sep 8, 2012

3 participants

@alup

These tests are some quick additions to increase coverage for actiondispatch/request/session.rb and actionpack/test/dispatch/session/cookie_store_test.rb .

@carlosantoniodasilva carlosantoniodasilva and 1 other commented on an outdated diff Sep 7, 2012
actionpack/test/dispatch/session/cookie_store_test.rb
@@ -195,6 +201,21 @@ def test_setting_session_value_after_session_reset
end
end
+ def test_class_type_after_session_reset
+ with_test_route_set do
+ get '/set_session_value'
+ assert_response :success
+ session_payload = response.body
@carlosantoniodasilva
Ruby on Rails member
carlosantoniodasilva added a line comment Sep 7, 2012

Why do you need session_payload here, if you're using response.body in the line below?

@alup
alup added a line comment Sep 8, 2012

Sorry, :s misplacement. I have fixed it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosantoniodasilva carlosantoniodasilva and 1 other commented on an outdated diff Sep 7, 2012
actionpack/test/dispatch/request/session_test.rb
+ env = {}
+ s = Session.create(store, env, {})
+ s['rails'] = 'ftw'
+ s['adequate'] = 'awesome'
+ s.clear
+ assert_equal([], s.values)
+ end
+
+ def test_destroy_clears_session_hash
+ env = {}
+ s = Session.create(store, env, {})
+ s['rails'] = 'ftw'
+ s['adequate'] = 'awesome'
+ s.destroy
+ assert_equal([], s.values)
+ end
@carlosantoniodasilva
Ruby on Rails member
carlosantoniodasilva added a line comment Sep 7, 2012

It's not particularly clear to me the difference between these two tests =(. I'll have to check their implementation later.

@alup
alup added a line comment Sep 8, 2012

The first test refers to clear method of ActionDispatch::Request::Session and the second ensures that destroy method calls clear in its body. The second test needs more assertions to be correct, but I didn't find some quick way to mock the necessary stuff. We need to revise the mocks in this set of tests and cover all the defined methods.

For now I will drop it and I will try to refine all tests in in a second iteration.

@carlosantoniodasilva
Ruby on Rails member
carlosantoniodasilva added a line comment Sep 8, 2012

Alright, great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosantoniodasilva carlosantoniodasilva merged commit 68ca549 into rails:master Sep 8, 2012
@steveklabnik
Ruby on Rails member

Nice, thank you for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment