[Django] #33301: Documentation for assertFormError and assertFormsetError doesn't explain all arguments
Django
all arguments
------------------------------------------------+------------------------
Reporter: Baptiste Mispelon | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
------------------------------------------------+------------------------
The reference documentation for `assertFormError` [1] and
`assertFormsetError` [2] lists `response` as a required first argument but
doesn't explain what it is.
In contrast, `assertContains` (the next method on the page) is more
explicit:
> Asserts that a Response instance produced the given status_code [...]
(though it would have been nice if `Response` was a link to the relevant
doc)
[1]
https://docs.djangoproject.com/en/dev/topics/testing/tools/#django.test.SimpleTestCase.assertFormError
[2]
https://docs.djangoproject.com/en/dev/topics/testing/tools/#django.test.SimpleTestCase.assertFormsetError
--
Ticket URL: <https://code.djangoproject.com/ticket/33301>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
all arguments
Reporter: Baptiste Mispelon | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 3.2
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Unreviewed => Accepted
Comment:
OK, yes, thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/33301#comment:1>
Django
all arguments
Reporter: Baptiste Mispelon | Owner: Baptiste
Type: | Mispelon
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Baptiste Mispelon):
* owner: nobody => Baptiste Mispelon
* status: new => assigned
* easy: 1 => 0
Comment:
I might have been too quick to mark this as an "easy picking"...
After digging a little, I found that the situation is a bit messier than
I'd seen at first. Several custom `assert...` methods take a response
object as a required argument, but some of them will accept any
`HttpReponse` (and subclasses) whereas some require a response object from
the test client:
||= assert =||= Type of response =||= attribute =||
||
[[https://docs.djangoproject.com/en/dev/topics/testing/tools/#django.test.SimpleTestCase.assertFormError|assertFormError]]
|| Test client || `response.context` ||
||
[[https://docs.djangoproject.com/en/dev/topics/testing/tools/#django.test.SimpleTestCase.assertFormsetError|assertFormsetError]]
|| Test client || `response.context` ||
||
[[https://docs.djangoproject.com/en/dev/topics/testing/tools/#django.test.SimpleTestCase.assertContains|assertContains]]
|| Any || ||
||
[[https://docs.djangoproject.com/en/dev/topics/testing/tools/#django.test.SimpleTestCase.assertNotContains|assertNotContains]]
|| Any || ||
||
[[https://docs.djangoproject.com/en/dev/topics/testing/tools/#django.test.SimpleTestCase.assertTemplateUsed|assertTemplateUsed]]
|| Test client || `response.templates` ||
||
[[https://docs.djangoproject.com/en/dev/topics/testing/tools/#django.test.SimpleTestCase.assertTemplateNotUsed|assertTemplateNotUsed]]
|| Test client || `response.templates` ||
||
[[https://docs.djangoproject.com/en/dev/topics/testing/tools/#django.test.SimpleTestCase.assertRedirects|assertRedirects]]
|| Any || ||
On top of that, while `assertTemplateUsed` (and `assertTemplateNotUsed`)
does specific error checking to alert the user if they're not using a test
client response, `assertFormError` (and `assertFormsetError`) do not and
simply trigger an `AttributeError` if you're passing them the wrong kind
of response.
I'll get started on a proposed fix for all these issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/33301#comment:2>
Django
all arguments
Reporter: Baptiste Mispelon | Owner: Baptiste
Type: | Mispelon
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Baptiste Mispelon):
* has_patch: 0 => 1
Comment:
Here's the PR, split into 3 commits:
1) The documentation changes described in the original ticket
2) Missing tests for `assertFormError` and `assertFormsetError` (the
failure cases were not tested at all)
3) A small API change to make `assertFormError` and `assertFormsetError`
raise a `ValueError` with a helpful message (instead of an
`AttributeError`) when used with a non test-client response (same behavior
as `assertTemplateUsed`).
https://github.com/django/django/pull/15106
--
Ticket URL: <https://code.djangoproject.com/ticket/33301#comment:3>
Django
all arguments
Reporter: Baptiste Mispelon | Owner: Baptiste
Type: | Mispelon
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33301#comment:4>
Django
all arguments
Reporter: Baptiste Mispelon | Owner: Baptiste
Type: | Mispelon
Component: Documentation | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"528691d1b66b4ecf7bbb55f783fc919d40d4a235" 528691d]:
{{{
#!CommitTicketReference repository=""
revision="528691d1b66b4ecf7bbb55f783fc919d40d4a235"
Fixed #33301 -- Clarified the type of arguments required by custom
assertions.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33301#comment:5>
Django
all arguments
Reporter: Baptiste Mispelon | Owner: Baptiste
Type: | Mispelon
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"9ac92b1efc5ff90e7cce5c47fbd05887d403e4cd" 9ac92b1]:
{{{
#!CommitTicketReference repository=""
revision="9ac92b1efc5ff90e7cce5c47fbd05887d403e4cd"
Refs #33301 -- Made SimpleTestCase.assertFormError()/assertFormsetErrors()
raise ValueError for non test client responses.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33301#comment:6>
Django
all arguments
Reporter: Baptiste Mispelon | Owner: Baptiste
Type: | Mispelon
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"aa0c8ff9a0a0418dd3956a0be3de46e4dec956df" aa0c8ff9]:
{{{
#!CommitTicketReference repository=""
revision="aa0c8ff9a0a0418dd3956a0be3de46e4dec956df"
[4.0.x] Fixed #33301 -- Clarified the type of arguments required by custom
assertions.
Backport of 528691d1b66b4ecf7bbb55f783fc919d40d4a235 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33301#comment:7>