Opened 17 years ago
Closed 17 years ago
#5441 closed (fixed)
Add unit tests for django.contrib.formtools
| Reported by: | Adrian Holovaty | Owned by: | simeon |
|---|---|---|---|
| Component: | contrib.formtools | Version: | dev |
| Severity: | Keywords: | ||
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
The django.contrib.formtools package needs unit tests. See also #5432 (the documentation ticket).
Attachments (6)
Change History (22)
comment:1 by , 17 years ago
| Needs tests: | set |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 17 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:3 by , 17 years ago
| Owner: | changed from to |
|---|---|
| Status: | assigned → new |
comment:4 by , 17 years ago
| Owner: | changed from to |
|---|
comment:5 by , 17 years ago
| Has patch: | set |
|---|
comment:6 by , 17 years ago
| Has patch: | unset |
|---|
by , 17 years ago
by , 17 years ago
| Attachment: | test_urls.py added |
|---|
Url file to be used only by unit tests. See comments
comment:7 by , 17 years ago
| Has patch: | set |
|---|---|
| Needs tests: | unset |
| Owner: | changed from to |
| Status: | new → assigned |
I've attached three files:
- models.py - this is empty but running
runtests.py formtoolsgenerated an import error as the test runner attempts to import models for modules with tests. - test_urls.py - this contains urlpattern to map a url to the generated form to be tested. Couldn't think of any other way to do this.
- tests.py - Uses the test Client object to test the form. Currently there are only 4 tests which verify a form is generated, verify the preview stage, and verify that the form's done() method is fired if the preview is accepted.
I'm not very experienced with python, UnitTest, or Django yet, so I'll be happy to revise if anyone has comments or requests.
comment:8 by , 17 years ago
As per suggestions from gdoubleu in the sprint IRC channel: I've uploaded the output of svn diff in the django/contrib/formtools directory. I also updated the tests to use self.assertContains() to handle string discovery and status codes. I also refactored slightly to eliminate duplications.
Gdoubleu also suggested that test_urls.py may not be needed. I'm not very familiar with django internals yet and based my hack on the url dispatch process description at http://www.djangoproject.com/documentation/url_dispatch/ - if there's a better way to temporarily map urls->functions for the purposes of testing, I'd be glad to pursue it...
by , 17 years ago
comment:9 by , 17 years ago
Um, yes and trac wants a .diff extension to recognise a diff file. Please ignore the .txt file... It is 11pm on a Sunday :)
comment:10 by , 17 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Nobody else has commented on this, so am changing status to "Ready for checkin". I am willing to make improvements if any committers see things they don't like...
comment:11 by , 17 years ago
| Patch needs improvement: | set |
|---|---|
| Triage Stage: | Ready for checkin → Accepted |
In the future, please don't mark your own tickets ready for checkin; ask a triager or developer to review it for you.
There's a couple of problems with this patch:
- You need to run the svn diff from the root of the Django tree. See the contributing guide for more info.
- You need to find a better name for the unit test class ("tests" isn't very descriptive).
- You need to fix some of your code to comply with the coding guidlines in the contributing guide.
None of these are a big deal, and a developer will happily do this for you given enough time. However, if you'd like to expedite this ticket you can help us by doing those things yourself.
comment:12 by , 17 years ago
In the future, please don't mark your own tickets ready for checkin; ask a triager or developer to review it for you.
Will do. Is this something I should request on the Django dev list or IRC channel? Or just leave a note on the bug itself?
I will work on the patch to fix the items you mention - I should be able to update it tomorrow...
by , 17 years ago
| Attachment: | 5441.2.diff added |
|---|
comment:13 by , 17 years ago
Ok, I updated the patch (run from trunk directory). Latest version is 5441.2.diff. Class name is now "PreviewTests" - tests for other (future) parts of formtools could be added with a different class name. Methods of PreviewTests are now underscored instead of CamelCased and running pep8.py on the tests.py file results in no errors (did I really terminate a line with a semicolon? Sorry - my day job still involves lots of PHP.) If there are any other style issues I need to be aware of, please let me know.
I'm also opening a bug on the documentation to suggest that the "Contributing" section mention pep8.py. Maybe this is common knowledge to experienced devs but it helped me catch some issues I wouldn't have seen (trailing whitespace, semi-colons)...
comment:14 by , 17 years ago
| Patch needs improvement: | unset |
|---|
Am I allowed to uncheck "patch needs improvement"? Or should I let jacob uncheck it since he set it in the first place. In any event, it's been a couple weeks since the last update to the bug so I'm just trying to get it off my mental todo list...
comment:15 by , 17 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
patch was updated, bumping up.
comment:16 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Empty models.py file to place in contrib.formtools. See comments