Opened 15 years ago

Closed 11 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 by anonymous, 15 years ago

Cc: egmanoj@… added

comment:2 by kragen@…, 15 years ago

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 by kragen@…, 15 years ago

...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 by kragen@…, 15 years ago

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.

in reply to:  4 comment:5 by Manoj Govidnan <egmanoj@…>, 15 years ago

  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 by kragen@…, 15 years ago

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?

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

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 by simonredfern, 15 years ago

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.

in reply to:  8 comment:9 by Manoj Govindan <egmanoj@…>, 15 years ago

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 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

comment:11 by nodet, 13 years ago

Cc: nodet added

comment:12 by Julien Phalip, 13 years ago

Severity: Normal
Type: Bug

comment:13 by bmihelac, 13 years ago

Cc: bmihelac@… added
Easy pickings: unset

comment:14 by Preston Timmons, 13 years ago

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

comment:15 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:16 by Kit Sunde, 11 years ago

Cc: kitsunde@… added

comment:17 by Hong Shuning, 11 years ago

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 by EmilStenstrom, 11 years ago

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 by Deni Bertovic, 11 years ago

Resolution: duplicate
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top