Opened 22 months ago
Closed 22 months ago
#34327 closed Bug (wontfix)
Test client session does not work as described when using signed cookie engine
Reported by: | Sergei | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | session signed_cookies |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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
Change History (4)
comment:1 by , 22 months ago
follow-up: 3 comment:2 by , 22 months ago
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
diff --git a/docs/topics/testing/tools.txt b/docs/topics/testing/tools.txt index 85652095a8..94d11197ca 100644
a b access these properties as part of a test condition. 688 688 689 689 To modify the session and then save it, it must be stored in a variable 690 690 first (because a new ``SessionStore`` is created every time this property 691 is accessed):: 691 is accessed). For example, if you use 692 ``'django.contrib.sessions.backends.db'`` engine (the default): 692 693 693 694 def test_something(self): 694 695 session = self.client.session
What do you think?
comment:3 by , 22 months ago
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
diff --git a/docs/topics/testing/tools.txt b/docs/topics/testing/tools.txt index 85652095a8..b3deb2b377 100644
a b access these properties as part of a test condition. 695 695 session['somekey'] = 'test' 696 696 session.save() 697 697 698 # if a particular session store changes session key on modification 699 # (like signed cookie store does), it's required to update the 700 # session key in cookies 701 from django.conf import settings 702 self.client.cookies[settings.SESSION_COOKIE_NAME] = session.session_key 703
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).
comment:4 by , 22 months ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
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.
Also, can I fix this and do a PR?
I think it's possible to make a test client API like this:
Old way from the docs would continue to work (if not signed cookies store).