Code

Opened 4 years ago

Closed 4 years ago

Last modified 2 years ago

#12226 closed (fixed)

Test HttpResponse template attribute needlessly hard to use

Reported by: carljm Owned by: carljm
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 carljm 4 years ago.
12226_r13760.2.diff (7.3 KB) - added by carljm 4 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 4 years ago by ericholscher

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 4 years ago by carljm

Changed 4 years ago by carljm

comment:2 Changed 4 years ago by carljm

  • Has patch set
  • Keywords sprintSep2010 added
  • Owner changed from nobody to carljm
  • Version changed from 1.1 to SVN

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 4 years ago by ericholscher

  • milestone set to 1.3

comment:4 Changed 4 years ago by russellm

  • 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 4 years ago by carljm

  • Resolution set to fixed
  • Status changed from new to closed

(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 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

comment:11 Changed 2 years ago by aaugustin

In [17839]:

Removed deprecated attribute Response.template in the test client. Refs #12226.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.