Opened 10 years ago

Closed 8 years ago

Last modified 8 years ago

#24423 closed Cleanup/optimization (fixed)

Combine i18n template tag tests

Reported by: Preston Timmons Owned by: Andy Craze
Component: Internationalization Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

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 Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted

Sounds good to me.

comment:2 by Tim Graham, 8 years ago

Easy pickings: set
Summary: Combine i18n template tag tests?Combine i18n template tag tests

comment:3 by Aleksandr Sobolev, 8 years ago

Owner: changed from nobody to Aleksandr Sobolev
Status: newassigned

comment:4 by Aleksandr Sobolev, 8 years ago

Triage Stage: AcceptedReady for checkin

Create pull request with three commits:
https://github.com/django/django/pull/7363

comment:5 by Tim Graham, 8 years ago

Has patch: set
Triage Stage: Ready for checkinAccepted

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:6 by Aleksandr Sobolev, 8 years ago

Sorry, my fault. Next time I'll avoid such mistakes.

comment:7 by Tim Graham, 8 years ago

Patch needs improvement: set

comment:8 by Aleksandr Sobolev, 8 years ago

Owner: Aleksandr Sobolev removed
Status: assignednew

comment:9 by Andy Craze, 8 years ago

Owner: set to Andy Craze
Status: newassigned

Added a PR with all the i18n template tags reorganized into syntax_tests/i18n:
https://github.com/django/django/pull/7724

comment:10 by Tim Graham, 8 years ago

Please uncheck "Patch needs improvement" on the ticket after updating the PR for the comments.

comment:11 by Andy Craze, 8 years ago

Patch needs improvement: unset

comment:12 by Tim Martin, 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 Tim Martin, 8 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:14 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 97c1931c:

Fixed #24423 -- Reorganized i18n tag tests.

comment:15 by Tim Graham <timograham@…>, 8 years ago

In a6b5321:

Refs #24423 -- Readded inadvertently deleted i18n tests.

Mistake in 97c1931c4f610e80053430d0297d51e1bed1e7ae.

comment:16 by Tim Graham <timograham@…>, 8 years ago

In 49de4f15:

[1.11.x] Refs #24423 -- Readded inadvertently deleted i18n tests.

Mistake in 97c1931c4f610e80053430d0297d51e1bed1e7ae.

Backport of 357a6428980961b2c5311eb75d16229c7fc0d982 from master

Note: See TracTickets for help on using tickets.
Back to Top