Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#33301 closed Cleanup/optimization (fixed)

Documentation for assertFormError and assertFormsetError doesn't explain all arguments

Reported by: Baptiste Mispelon Owned by: Baptiste Mispelon
Component: Documentation Version: 3.2
Severity: Normal Keywords:
Cc: Baptiste Mispelon Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


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)


Change History (7)

comment:1 by Carlton Gibson, 2 years ago

Triage Stage: UnreviewedAccepted

OK, yes, thanks.

comment:2 by Baptiste Mispelon, 2 years ago

Easy pickings: unset
Owner: changed from nobody to Baptiste Mispelon
Status: newassigned

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
assertFormError Test client response.context
assertFormsetError Test client response.context
assertContains Any
assertNotContains Any
assertTemplateUsed Test client response.templates
assertTemplateNotUsed Test client response.templates
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.

comment:3 by Baptiste Mispelon, 2 years ago

Has patch: set

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).

comment:4 by Mariusz Felisiak, 2 years ago

Triage Stage: AcceptedReady for checkin

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In 528691d:

Fixed #33301 -- Clarified the type of arguments required by custom assertions.

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 9ac92b1:

Refs #33301 -- Made SimpleTestCase.assertFormError()/assertFormsetErrors() raise ValueError for non test client responses.

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In aa0c8ff9:

[4.0.x] Fixed #33301 -- Clarified the type of arguments required by custom assertions.

Backport of 528691d1b66b4ecf7bbb55f783fc919d40d4a235 from main

Note: See TracTickets for help on using tickets.
Back to Top