Opened 6 years ago

Closed 2 years ago

#11475 closed Bug (duplicate)

test.Client.session.save() raises error for anonymous users

Reported by: egmanoj@… Owned by: nobody
Component: Testing framework Version: 1.1-beta
Severity: Normal Keywords:
Cc: egmanoj@…, nodet, bmihelac@…, kitsunde@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I am trying to save data into the session object for an anonymous user. While I can do this for "real" using request.session[key] = value I am not able to simulate this during testing (by calling test.Client.session.save()). A quick round of debugging revealed that the test client uses a regular dictionary object for storing session data in the case of anonymous users (as opposed to a SessionStore object for known/authenticated users). This causes an error when I try to call self.client.session.save() from the setUp() method of my test class before running a test case.

from django.test import Client, TestCase

class MyTestCase(TestCase):
    def setUp(self):
        self.client = Client()
        self.client.session['key'] = 'value'
        self.client.session.save() # AttributeError: 'dict' object has no attribute 'save'

    def test_foo(self):
        self.assertEqual(1, 1)

I have included django.contrib.sessions in my INSTALLED_APPS.

Change History (19)

comment:1 Changed 6 years ago by anonymous

  • Cc egmanoj@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by kragen@…

I'm just now running into the same thing. The simple fix, of just having Client._session return engine.SessionStore(None) in the case where the cookie isn't set, generates a new session (with a new session ID) every time you refer to self.client.session, so we probably need to make session a lazy attribute if we want Client to be useful for testing storage of data for anonymous sessions; but I think also we have to somehow get the cookie set properly; there's code to do that down in the middle of Client.login which could be factored out, and of course that's already very much duplicative with SessionMiddleware.process_response, which is the code path that normally runs to set the browser cookie for the session; so we could factor that out from two places and use it in a third. (But I guess it doesn't run in Client.get. Does Client.get bypass middleware?)

I don't know my way around the code very well, so it's possible that the anonymous user above and I are doing something dumb.

comment:3 Changed 6 years ago by kragen@…

...also, making Client.session a lazy property would solve a funny little interface bug with authenticated sessions in tests. (Is that one logged in Trac yet?)

comment:4 follow-up: Changed 6 years ago by kragen@…

Aha! Now I understand.

  1. Until you've gotten back a cookie from the middleware (which *does* run from Client.get) you don't have the session object. Instead you get the stupid useless empty dict.
  2. Obviously you don't get a cookie until after you make a request. (Unless you call Client.login.)
  3. But even if you make a request, Django won't set the cookie unless the view tries to store something in request.session.
  4. At that point self.client.session will give you a useful object, but it's not the same object the view will use, so if you want to communicate with the view, you have to call .save() on it.
  5. But you can't just say self.client.session['foo'] = 'bar'; self.client.session.save() because, as explained in my previous two comments, you get a separate session object for each time you say self.client.session. Instead you have to say s = self.client.session; s['foo'] = 'bar'; s.save(), and then everything will work.

So now I know. It's not a bug, really; it's just that Django-under-test behaves in a way that makes it error-prone to write unit tests for.

It might still be useful to factor out the code in Client.login that creates a session and saves the session cookie without making an HTTP request, for tests that want to set up test session data without having to go through whatever series of views are necessary for a real person to set that data.

Maybe this is some kind of a Django FAQ.

comment:5 in reply to: ↑ 4 Changed 6 years ago by Manoj Govidnan <egmanoj@…>

  1. But you can't just say self.client.session['foo'] = 'bar'; self.client.session.save() because, as explained in my previous two comments, you get a separate session object for each time you say self.client.session. Instead you have to say s = self.client.session; s['foo'] = 'bar'; s.save(), and then everything will work.


Can you please point where these changes should be made in the context of the sample test case given above? I tested after making the changes you suggested inside the setUp() method as well as the test_foo() method and it still did not work.

comment:6 follow-up: Changed 6 years ago by kragen@…

Manoj: let's say that you have some view at /blort that will store something into the session. Then I think you can do this (although I haven't tested this code):

class MyTestCase(TestCase):
    def setUp(self):
        # self.client = Client() (not necessary in modern Django)
        self.client.get('/blort') # after this, self.client.session is a real session
        s = self.client.session
        s['key'] = 'value'
        s.save()

Does that work for you?

comment:7 in reply to: ↑ 6 Changed 6 years ago by Manoj Govindan <egmanoj@…>

Replying to kragen@canonical.org:

Manoj: let's say that you have some view at /blort that will store something into the session. Then I think you can do this (although I haven't tested this code):

[snip]

Does that work for you?


I tested the code. It works like you said: *if* I call a view that saves something to the session from within my setUp.

That said, it does not solve my original problem. I don't want to call a view in the setUp method for this purpose alone.


So now I know. It's not a bug, really; it's just that Django-under-test behaves in a way that makes it error-prone to write unit tests for.


I think it *is* a bug in that the client is not simulating "real world" behavior.

comment:8 follow-up: Changed 6 years ago by simonredfern

I just came across the same problem. Feels like a Bug with a capital B to me. Can't get the workaround to work either.

comment:9 in reply to: ↑ 8 Changed 6 years ago by Manoj Govindan <egmanoj@…>

Replying to simonredfern:

I just came across the same problem. Feels like a Bug with a capital B to me. Can't get the workaround to work either.

I created a dummy, non-staff user and logged him in inside my setUp(). Not clean or ideal, but works for me as I don't have any code based on user identity (yet). I don't know your scenario but this might be worth a try.

comment:10 Changed 6 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

comment:11 Changed 5 years ago by nodet

  • Cc nodet added

comment:12 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:13 Changed 4 years ago by bmihelac

  • Cc bmihelac@… added
  • Easy pickings unset

comment:14 Changed 4 years ago by prestontimmons

Crosslinking to #10899 which would fix this issue. Also #15740, which is strictly the bug fix portion of #10899.

comment:15 Changed 4 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:16 Changed 3 years ago by kitsunde

  • Cc kitsunde@… added

comment:17 Changed 3 years ago by hongshuning

I think kragen is right. The web server will not create any session object unless a client try to visit a web page. In the origin test case's setUp() method, the client does not visit any web page, so no session object created by django. At that time, self.client.session["key"] is nonsense. So, if you want to use the session object, you must visit a web page at first, just like a login access or any others.

comment:18 Changed 2 years ago by EmilStenstrom

If I'm understanding this bug correctly, it asks for a way to create session data (which is essentially server-based) from the client side. This workflow becomes really strange, and as @hongshuning suggests, the best way to simulate this is to do requests just like a normal web server does.

I suggest this bug is WONTFIX:ed.

comment:19 Changed 2 years ago by deni

  • Resolution set to duplicate
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.
Back to Top