#20165 closed Cleanup/optimization (fixed)
Doc test example may lead to programmer errors
Reported by: | Lorin Hochstein | Owned by: | Lorin Hochstein |
---|---|---|---|
Component: | Documentation | Version: | dev |
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.
Change History (12)
comment:1 by , 12 years ago
Cc: | added |
---|---|
UI/UX: | set |
follow-up: 4 comment:2 by , 12 years ago
follow-up: 5 comment:3 by , 12 years ago
This idea makes sense, but I think the example should be modified to actually require the database.
comment:4 by , 12 years ago
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 by , 12 years ago
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 by , 12 years ago
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 by , 12 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
Regarding the pull request, I think either way is fine.
comment:8 by , 12 years ago
Type: | Uncategorized → Cleanup/optimization |
---|
comment:10 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:11 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
What is the difference with #15896?