Test client session does not work as described when using signed cookie engine

[Sergei]

The following snippet from ​the documentation does not work when using django.contrib.sessions.backends.signed_cookies:

def test_something(self):
    session = self.client.session
    session['somekey'] = 'test'
    session.save()

session.save() is not enough in this case: session key is session data itself and test client cookies need to be updated.
Like session getter does when creating session store object (django/test/client.py:732):

self.cookies[settings.SESSION_COOKIE_NAME] = session.session_key

[Sergei]

Also, can I fix this and do a PR?

I think it's possible to make a test client API like this:

# introduce setter for session
# short-circuit if passing SessionBase type
client.session = session_store

# if not SessionBase, then assume it's a map type -> .items()
# completely replace session data
client.session = {'foo': 'bar'}

# update
session = client.session
session.update({'foo': 'bar'})
client.session = session

Old way from the docs would continue to work (if not signed cookies store).

[Mariusz Felisiak]

Replying to Sergei:

The following snippet from ​the documentation does not work when using django.contrib.sessions.backends.signed_cookies:

This is not an universal snippet intended to work with all kind of built-in and 3-party session engines. It's only an example of how the session can be modified. We could clarify this with:

docs/topics/testing/tools.txt

@@ -688 +688 @@
 
     To modify the session and then save it, it must be stored in a variable
     first (because a new ``SessionStore`` is created every time this property
-    is accessed)::
+    is accessed). For example, if you use
+    ``'django.contrib.sessions.backends.db'`` engine (the default):
 
         def test_something(self):
             session = self.client.session

What do you think?

[Sergei]

Replying to Mariusz Felisiak:

What do you think?

Thanks for the suggestion! I guess it could be solved with documentation. Then I think there is a need to explain what's going on. Maybe like this?

docs/topics/testing/tools.txt

@@ -695 +695 @@
             session['somekey'] = 'test'
             session.save()
 
+            # if a particular session store changes session key on modification
+            # (like signed cookie store does), it's required to update the
+            # session key in cookies
+            from django.conf import settings
+            self.client.cookies[settings.SESSION_COOKIE_NAME] = session.session_key
+

But I still think it's too much details spilling into a test, could be handled internally in the ClientMixin, independent of a concrete store implementation (it already does everything actually).

[Carlton Gibson]

Hi Sergei.

I'm going to mark this as wontfix, as I'm not sure it's worth even the clarification Mariusz offered. (For me, it clearly reads as assume the default backend — it's an example...) If you want to make that tweak, I don't object though.

...could be handled internally in the ClientMixin, independent of a concrete store implementation (it already does everything actually).

I don't see from what's written here exactly what you have in mind. If you wanted to make a draft PR showing the API changes then we could see if it looks worthwhile.
(If folks think so, then we can reopen here.)

I hope that makes sense. Thanks.