Code

Opened 2 years ago

Closed 2 years ago

#18103 closed Bug (fixed)

Change to _reset_dicts causes wholesale failure

Reported by: vsajip Owned by: aaugustin
Component: Template system Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by lrekucki)

On updating to r17895, lots of tests appear to have stopped working. Here's the tail end of the test run:

======================================================================
ERROR: test_user_permission_performance
(regressiontests.admin_views.tests.UserAdminTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vinay/projects/django/tests/regressiontests/admin_views/
tests.py", line 3244, in test_user_permission_performance
    response = self.client.get('/test_admin/admin/auth/user/%s/' %
u.pk)
  File "/home/vinay/projects/django/django/test/client.py", line 427,
in get
    response = super(Client, self).get(path, data=data, **extra)
  File "/home/vinay/projects/django/django/test/client.py", line 243,
in get
    return self.request(**r)
  File "/home/vinay/projects/django/django/core/handlers/base.py",
line 136, in get_response
    response = response.render()
  File "/home/vinay/projects/django/django/template/response.py", line
104, in render
    self._set_content(self.rendered_content)
  File "/home/vinay/projects/django/django/template/response.py", line
81, in rendered_content
    content = template.render(context)
  File "/home/vinay/projects/django/django/template/base.py", line
140, in render
    return self._render(context)
  File "/home/vinay/projects/django/django/test/utils.py", line 60, in
instrumented_test_render
    return self.nodelist.render(context)
  File "/home/vinay/projects/django/django/template/base.py", line
823, in render
    bit = self.render_node(node, context)
  File "/home/vinay/projects/django/django/template/base.py", line
837, in render_node
    return node.render(context)
  File "/home/vinay/projects/django/django/template/loader_tags.py",
line 123, in render
    return compiled_parent._render(context)
  File "/home/vinay/projects/django/django/test/utils.py", line 60, in
instrumented_test_render
    return self.nodelist.render(context)
  File "/home/vinay/projects/django/django/template/base.py", line
823, in render
    bit = self.render_node(node, context)
  File "/home/vinay/projects/django/django/template/base.py", line
837, in render_node
    return node.render(context)
  File "/home/vinay/projects/django/django/template/loader_tags.py",
line 123, in render
    return compiled_parent._render(context)
  File "/home/vinay/projects/django/django/test/utils.py", line 60, in
instrumented_test_render
    return self.nodelist.render(context)
  File "/home/vinay/projects/django/django/template/base.py", line
823, in render
    bit = self.render_node(node, context)
  File "/home/vinay/projects/django/django/template/base.py", line
837, in render_node
    return node.render(context)
  File "/home/vinay/projects/django/django/template/loader_tags.py",
line 62, in render
    result = block.nodelist.render(context)
  File "/home/vinay/projects/django/django/template/base.py", line
823, in render
    bit = self.render_node(node, context)
  File "/home/vinay/projects/django/django/template/base.py", line
837, in render_node
    return node.render(context)
  File "/home/vinay/projects/django/django/template/base.py", line
1193, in render
    'use_tz': context.use_tz,
  File "/home/vinay/projects/django/django/template/context.py", line
96, in __init__
    super(Context, self).__init__(dict_)
  File "/home/vinay/projects/django/django/template/context.py", line
18, in __init__
    self._reset_dicts(dict_)
  File "/home/vinay/projects/django/django/template/context.py", line
23, in _reset_dicts
    builtins.update(value)
ValueError: dictionary update sequence element #0 has length 1; 2 is
required

----------------------------------------------------------------------
Ran 4698 tests in 956.978s

FAILED (errors=166, skipped=109, expected failures=2)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...
vinay@eta-oneiric64:~/projects/django/tests$

All the errors appear to have the same root cause. I'm testing with
Python 2.7.2+ (default, Oct 4 2011, 20:06:09) on Ubuntu Oneiric 64-
bit. The tests were run using

PYTHONPATH=.. python runtests.py --settings test_sqlite

in the tests subdirectory.

The errors seem to be related to Aymeric's change in r17894. If I change

    def _reset_dicts(self, value=None):
        builtins = {'True': True, 'False': False, 'None': None}
        if value:
            builtins.update(value)
        self.dicts = [builtins]

to the seemingly equivalent

    def _reset_dicts(self, value=None):
        value = copy(value or {})
        value.update({'True': True, 'False': False, 'None': None})
        self.dicts = [value]

then the errors no longer occur.

Attachments (2)

context.diff (754 bytes) - added by vsajip 2 years ago.
Suggested change to django.template.context.BaseContext._reset_dicts
18103-2.diff (7.9 KB) - added by claudep 2 years ago.
Prevent passing a Context to another Context init

Download all attachments as: .zip

Change History (6)

Changed 2 years ago by vsajip

Suggested change to django.template.context.BaseContext._reset_dicts

comment:1 Changed 2 years ago by lrekucki

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Fixed formatting for you ;)

comment:2 Changed 2 years ago by aaugustin

  • Owner changed from nobody to aaugustin
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 2 years ago by claudep

It might fix the test suite, but is it the right fix? With your solution, when a Context is initialized with another Context param, the resulting structure in context.dicts is a list of lists of dicts, instead of a list of dicts. I admit I'm not familiar enough with Context handling to know whether it is damageable or not, but another approach would be to prevent passing a Context instance to the Context __init__. This is my approach with the following patch.

Changed 2 years ago by claudep

Prevent passing a Context to another Context init

comment:4 Changed 2 years ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to closed

In [17896]:

Fixed #18103 -- Regression introduced in r17895.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.