Opened 8 years ago

Closed 6 years ago

Last modified 5 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 Changed 8 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

Sounds good to me.

comment:2 Changed 6 years ago by Tim Graham

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

comment:3 Changed 6 years ago by Aleksandr Sobolev

Owner: changed from nobody to Aleksandr Sobolev
Status: newassigned

comment:4 Changed 6 years ago by Aleksandr Sobolev

Triage Stage: AcceptedReady for checkin

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

comment:5 Changed 6 years ago by Tim Graham

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 Changed 6 years ago by Aleksandr Sobolev

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

comment:7 Changed 6 years ago by Tim Graham

Patch needs improvement: set

comment:8 Changed 6 years ago by Aleksandr Sobolev

Owner: Aleksandr Sobolev deleted
Status: assignednew

comment:9 Changed 6 years ago by Andy Craze

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 Changed 6 years ago by Tim Graham

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

comment:11 Changed 6 years ago by Andy Craze

Patch needs improvement: unset

comment:12 Changed 6 years ago by Tim Martin

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 Changed 6 years ago by Tim Martin

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:14 Changed 6 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 97c1931c:

Fixed #24423 -- Reorganized i18n tag tests.

comment:15 Changed 5 years ago by Tim Graham <timograham@…>

In a6b5321:

Refs #24423 -- Readded inadvertently deleted i18n tests.

Mistake in 97c1931c4f610e80053430d0297d51e1bed1e7ae.

comment:16 Changed 5 years ago by Tim Graham <timograham@…>

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