get_template_from_string default arguments break assertTemplateUsed

[Ian Clelland]

django.template.loader.get_template_from_string only requires a single argument -- the template source string. The origin and name parameters are optional, and default to None. (This function is never called in the Django codebase with missing parameters, except in the test suite)

If the name parameter is left out, it is explicitly passed as None to the Template constructor, and so the new template receives a name of None, rather than the default "<Unknown Template>".

django.test.testcases.TransactionTestCase.assertTemplateUsed contains an assertion which joins the 'name' parameter of all of the templates in the request. If any templates used were constructed without a name, the None in the list will cause a TypeError before the assertion can even be tested.

This is the simplest project I can create which exemplifies this problem:

templatetest/urls.py:

from django.conf.urls import patterns, include, url

urlpatterns = patterns('',
    url(r'^test/$', 'templatetest.views.test_view', name='test'),
)

templatetest/views.py:

from django.template import loader, Context
from django.http import HttpResponse

def test_view(request):
    template = loader.get_template_from_string("This is a string-based template")
    return HttpResponse(template.render(Context({})))

templatetest/tests.py:

from django.test import TestCase

class TemplateFromStringTest(TestCase):
    def test_none_template_used(self):
        response = self.client.get('/test/')
        self.assertTemplateUsed(response, "base.html")

./manage.py test templatetest:

Creating test database for alias 'default'...
E
======================================================================
ERROR: test_template_used (templatetest.tests.TemplateFromStringTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/ian/Code/Tests/django-template-test/templatetest/templatetest/tests.py", line 6, in test_template_used
    self.assertTemplateUsed(response, "base.html")
  File "/Users/ian/.virtualenvs/django-trunk/lib/python2.7/site-packages/django/test/testcases.py", line 629, in assertTemplateUsed
    (template_name, u', '.join(template_names)))
TypeError: sequence item 0: expected string or Unicode, NoneType found

----------------------------------------------------------------------
Ran 1 test in 0.133s

FAILED (errors=1)
Destroying test database for alias 'default'...

I believe that either assertTemplateUsed needs to guard against None values appearing in the list of template names, or get_template_from_string needs to be modified to callTemplate.__init__ in a way which uses *that* class's default parameters, or possibly both. I'll work up a quick patch once I get a feel for the right place to fix this.

[Claude Paroz]

I would be tempted to fix it this way in assertTemplateUsed:

  -      template_names = [t.name for t in response.templates]
  +      template_names = [t.name for t in response.templates if t.name]
         if not template_names:
  -         self.fail(msg_prefix + "No templates used to render the response")
  +         self.fail(msg_prefix + "No named templates used to render the response")

[Ian Clelland]

Claude, that's in fact how I've handled it locally, in a TransactionTestCase subclass -- my only reservation is that this still leaves a difference between the objects returned from

t = loader.get_template_from_string("template source")

and

t = Template("template source")

So I'm torn between considering this a bug in the testing framework and a bug in the template system.

[Claude Paroz]

I see your point. It's about the semantic of the name property of a template created from a direct string parameter, None or '<Unknown Template>'. I would vote for replacing '<Unknown Template>' by None in the Template __init__ which seems a slightly more pythonic way of stating that a property is unset. I've run the entire test suite with this change without failure.

FWIW, I traced the adding of this code: changeset:3707.

[Unai Zalakain]

PR sent: ​https://github.com/django/django/pull/1867

Do I have to note this change in the release notes? It's kind of insignificant.

[Claude Paroz <claude@…>]

In 72f63bd24d44b5a4f24ad8fa27ebba96d9a507d8:

Fixed #17529 -- get_template_from_string default arguments break

get_template_from_string default arguments were breaking
assertTemplateUsed. The solution has been to return only the names of the
templates with a name attribute distinct of None. The default name
kwarg of Template has been changed to None, more pythonic than '<Unknown
Template>'.

[Unai Zalakain]

Great, thanks!