Rewrite template_tag.tests.TemplateTests as individual test cases

[Preston Timmons]

There are some downsides to the current format of the template tests:

• It's impossible to use --failfast
• All errors are lumped into a single failure. When working on the template code, a small problem can result in hundreds of errors worth of output.
• When there is an error, the output isn't helpful since the test cases themselves are difficult to read.

I think it would be an improvement if each template tag had it's own test case with each line as a separate test method.

[Yamila]

Agree. May be labourious as there are lots of tests, but tests should be smaller and isolated.

[Yamila]

I've introduced an error in the test, and run the tests like follows:

PYTHONPATH=..:$PYTHONPATH ./runtests.py --settings=test_sqlite template_tests --failfast

and it works properly.

But anyway, the errors are shown dirtier than expected.

[Preston Timmons]

If you introduce multiple errors to TemplateTests and run ./runtests.py --settings=test_sqlite template_tests.tests.TemplateTests --failfast, it will not end on the first failure since all the tests are treated as a single test method.

[Preston Timmons]

I've added a proof-of-concept for this here:

​https://github.com/prestontimmons/django/compare/ticket-23768

This is a rewrite of most of the "basic-syntax-*" tests into a BasicSyntaxTests class.

It maintains the current behavior of the test runner, which runs each test 6 times with combinations of TEMPLATE_DEBUG, TEMPLATE_STRING_IF_INVALID, and the cached loader, with the following advantages:

• Individual tests can be run easily, like ./runtests.py --settings=test_sqlite template_tests.test_clean.BasicSyntaxTests.test_text.
• The --failfast option works correctly. ./runtests.py --settings=test_sqlite template_tests.test_clean.BasicSyntaxTests --failfast.
• Readability is improved.

Is this approach acceptable? Porting existings tests over to it is easy.

[Preston Timmons]

I added a pull request here:

​https://github.com/django/django/pull/3631

[Aymeric Augustin]

The tests are still hard to figure out because there's a lot of code factorization but I think it's an improvement.

Would you like me to merge the pull request now? Or do you want to port the remainder of the tests first?

[Preston Timmons]

I can work on the remainder of the tests. I'll post an update when I'm done.

[Preston Timmons]

I updated my pull request to include the remainder of the template tests. I'm going to do a final check through everything before unchecking patch needs improvement.

For ease of auditing, I kept all the test methods named the same as before, i.e. include01, include02, etc. I think it would be good to add meaningful names in a separate patch.

The new suite adds 640 tests, including the additional test for the setup function. The previous suite had 641 tests, two of which weren't really tests themselves: include 05 and include-cycle. I verified this with this script:

​https://gist.github.com/prestontimmons/01ed288aaa2083afa2d4

I didn't update the filter tests yet. I think these should also be done in a separate patch.

I have a couple specific questions:

1) There are two tests that seem to have wrong comments:

• filter-syntax25
• if-tag-shortcircuit02

In both cases, the tests do the opposite of what the comment says. For filter-syntax25 I believe the comment is wrong. For if-tag-shortcircuit02, I think the test is wrong.

Does anyone else agree?

2) I added a warnings filter for the url tag tests to the setup function. Is this done right? I'm not too familiar with how warnings work.

[Berker Peksag]

2) I added a warnings filter for the url tag tests to the setup function. Is this done right? I'm not too familiar with how warnings work.

Code LGTM. Just one nitpick: You could use warnings.simplefilter("ignore", ...) if you don't planning to pass a message parameter to warnings.filterwarnings.

[Berker Peksag]

For filter-syntax25 I believe the comment is wrong.

+1.

For if-tag-shortcircuit02, I think the test is wrong.

I think the comment is a bit misleading here.

The original comment was:

When the if clause is shortcircuiting correctly, the is_bad() function shouldn't be evaluated,

Copied from dc3b2a0fdf2a69864c5508a9c49aca43acac9bb2.

[Aymeric Augustin]

I'll take a look at your questions and merge the patch as soon as possible. I don't want it to become stale.

Please add yourself in the AUTHORS file (in alphabetical order) if you aren't already there.

Only dealing with tags is good for now. The patch is large enough already.

Thank you very much.

[Preston Timmons]

The patch has been updated with some good feedback by Tim Graham. I think it's good to go now.

Some things to do after merge are:

• Add meaningful names to test methods
• Update filter tests
• Decide if each test needs to run with TEMPLATE_DEBUG and TEMPLATE_STRING_IF_INVALID. Tim mentioned it may be more appropriate to just add separate test cases where these are used.

[Tim Graham <timograham@…>]

In b872134bfc14f6322bd1e4b0a08bf5bfd2c43a52:

Fixed #23768 -- Rewrote template tests as unit tests.

[Berker Peksag]

Hi,

I'm trying to review ​https://github.com/django/django/pull/3660 and I have a question about this patch:

Is there a reason why setup invokes a test method twice?

​https://github.com/django/django/blob/master/tests/template_tests/syntax_tests/utils.py#L58

This will make harder to test deprecation warnings correctly.

[Berker Peksag]

I've found a bug in the setup decorator.

$ ./tests/runtests.py template_tests.syntax_tests.test_if.IfTagTests.test_if_tag_eq06
Testing against Django installed in '/home/berker/projects/django/django'
Traceback (most recent call last):
  File "./tests/runtests.py", line 410, in <module>
    options.reverse, options.modules)
  File "./tests/runtests.py", line 236, in django_tests
    test_labels or get_installed(), extra_tests=extra_tests)
  File "/home/berker/projects/django/django/test/runner.py", line 154, in run_tests
    suite = self.build_suite(test_labels, extra_tests)
  File "/home/berker/projects/django/django/test/runner.py", line 74, in build_suite
    tests = self.test_loader.loadTestsFromName(label)
  File "/usr/lib/python2.7/unittest/loader.py", line 109, in loadTestsFromName
    return self.suiteClass([parent(obj.__name__)])
  File "/usr/lib/python2.7/unittest/case.py", line 191, in __init__
    (self.__class__, methodName))
ValueError: no such test method in <class 'template_tests.syntax_tests.test_if.IfTagTests'>: inner

Here is a PR to fix this: ​https://github.com/django/django/pull/3671

[Baptiste Mispelon <bmispelon@…>]

In adacbd64a062662f54d6e91dc4e460eff96b5dd5:

Fixed "no such test method" error in template_tests.

Without this patch, you couldn't run an individual test
case in template_tests.

Refs #23768

[Tim Graham <timograham@…>]

In 16f26defa7510707742a15aa89cae56f11d14c3f:

Converted recently refactored templates tests to SimpleTestCase.

These test methods don't need DB setup/teardown.

Refs #23768 and b872134b.