Opened 7 years ago

Closed 6 years ago

Last modified 5 years ago

#12226 closed (fixed)

Test HttpResponse template attribute needlessly hard to use

Reported by: Carl Meyer Owned by: Carl Meyer
Component: Testing framework Version: master
Severity: Keywords: sprintSep2010
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

The template attribute of the response objects returned by Django's test client might be a single template object or might be a list of template objects, depending whether template inheritance was used. If one tries to write tests that are usable in the presence of user-overridden templates, this results in noisy boilerplate in the tests:

response = client.get('/')
try:
    template_name = response.template.name
except AttributeError:
    template_name = response.template[0].name

There's no need for all this: the attribute should simply always be a list, sometimes of length 1.

I would suggest that a new attribute templates could be added which is always a list, and the old attribute template could go through the usual deprecation cycle.

I'll work on a patch if this approach is accepted in principle.

Attachments (2)

12226_r13760.diff (3.4 KB) - added by Carl Meyer 6 years ago.
12226_r13760.2.diff (7.3 KB) - added by Carl Meyer 6 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by Eric Holscher

Triage Stage: UnreviewedAccepted

Sounds like this may be something we want to do similar to how context is accessed.

http://docs.djangoproject.com/en/dev/topics/testing/#django.test.client.Response.context

Changed 6 years ago by Carl Meyer

Attachment: 12226_r13760.diff added

Changed 6 years ago by Carl Meyer

Attachment: 12226_r13760.2.diff added

comment:2 Changed 6 years ago by Carl Meyer

Has patch: set
Keywords: sprintSep2010 added
Owner: changed from nobody to Carl Meyer
Version: 1.1SVN

Attached patch starts response.template on deprecation path with PendingDeprecationWarning, and adds response.templates API that is always a list, whether empty, length 1, or longer.

I wasn't sure what the policy is for tests in a case like this. The patch includes conversion of most tests to the new API, and addition of a couple new tests to verify the back-compat API still works. Alternatively, the current trunk tests still all pass unchanged if only the non-test portion of the patch is applied, so if the policy is to wait to convert tests until PendingDeprecation becomes Deprecation, that's easily done as well.

Git branch (same as patch) at http://github.com/carljm/django/commits/ticket_12226

comment:3 Changed 6 years ago by Eric Holscher

milestone: 1.3

comment:4 Changed 6 years ago by Russell Keith-Magee

Needs documentation: set
Patch needs improvement: set

Me likey.

However, this patch needs docs. We're deprecating a feature, so:

  • We need to put an entry in the release notes.
  • We need to put an entry in internals/deprecation.

comment:5 Changed 6 years ago by Carl Meyer

Resolution: fixed
Status: newclosed

(In [14106]) Fixed #12226 -- Deprecated test client Response.template attribute in favor of templates attribute, which is always a list. Thanks Russell for patch review.

comment:6 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

comment:11 Changed 5 years ago by Aymeric Augustin

In [17839]:

(The changeset message doesn't reference this ticket)

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