Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

DON'T MERGE -- Add further tests for #19949. #1511

Closed
wants to merge 1 commit into from

3 participants

@ramiro
Collaborator

No description provided.

@onceuponatimeforever onceuponatimeforever commented on the diff
tests/template_tests/test_loaders.py
((13 lines not shown))
self.assertRaises(TemplateDoesNotExist, template_loader.load_template, "missing.html")
+ # Verify that the fact that the template hasn't been found has actually
+ # been cached:
+ self.assertTrue(template_loader.is_cached_notfoud("missing.html"))

The unit test fails on L142. The quickfix for L142 is "self.assertTrue(TemplateDoesNotExist, template_loader.is_cached_notfoud("missing.html"))". Also, typo: "found" in comment.

@ramiro Collaborator
ramiro added a note

Yes, actually that failure was on purpose. It demonstrates we haven't fixed the bug and should help us to actually fix it.

It looks to me today that the is_cached_notfoud() method I added is overnegineered. Maybe we can simply remove it and replace the test with

self.assertEqual(template_loader.template_cache["missing.html"], TemplateDoesNotExist)

The intention was to avoid accessing the guts (template_cache) of the loader, but we are doing it above already.

I added a 1-line fix in template/loaders/cached.py that pass all tests (and when removed, that last test fails).
Here it is:

L59 of cached.py:
self.template_cache[key] = TemplateDoesNotExist

See my PR on your PR: ramiro#1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@onceuponatimeforever onceuponatimeforever commented on the diff
django/template/loaders/cached.py
@@ -79,3 +82,17 @@ def reset(self):
"Empty the template cache."
self.template_cache.clear()
self.find_template_cache.clear()
+
+ def inspect_cache(self, template_name, template_dirs=None):
+ """
+ Return the value stored in the cache for template_name.
+ Test instrumentation method.
+ """
+ key = self.cache_key(template_name, template_dirs)
+ return self.template_cache.get(key)
+
+ def is_cached_notfoud(self, template_name, template_dirs=None):

Typo: "found".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham
Owner

merged in b785a80

Thanks for catching this.

@timgraham timgraham closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 25, 2013
  1. @ramiro

    Add further tests for #19949.

    ramiro authored
This page is out of date. Refresh to see the latest.
View
19 django/template/loaders/cached.py
@@ -37,6 +37,9 @@ def cache_key(self, template_name, template_dirs):
return template_name
def find_template(self, name, dirs=None):
+ """
+ Helper method. Lookup the template :param name: in all the configured loaders
+ """
key = self.cache_key(name, dirs)
try:
result = self.find_template_cache[key]
@@ -58,7 +61,7 @@ def find_template(self, name, dirs=None):
def load_template(self, template_name, template_dirs=None):
key = self.cache_key(template_name, template_dirs)
template_tuple = self.template_cache.get(key)
- # cached a previous failure:
+ # A cached previous failure:
if template_tuple is TemplateDoesNotExist:
raise TemplateDoesNotExist
elif template_tuple is None:
@@ -79,3 +82,17 @@ def reset(self):
"Empty the template cache."
self.template_cache.clear()
self.find_template_cache.clear()
+
+ def inspect_cache(self, template_name, template_dirs=None):
+ """
+ Return the value stored in the cache for template_name.
+ Test instrumentation method.
+ """
+ key = self.cache_key(template_name, template_dirs)
+ return self.template_cache.get(key)
+
+ def is_cached_notfoud(self, template_name, template_dirs=None):

Typo: "found".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ """
+ Test instrumentation method.
+ """
+ return self.inspect_cache(template_name, template_dirs) == TemplateDoesNotExist
View
8 tests/template_tests/test_loaders.py
@@ -129,13 +129,17 @@ def test_templatedir_caching(self):
self.assertNotEqual(t1.render(Context({})), t2.render(Context({})))
def test_missing_template_is_cached(self):
- "Check that the missing template is cached."
+ "#19949 -- Check that the missing template is cached."
template_loader = loader.find_template_loader(settings.TEMPLATE_LOADERS[0])
# Empty cache, which may be filled from previous tests.
template_loader.reset()
- # Check that 'missing.html' isn't already in cache before 'missing.html' is loaed
+ # Check that 'missing.html' isn't already in cache before 'missing.html' is loaded
self.assertRaises(KeyError, lambda: template_loader.template_cache["missing.html"])
+ # Try to load it, it should fail
self.assertRaises(TemplateDoesNotExist, template_loader.load_template, "missing.html")
+ # Verify that the fact that the template hasn't been found has actually
+ # been cached:
+ self.assertTrue(template_loader.is_cached_notfoud("missing.html"))

The unit test fails on L142. The quickfix for L142 is "self.assertTrue(TemplateDoesNotExist, template_loader.is_cached_notfoud("missing.html"))". Also, typo: "found" in comment.

@ramiro Collaborator
ramiro added a note

Yes, actually that failure was on purpose. It demonstrates we haven't fixed the bug and should help us to actually fix it.

It looks to me today that the is_cached_notfoud() method I added is overnegineered. Maybe we can simply remove it and replace the test with

self.assertEqual(template_loader.template_cache["missing.html"], TemplateDoesNotExist)

The intention was to avoid accessing the guts (template_cache) of the loader, but we are doing it above already.

I added a 1-line fix in template/loaders/cached.py that pass all tests (and when removed, that last test fails).
Here it is:

L59 of cached.py:
self.template_cache[key] = TemplateDoesNotExist

See my PR on your PR: ramiro#1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
class RenderToStringTest(unittest.TestCase):
Something went wrong with that request. Please try again.