1.1 feature: unify access to response.context in test client
James Bennett
response, so that unit tests can inspect that Context and verify that
it contained what it was expected to contain. This is all well and
good, except that there is no consistent way to write tests which do
this.
When an inheritance chain of multiple templates is used to render the
response, ``response.context`` is a list of dictionaries,
corresponding to the Context at each template and so, for example, one
might want to check ``response.context[0]['some_var']``. But when
inheritance is not used, ``response.context`` is simply a dictionary,
leading to a check lik ``response.context['some_var']``.
This makes it extremely difficult/tedious to write truly portable unit
tests, since a test which passes on one installation of an application
might fail on another for no other reason than that the type of
``request.context`` has changed from dictionary to list, or
vice-versa, raising spurious ``TypeError`` or ``KeyError`` depending
on which way it changed.
For a real-world example, consider django-registration: I have a local
project set up to run its unit tests, with minimal (non-inheriting)
templates; the test suite accesses ``request.context``
dictionary-style, and passes. But a user of django-registration
attempted to run the test suite on an installation which uses
inheritance, and saw multiple test failures as a result:
http://www.bitbucket.org/ubernostrum/django-registration/issue/3/failed-in-test
I believe quite strongly that unit tests, if they are to be useful,
need to be portable. And currently, it seems the only way to make them
portable is to include type checks on response.context each time a
test will inspect the context, e.g.,::
if isinstance(response.context, list):
self.assertEqual(response.context[0]['foo'], 'bar')
else:
self.assertEqual(response.context['foo'], 'bar')
This is painful and ugly.
For 1.1, could we look into unifying the interface to
``response.context`` to avoid this sort of problem? Unless I'm
thinking about this the wrong way, it shouldn't be too hard to
differentiate dictionary-style access from list-style access, since
the former -- in the case of a Context -- will always be using string
keys and the latter will always be using integer indexes.
--
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."
Eric Holscher
I did something like this:
con = context.dicts[0]
if 'MEDIA_URL' in con:
con = context.dicts[-1]
Obviously a hack, but it seems to work most of the time.
For my pony request, it would be really nice to have a way to get "user defined" context. This being things that were passed from views, set in template tags, (and maybe other places?). That is what the above code is trying to do. I haven't thought about how to do it, but I agree with James that some thought needs to be placed into this.
A simple workaround might be to flatten the lists in request.context, but then keys in the dictionaries might be overwritten.
Eric Holscher
Web Developer at The World Company in Lawrence, Ks
http://www.ericholscher.com
er...@ericholscher.com
Russell Keith-Magee
>
> For 1.1, could we look into unifying the interface to
> ``response.context`` to avoid this sort of problem? Unless I'm
> thinking about this the wrong way, it shouldn't be too hard to
> differentiate dictionary-style access from list-style access, since
> the former -- in the case of a Context -- will always be using string
> keys and the latter will always be using integer indexes.
No objections to looking at this for v1.1. It's certainly a wart that
needs some attention.
However, to clarify - are you talking about a backwards incompatible
change, or are you talking about putting a backwards compatible layer
in place that tries to tell the difference between the two modes of
access?
The backwards incompatible change is actually trivial - all that is
required is to remove the gymnastics that the test code does to turn a
list of one context entry into a single standalone entry. This is one
of those things that seemed like a really good idea at the time, but
which grew old really quickly. I'm very conscious of maintaining
backwards compatibility, but this one would be high on the list of
things that, with hindsight, I wouldn't mind changing.
I can see a number of possible interpretations for the backwards
compatible access layer:
- Only look in context [0], but as Eric notes, the value you seek
won't always be in the first context.
- Iterate through all the contexts looking for the first match, and return it
- Iterate through all the contexts looking for any matches, and only
return if the match is unique
Another option would be to duplicate the functionality - maintain
request.context as-is, but add a new request.full_context (or some
other bikeshed) which preserves the list-nature of the contexts. This
does introduce some functional duplication in the API, but it would be
backwards compatible.
Yours,
Russ Magee %-)
Russell Keith-Magee
> For my pony request, it would be really nice to have a way to get "user
> defined" context. This being things that were passed from views, set in
> template tags, (and maybe other places?). That is what the above code is
> trying to do. I haven't thought about how to do it, but I agree with James
> that some thought needs to be placed into this.
>
> A simple workaround might be to flatten the lists in request.context, but
> then keys in the dictionaries might be overwritten.
This has some overlap with ticket #5333, which describes a request for
assertContext; the ticket discussion describes why that idea isn't
necessarily what we want, but the broader idea is something worth
addressing. It is possible to use contexts in templates, but it
certainly isn't easy. Finding the best way to wrap an API around
contexts to make them easily testable is definitely worth doing.
Yours,
Russ Magee %-)
James Bennett
<freakb...@gmail.com> wrote:
> However, to clarify - are you talking about a backwards incompatible
> change, or are you talking about putting a backwards compatible layer
> in place that tries to tell the difference between the two modes of
> access?
I'd prefer backwards compatibility. The way I'm envisioning it would
complicate the code a bit, but I think preserving compatibility is
worth it:
1. response.context simply stuffs away a complete copy of the final
Context used in rendering, as well as the current behavior of
maintaining a list of contexts.
2. A call to response.context.__getitem__() with a string argument
goes straight into that Context; thanks to Context's own fall-through
semantics, this will find a key (if it's there to be found) in
whatever layer of the context stack it happens to be in.
3. A call to response.context.__getitem__() with an integer argument
'n' returns Context 'n' out of the list.
This maintains backward compatibility for people doing things like
"response.context[0]['foo']", and maybe we can toss in a
DeprecationWarning and eventually get rid of that behavior, but more
importantly it makes "response.context['foo']" always work.
oggie rob
> complicate the code a bit, but I think preserving compatibility is
> worth it:
the quickest way to test your code against a django upgrade is to run
tests - it would be ideal to migrate to 1.1 without having to refactor
any tests just to get there.
OTOH, I don't think it would be a huge deal to not preserve backwards
compat - in the worst case scenario, users can simply comment out the
offending code temporarily, and although backwards compat is
emphasized from 1.0 on I think there would be some leeway with a
fairly specific test feature. The worst situation would be buggy test
code, so if it turns out to be very complicated to maintain backwards
compat, we should leave it behind.
-rob
Gábor Farkas
>
> On Sat, Nov 8, 2008 at 5:34 PM, Russell Keith-Magee
> <freakb...@gmail.com> wrote:
>> However, to clarify - are you talking about a backwards incompatible
>> change, or are you talking about putting a backwards compatible layer
>> in place that tries to tell the difference between the two modes of
>> access?
>
> I'd prefer backwards compatibility. The way I'm envisioning it would
> complicate the code a bit, but I think preserving compatibility is
> worth it:
>
> 1. response.context simply stuffs away a complete copy of the final
> Context used in rendering, as well as the current behavior of
> maintaining a list of contexts.
> 2. A call to response.context.__getitem__() with a string argument
> goes straight into that Context; thanks to Context's own fall-through
> semantics, this will find a key (if it's there to be found) in
> whatever layer of the context stack it happens to be in.
> 3. A call to response.context.__getitem__() with an integer argument
> 'n' returns Context 'n' out of the list.
does this mechanism work with contexts with integer-keys?
gabor
James Bennett
> does this mechanism work with contexts with integer-keys?
A context variable can't really be an integer, so far as I can tell...
Julien Phalip
> The Djangotestclient exposes theContextused to render the returned
> response, so that unit tests can inspect thatContextand verify that
> good, except that there is no consistent way to write tests which do
> this.
>
> When an inheritance chain of multiple templates is used to render the
> response, ``response.context`` is alistof dictionaries,
> inheritance is not used, ``response.context`` is simply a dictionary,
> leading to a check lik ``response.context['some_var']``.
>
> This makes it extremely difficult/tedious to write truly portable unit
> on which way it changed.
>
> For a real-world example, consider django-registration: I have a local
> project set up to run its unit tests, with minimal (non-inheriting)
>
> I believe quite strongly that unit tests, if they are to be useful,
> need to be portable. And currently, it seems the only way to make them
> portable is to include type checks on response.contexteach time atestwill inspect thecontext, e.g.,::
>
> if isinstance(response.context,list):
> self.assertEqual(response.context[0]['foo'], 'bar')
> else:
> self.assertEqual(response.context['foo'], 'bar')
>
> This is painful and ugly.
>
> For 1.1, could we look into unifying the interface to
> ``response.context`` to avoid this sort of problem? Unless I'm
> thinking about this the wrong way, it shouldn't be too hard to
> the former -- in the case of aContext-- will always be using string
I could not find any mention of this in the Version 1.1 features or
roadmap. I guess it'll have to wait till next release. Could you
please confirm?
Thanks,
Julien
Jacob Kaplan-Moss
> I could not find any mention of this in the Version 1.1 features or
> roadmap. I guess it'll have to wait till next release. Could you
> please confirm?
It's a pretty small change with no real backwards-incompatibility
implications. The list isn't and doesn't need to be the be-all and
end-all of features for 1.1. Anything that gets done by feature freeze
is a candidate; the list exists to help us focus efforts.
Jacob