#24423 closed Cleanup/optimization (fixed)
Combine i18n template tag tests
Description ¶
I was working on #25372 and noticed there isn't an obvious place to add extra tests. This is because the i18n
tag tests are currently split among i18n
and template_tests.syntax_tests.test_i18n
. I think these tests should be combined in one place or the other.
I propose the following:
1) Move the tag-related tests from i18n
into template_tests
2) Split template_tests.syntax_tests.test_i18n
into separate files for each tag (test_trans.py, test_blocktrans.py, etc.)
I think change 1 makes sense because template_tests
is also where we have tests for things like staticfiles, cache, etc. The timezone tags are the only outlier here.
Change 2 is a pattern we adopted when converting the filter tests to unit tests. I think it would be good for template tag tests to follow the same pattern.
Does this seem like the right grouping?
Change History (16)
comment:1 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 8 years ago
Easy pickings: | set |
---|---|
Summary: | Combine i18n template tag tests? → Combine i18n template tag tests |
comment:3 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 8 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Create pull request with three commits:
https://github.com/django/django/pull/7363
comment:5 by , 8 years ago
Has patch: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
Thanks for the patch. The correct triage action is to check "Has patch"' when adding a pull request. "Ready for checkin" is set by the person who reviews the patch.
comment:7 by , 8 years ago
Patch needs improvement: | set |
---|
comment:8 by , 8 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:9 by , 8 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
Added a PR with all the i18n template tags reorganized into syntax_tests/i18n:
https://github.com/django/django/pull/7724
comment:10 by , 8 years ago
Please uncheck "Patch needs improvement" on the ticket after updating the PR for the comments.
comment:11 by , 8 years ago
Patch needs improvement: | unset |
---|
comment:12 by , 8 years ago
Patch needs improvement: | set |
---|
The patch looks good to me, but IIUC you need to squash it into a single commit with a message following the correct commit message format.
comment:13 by , 8 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Sounds good to me.