Code

Opened 16 months ago

Closed 14 months ago

Last modified 14 months ago

#20165 closed Cleanup/optimization (fixed)

Doc test example may lead to programmer errors

Reported by: lorinh Owned by: lorinh
Component: Documentation Version: master
Severity: Normal Keywords:
Cc: lorin@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: yes

Description

On the Testing Django applications page, the initial example subclasses from unittest.TestCase instead of django.test.TestCase.

Although there is some warning text beneath the example about the need to subclass from django.test.TestCase when hitting the database, it's likely that a Django programmer searching for a quick reference on how to structure a test class will treat this as a canonical example and miss the warning text below. (I know at least one person who this has happened to).

I believe it would minimize programmer error to use django.test.TestCase and to have the accompanying test discuss how you can use unittest.TestCase as an optimization when the test doesn't hit the database (as in the example).

I submitted a pull request but it was closed, referencing #15896, which is a slightly different issue. I also started a django-developers thread about this.

Attachments (0)

Change History (12)

comment:1 Changed 16 months ago by lorinh

  • Cc lorin@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • UI/UX set

comment:2 follow-up: Changed 16 months ago by aaugustin

What is the difference with #15896?

comment:3 follow-up: Changed 16 months ago by timo

This idea makes sense, but I think the example should be modified to actually require the database.

comment:4 in reply to: ↑ 2 Changed 16 months ago by lorinh

Replying to aaugustin:

What is the difference with #15896?

My understanding is that #15896 was based on a misunderstanding about the difference between django.utils.unittest.TestCase and django.test.TestCase, and so the author claimed (incorrectly) that there was a bug in the test example.

I'm suggesting that the test example, while technically correct, could be improved in a way that would reduce the likelihood of future programmer errors.

comment:5 in reply to: ↑ 3 Changed 16 months ago by lorinh

Replying to timo:

This idea makes sense, but I think the example should be modified to actually require the database.

Sounds good to me. I'd be happy to make the change. Would it be better to open a new pull request or modify the existing (closed) one?

comment:6 Changed 16 months ago by lorinh

The original ticket description had an incorrect link for the django-dev discussion, it's https://groups.google.com/d/topic/django-developers/xSWH2QYS4cE/discussion

comment:7 Changed 16 months ago by timo

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Regarding the pull request, I think either way is fine.

comment:8 Changed 15 months ago by charettes

  • Type changed from Uncategorized to Cleanup/optimization

comment:10 Changed 15 months ago by lorinh

  • Owner changed from nobody to lorinh
  • Status changed from new to assigned

comment:11 Changed 14 months ago by Tim Graham <timograham@…>

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

In 84d8b247d20aafb376368dc458f277ad2cd71ecd:

Fixed #20165 - Updated testing example to use django.test.TestCase.

Thanks Lorin Hochstein.

comment:12 Changed 14 months ago by Tim Graham <timograham@…>

In 7f4269229cd6ca0ef94a34fd045e3ced9bdd57d0:

[1.5.x] Fixed #20165 - Updated testing example to use django.test.TestCase.

Thanks Lorin Hochstein.

Backport of 84d8b247d2 from master.

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.